Author: Alexandre Ganea Date: 2020-09-24T08:21:43-04:00 New Revision: f5314d15af4f4514103ea12c74cb208538b8bef5
URL: https://github.com/llvm/llvm-project/commit/f5314d15af4f4514103ea12c74cb208538b8bef5 DIFF: https://github.com/llvm/llvm-project/commit/f5314d15af4f4514103ea12c74cb208538b8bef5.diff LOG: [Support] On Unix, let the CrashRecoveryContext return the signal code Before this patch, the CrashRecoveryContext was returning -2 upon a signal, like ExecuteAndWait does. This didn't match the behavior on Windows, where the the exception code was returned. We now return the signal's code, which optionally allows for re-throwing the signal later. Doing so requires all custom handlers to be removed first, through llvm::sys::unregisterHandlers() which we made a public API. This is part of https://reviews.llvm.org/D70378 Added: Modified: clang/tools/driver/driver.cpp llvm/include/llvm/Support/Signals.h llvm/lib/Support/CrashRecoveryContext.cpp llvm/lib/Support/Unix/Signals.inc llvm/lib/Support/Windows/Signals.inc llvm/unittests/Support/CrashRecoveryTest.cpp Removed: ################################################################################ diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index f24fd61e61a5..f67af6790fff 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -531,6 +531,13 @@ int main(int argc_, const char **argv_) { IsCrash = CommandRes < 0 || CommandRes == 70; #ifdef _WIN32 IsCrash |= CommandRes == 3; +#endif +#if LLVM_ON_UNIX + // When running in integrated-cc1 mode, the CrashRecoveryContext returns + // the same codes as if the program crashed. See section "Exit Status for + // Commands": + // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html + IsCrash |= CommandRes > 128; #endif if (IsCrash) { TheDriver.generateCompilationDiagnostics(*C, *FailingCommand); diff --git a/llvm/include/llvm/Support/Signals.h b/llvm/include/llvm/Support/Signals.h index c5b94f5ac776..44f5a750ff5c 100644 --- a/llvm/include/llvm/Support/Signals.h +++ b/llvm/include/llvm/Support/Signals.h @@ -117,6 +117,8 @@ namespace sys { /// Context is a system-specific failure context: it is the signal type on /// Unix; the ExceptionContext on Windows. void CleanupOnSignal(uintptr_t Context); + + void unregisterHandlers(); } // End sys namespace } // End llvm namespace diff --git a/llvm/lib/Support/CrashRecoveryContext.cpp b/llvm/lib/Support/CrashRecoveryContext.cpp index 2a41754c7786..7609f04cf68c 100644 --- a/llvm/lib/Support/CrashRecoveryContext.cpp +++ b/llvm/lib/Support/CrashRecoveryContext.cpp @@ -375,9 +375,10 @@ static void CrashRecoverySignalHandler(int Signal) { sigaddset(&SigMask, Signal); sigprocmask(SIG_UNBLOCK, &SigMask, nullptr); - // As per convention, -2 indicates a crash or timeout as opposed to failure to - // execute (see llvm/include/llvm/Support/Program.h) - int RetCode = -2; + // Return the same error code as if the program crashed, as mentioned in the + // section "Exit Status for Commands": + // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html + int RetCode = 128 + Signal; // Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp) if (Signal == SIGPIPE) diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc index 50b2ad4b5772..03181a713257 100644 --- a/llvm/lib/Support/Unix/Signals.inc +++ b/llvm/lib/Support/Unix/Signals.inc @@ -331,7 +331,7 @@ static void RegisterHandlers() { // Not signal-safe. registerHandler(S, SignalKind::IsInfo); } -static void UnregisterHandlers() { +void sys::unregisterHandlers() { // Restore all of the signal handlers to how they were before we showed up. for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) { sigaction(RegisteredSignalInfo[i].SigNo, @@ -367,7 +367,7 @@ static RETSIGTYPE SignalHandler(int Sig) { // crashes when we return and the signal reissues. This also ensures that if // we crash in our signal handler that the program will terminate immediately // instead of recursing in the signal handler. - UnregisterHandlers(); + sys::unregisterHandlers(); // Unmask all potentially blocked kill signals. sigset_t SigMask; diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc index 71dc6324e99f..3758582b35f7 100644 --- a/llvm/lib/Support/Windows/Signals.inc +++ b/llvm/lib/Support/Windows/Signals.inc @@ -869,3 +869,5 @@ static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) { #pragma GCC diagnostic warning "-Wformat" #pragma GCC diagnostic warning "-Wformat-extra-args" #endif + +void sys::unregisterHandlers() {} diff --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp index a8040dc0110e..fc65bf6329b3 100644 --- a/llvm/unittests/Support/CrashRecoveryTest.cpp +++ b/llvm/unittests/Support/CrashRecoveryTest.cpp @@ -7,10 +7,12 @@ //===----------------------------------------------------------------------===// #include "llvm/ADT/Triple.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Host.h" +#include "llvm/Support/Program.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" #include "gtest/gtest.h" @@ -131,3 +133,43 @@ TEST(CrashRecoveryTest, Abort) { EXPECT_FALSE(CrashRecoveryContext().RunSafely(A)); } #endif + +// Specifically ensure that programs that signal() or abort() through the +// CrashRecoveryContext can re-throw again their signal, so that `not --crash` +// succeeds. +#ifdef LLVM_ON_UNIX +// See llvm/utils/unittest/UnitTestMain/TestMain.cpp +extern const char *TestMainArgv0; + +// Just a reachable symbol to ease resolving of the executable's path. +static cl::opt<std::string> CrashTestStringArg1("crash-test-string-arg1"); + +TEST(CrashRecoveryTest, UnixCRCReturnCode) { + using namespace llvm::sys; + if (getenv("LLVM_CRC_UNIXCRCRETURNCODE")) { + llvm::CrashRecoveryContext::Enable(); + CrashRecoveryContext CRC; + EXPECT_FALSE(CRC.RunSafely(abort)); + EXPECT_EQ(CRC.RetCode, 128 + SIGABRT); + // re-throw signal + llvm::sys::unregisterHandlers(); + raise(CRC.RetCode - 128); + llvm_unreachable("Should have exited already!"); + } + + std::string Executable = + sys::fs::getMainExecutable(TestMainArgv0, &CrashTestStringArg1); + StringRef argv[] = { + Executable, "--gtest_filter=CrashRecoveryTest.UnixCRCReturnCode"}; + + // Add LLVM_CRC_UNIXCRCRETURNCODE to the environment of the child process. + std::vector<StringRef> EnvTable; + EnvTable.push_back("LLVM_CRC_UNIXCRCRETURNCODE=1"); + + std::string Error; + bool ExecutionFailed; + int RetCode = ExecuteAndWait(Executable, argv, makeArrayRef(EnvTable), {}, 0, 0, &Error, + &ExecutionFailed); + ASSERT_EQ(-2, RetCode); +} +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
