[clang] f5314d1 - [Support] On Unix, let the CrashRecoveryContext return the signal code
Alexandre Ganea via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 24 05:21:59 PDT 2020
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
More information about the cfe-commits
mailing list