Skip to content

Commit f5314d1

Browse files
committed
[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
1 parent b3418cb commit f5314d1

File tree

6 files changed

+59
-5
lines changed

6 files changed

+59
-5
lines changed

clang/tools/driver/driver.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,13 @@ int main(int argc_, const char **argv_) {
531531
IsCrash = CommandRes < 0 || CommandRes == 70;
532532
#ifdef _WIN32
533533
IsCrash |= CommandRes == 3;
534+
#endif
535+
#if LLVM_ON_UNIX
536+
// When running in integrated-cc1 mode, the CrashRecoveryContext returns
537+
// the same codes as if the program crashed. See section "Exit Status for
538+
// Commands":
539+
// https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
540+
IsCrash |= CommandRes > 128;
534541
#endif
535542
if (IsCrash) {
536543
TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);

llvm/include/llvm/Support/Signals.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ namespace sys {
117117
/// Context is a system-specific failure context: it is the signal type on
118118
/// Unix; the ExceptionContext on Windows.
119119
void CleanupOnSignal(uintptr_t Context);
120+
121+
void unregisterHandlers();
120122
} // End sys namespace
121123
} // End llvm namespace
122124

llvm/lib/Support/CrashRecoveryContext.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,10 @@ static void CrashRecoverySignalHandler(int Signal) {
375375
sigaddset(&SigMask, Signal);
376376
sigprocmask(SIG_UNBLOCK, &SigMask, nullptr);
377377

378-
// As per convention, -2 indicates a crash or timeout as opposed to failure to
379-
// execute (see llvm/include/llvm/Support/Program.h)
380-
int RetCode = -2;
378+
// Return the same error code as if the program crashed, as mentioned in the
379+
// section "Exit Status for Commands":
380+
// https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
381+
int RetCode = 128 + Signal;
381382

382383
// Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
383384
if (Signal == SIGPIPE)

llvm/lib/Support/Unix/Signals.inc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ static void RegisterHandlers() { // Not signal-safe.
331331
registerHandler(S, SignalKind::IsInfo);
332332
}
333333

334-
static void UnregisterHandlers() {
334+
void sys::unregisterHandlers() {
335335
// Restore all of the signal handlers to how they were before we showed up.
336336
for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
337337
sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@ static RETSIGTYPE SignalHandler(int Sig) {
367367
// crashes when we return and the signal reissues. This also ensures that if
368368
// we crash in our signal handler that the program will terminate immediately
369369
// instead of recursing in the signal handler.
370-
UnregisterHandlers();
370+
sys::unregisterHandlers();
371371

372372
// Unmask all potentially blocked kill signals.
373373
sigset_t SigMask;

llvm/lib/Support/Windows/Signals.inc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,3 +869,5 @@ static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
869869
#pragma GCC diagnostic warning "-Wformat"
870870
#pragma GCC diagnostic warning "-Wformat-extra-args"
871871
#endif
872+
873+
void sys::unregisterHandlers() {}

llvm/unittests/Support/CrashRecoveryTest.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "llvm/ADT/Triple.h"
10+
#include "llvm/Support/CommandLine.h"
1011
#include "llvm/Support/Compiler.h"
1112
#include "llvm/Support/CrashRecoveryContext.h"
1213
#include "llvm/Support/FileSystem.h"
1314
#include "llvm/Support/Host.h"
15+
#include "llvm/Support/Program.h"
1416
#include "llvm/Support/Signals.h"
1517
#include "llvm/Support/raw_ostream.h"
1618
#include "gtest/gtest.h"
@@ -131,3 +133,43 @@ TEST(CrashRecoveryTest, Abort) {
131133
EXPECT_FALSE(CrashRecoveryContext().RunSafely(A));
132134
}
133135
#endif
136+
137+
// Specifically ensure that programs that signal() or abort() through the
138+
// CrashRecoveryContext can re-throw again their signal, so that `not --crash`
139+
// succeeds.
140+
#ifdef LLVM_ON_UNIX
141+
// See llvm/utils/unittest/UnitTestMain/TestMain.cpp
142+
extern const char *TestMainArgv0;
143+
144+
// Just a reachable symbol to ease resolving of the executable's path.
145+
static cl::opt<std::string> CrashTestStringArg1("crash-test-string-arg1");
146+
147+
TEST(CrashRecoveryTest, UnixCRCReturnCode) {
148+
using namespace llvm::sys;
149+
if (getenv("LLVM_CRC_UNIXCRCRETURNCODE")) {
150+
llvm::CrashRecoveryContext::Enable();
151+
CrashRecoveryContext CRC;
152+
EXPECT_FALSE(CRC.RunSafely(abort));
153+
EXPECT_EQ(CRC.RetCode, 128 + SIGABRT);
154+
// re-throw signal
155+
llvm::sys::unregisterHandlers();
156+
raise(CRC.RetCode - 128);
157+
llvm_unreachable("Should have exited already!");
158+
}
159+
160+
std::string Executable =
161+
sys::fs::getMainExecutable(TestMainArgv0, &CrashTestStringArg1);
162+
StringRef argv[] = {
163+
Executable, "--gtest_filter=CrashRecoveryTest.UnixCRCReturnCode"};
164+
165+
// Add LLVM_CRC_UNIXCRCRETURNCODE to the environment of the child process.
166+
std::vector<StringRef> EnvTable;
167+
EnvTable.push_back("LLVM_CRC_UNIXCRCRETURNCODE=1");
168+
169+
std::string Error;
170+
bool ExecutionFailed;
171+
int RetCode = ExecuteAndWait(Executable, argv, makeArrayRef(EnvTable), {}, 0, 0, &Error,
172+
&ExecutionFailed);
173+
ASSERT_EQ(-2, RetCode);
174+
}
175+
#endif

0 commit comments

Comments
 (0)