Index: clang/lib/Lex/Pragma.cpp =================================================================== --- clang/lib/Lex/Pragma.cpp +++ clang/lib/Lex/Pragma.cpp @@ -1105,10 +1105,6 @@ M->dump(); } else if (II->isStr("overflow_stack")) { DebugOverflowStack(); - } else if (II->isStr("handle_crash")) { - llvm::CrashRecoveryContext *CRC =llvm::CrashRecoveryContext::GetCurrent(); - if (CRC) - CRC->HandleCrash(); } else if (II->isStr("captured")) { HandleCaptured(PP); } else { Index: llvm/include/llvm/Support/CrashRecoveryContext.h =================================================================== --- llvm/include/llvm/Support/CrashRecoveryContext.h +++ llvm/include/llvm/Support/CrashRecoveryContext.h @@ -44,11 +44,11 @@ /// executed in any case, whether crash occurs or not. These actions may be used /// to reclaim resources in the case of crash. class CrashRecoveryContext { - void *Impl; - CrashRecoveryContextCleanup *head; + void *Impl = nullptr; + CrashRecoveryContextCleanup *head = nullptr; public: - CrashRecoveryContext() : Impl(nullptr), head(nullptr) {} + CrashRecoveryContext(); ~CrashRecoveryContext(); /// Register cleanup handler, which is used when the recovery context is @@ -97,9 +97,13 @@ return RunSafelyOnThread([&]() { Fn(UserData); }, RequestedStackSize); } - /// Explicitly trigger a crash recovery in the current process, and - /// return failure from RunSafely(). This function does not return. - void HandleCrash(); + /// In case of a crash, this is the crash identifier. + int RetCode = 0; + + /// Selects whether handling of exceptions should be done in the same way as + /// for global exceptions. When this is active, a crash would print the + /// callstack, clean-up any temporary files and create a coredump/minidump. + bool DumpStackAndCleanupOnFailure = false; }; /// Abstract base class of cleanup handlers. Index: llvm/include/llvm/Support/Signals.h =================================================================== --- llvm/include/llvm/Support/Signals.h +++ llvm/include/llvm/Support/Signals.h @@ -106,6 +106,13 @@ /// On Unix systems, this function exits with an "IO error" exit code. /// This is a no-op on Windows. void DefaultOneShotPipeSignalHandler(); + + /// This function does the following: + /// - clean up any temporary files registered with RemoveFileOnSignal() + /// - dump the callstack from the exception context + /// - call any relevant interrupt/signal handlers + /// - create a core/mini dump of the exception context whenever possible + void CleanupOnSignal(uintptr_t ExceptContext); } // End sys namespace } // End llvm namespace Index: llvm/lib/Support/CrashRecoveryContext.cpp =================================================================== --- llvm/lib/Support/CrashRecoveryContext.cpp +++ llvm/lib/Support/CrashRecoveryContext.cpp @@ -10,9 +10,17 @@ #include "llvm/Config/llvm-config.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/ManagedStatic.h" +#include "llvm/Support/Signals.h" #include "llvm/Support/ThreadLocal.h" #include #include +#ifdef _WIN32 +#include // for GetExceptionInformation +#endif +#if LLVM_ON_UNIX +#include // EX_IOERR +#endif + using namespace llvm; namespace { @@ -54,7 +62,7 @@ #endif } - void HandleCrash() { + void HandleCrash(int RetCode, uintptr_t ExceptContext) { // Eliminate the current context entry, to avoid re-entering in case the // cleanup code crashes. CurrentContext->set(Next); @@ -62,7 +70,10 @@ assert(!Failed && "Crash recovery context already failed!"); Failed = true; - // FIXME: Stash the backtrace. + if (CRC->DumpStackAndCleanupOnFailure) + sys::CleanupOnSignal(ExceptContext); + + CRC->RetCode = RetCode; // Jump back to the RunSafely we were called under. longjmp(JumpBuffer, 1); @@ -71,8 +82,9 @@ } +static std::atomic gCrashRecoveryEnabledCount; static ManagedStatic gCrashRecoveryContextMutex; -static bool gCrashRecoveryEnabled = false; +static bool gCrashRecoveryInstalled; static ManagedStatic> tlIsRecoveringFromCrash; @@ -80,8 +92,30 @@ static void installExceptionOrSignalHandlers(); static void uninstallExceptionOrSignalHandlers(); +static void Install() { + if (gCrashRecoveryEnabledCount <= 0) + return; + std::lock_guard L(*gCrashRecoveryContextMutex); + if (gCrashRecoveryInstalled) + return; + installExceptionOrSignalHandlers(); + gCrashRecoveryInstalled = true; +} + +static void Uninstall(bool Force) { + if (!Force && gCrashRecoveryEnabledCount > 0) + return; + std::lock_guard L(*gCrashRecoveryContextMutex); + if (!gCrashRecoveryInstalled) + return; + uninstallExceptionOrSignalHandlers(); + gCrashRecoveryInstalled = false; +} + CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {} +CrashRecoveryContext::CrashRecoveryContext() { Install(); } + CrashRecoveryContext::~CrashRecoveryContext() { // Reclaim registered resources. CrashRecoveryContextCleanup *i = head; @@ -105,7 +139,7 @@ } CrashRecoveryContext *CrashRecoveryContext::GetCurrent() { - if (!gCrashRecoveryEnabled) + if (gCrashRecoveryEnabledCount <= 0) return nullptr; const CrashRecoveryContextImpl *CRCI = CurrentContext->get(); @@ -115,21 +149,11 @@ return CRCI->CRC; } -void CrashRecoveryContext::Enable() { - std::lock_guard L(*gCrashRecoveryContextMutex); - // FIXME: Shouldn't this be a refcount or something? - if (gCrashRecoveryEnabled) - return; - gCrashRecoveryEnabled = true; - installExceptionOrSignalHandlers(); -} +void CrashRecoveryContext::Enable() { ++gCrashRecoveryEnabledCount; } void CrashRecoveryContext::Disable() { - std::lock_guard L(*gCrashRecoveryContextMutex); - if (!gCrashRecoveryEnabled) - return; - gCrashRecoveryEnabled = false; - uninstallExceptionOrSignalHandlers(); + --gCrashRecoveryEnabledCount; + Uninstall(/*Force=*/ false); } void CrashRecoveryContext::registerCleanup(CrashRecoveryContextCleanup *cleanup) @@ -171,19 +195,32 @@ static void installExceptionOrSignalHandlers() {} static void uninstallExceptionOrSignalHandlers() {} -bool CrashRecoveryContext::RunSafely(function_ref Fn) { - if (!gCrashRecoveryEnabled) { +// We need this function because the call to GetExceptionInformation() can only +// occur inside the __except evaluation block +static inline int ExceptionFilter(bool DumpStackAndCleanup, + _EXCEPTION_POINTERS *Except) { + if (DumpStackAndCleanup) + sys::CleanupOnSignal((uintptr_t)Except); + return EXCEPTION_EXECUTE_HANDLER; +} + +static bool InvokeFunctionCall(function_ref Fn, + bool DumpStackAndCleanup, int &RetCode) { + __try { Fn(); - return true; + } __except (ExceptionFilter(DumpStackAndCleanup, GetExceptionInformation())) { + RetCode = GetExceptionCode(); + return false; } + return true; +} - bool Result = true; - __try { +bool CrashRecoveryContext::RunSafely(function_ref Fn) { + if (gCrashRecoveryEnabledCount <= 0) { Fn(); - } __except (1) { // Catch any exception. - Result = false; + return true; } - return Result; + return InvokeFunctionCall(Fn, DumpStackAndCleanupOnFailure, RetCode); } #else // !_MSC_VER @@ -229,7 +266,7 @@ if (!CRCI) { // Something has gone horribly wrong, so let's just tell everyone // to keep searching - CrashRecoveryContext::Disable(); + Uninstall(/*Force=*/true); return EXCEPTION_CONTINUE_SEARCH; } @@ -237,7 +274,8 @@ // implementation if we so choose. // Handle the crash - const_cast(CRCI)->HandleCrash(); + const_cast(CRCI)->HandleCrash( + (int)ExceptionInfo->ExceptionRecord->ExceptionCode, ExceptionInfo); // Note that we don't actually get here because HandleCrash calls // longjmp, which means the HandleCrash function never returns. @@ -304,9 +342,7 @@ // Disable crash recovery and raise the signal again. The assumption here is // that the enclosing application will terminate soon, and we won't want to // attempt crash recovery again. - // - // This call of Disable isn't thread safe, but it doesn't actually matter. - CrashRecoveryContext::Disable(); + Uninstall(/*Force=*/ true); raise(Signal); // The signal will be thrown once the signal mask is restored. @@ -319,8 +355,16 @@ 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; + + // Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp) + if (Signal == SIGPIPE) + RetCode = EX_IOERR; + if (CRCI) - const_cast(CRCI)->HandleCrash(); + const_cast(CRCI)->HandleCrash(RetCode, Signal); } static void installExceptionOrSignalHandlers() { @@ -345,7 +389,7 @@ bool CrashRecoveryContext::RunSafely(function_ref Fn) { // If crash recovery is disabled, do nothing. - if (gCrashRecoveryEnabled) { + if (gCrashRecoveryEnabledCount > 0) { assert(!Impl && "Crash recovery context already initialized!"); CrashRecoveryContextImpl *CRCI = new CrashRecoveryContextImpl(this); Impl = CRCI; @@ -361,12 +405,6 @@ #endif // !_MSC_VER -void CrashRecoveryContext::HandleCrash() { - CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl; - assert(CRCI && "Crash recovery context never initialized!"); - CRCI->HandleCrash(); -} - // FIXME: Portability. static void setThreadBackgroundPriority() { #ifdef __APPLE__ Index: llvm/lib/Support/Unix/Signals.inc =================================================================== --- llvm/lib/Support/Unix/Signals.inc +++ llvm/lib/Support/Unix/Signals.inc @@ -209,7 +209,7 @@ /// if there is, it's not our direct responsibility. For whatever reason, our /// continued execution is no longer desirable. static const int IntSigs[] = { - SIGHUP, SIGINT, SIGTERM, SIGUSR2 + SIGHUP, SIGINT, SIGTERM, SIGUSR2, SIGPIPE }; /// Signals that represent that we have a bug, and our prompt termination has @@ -240,7 +240,7 @@ static const size_t NumSigs = array_lengthof(IntSigs) + array_lengthof(KillSigs) + - array_lengthof(InfoSigs) + 1 /* SIGPIPE */; + array_lengthof(InfoSigs); static std::atomic NumRegisteredSignals = ATOMIC_VAR_INIT(0); @@ -325,8 +325,6 @@ registerHandler(S, SignalKind::IsKill); for (auto S : KillSigs) registerHandler(S, SignalKind::IsKill); - if (OneShotPipeSignalFunction) - registerHandler(SIGPIPE, SignalKind::IsKill); for (auto S : InfoSigs) registerHandler(S, SignalKind::IsInfo); } @@ -345,6 +343,20 @@ FileToRemoveList::removeAllFiles(FilesToRemove); } +void sys::CleanupOnSignal(uintptr_t ExceptContext) { + int Sig = (int)ExceptContext; + + if (llvm::is_contained(InfoSigs, Sig)) { + InfoSignalHandler(Sig); + return; + } + + RemoveFilesToRemove(); + + if (!llvm::is_contained(IntSigs, Sig)) + llvm::sys::RunSignalHandlers(); +} + // The signal handler that runs. static RETSIGTYPE SignalHandler(int Sig) { // Restore the signal behavior to default, so that the program actually @@ -358,27 +370,21 @@ sigfillset(&SigMask); sigprocmask(SIG_UNBLOCK, &SigMask, nullptr); - { - RemoveFilesToRemove(); + sys::CleanupOnSignal(Sig); + + if (llvm::is_contained(IntSigs, Sig)) { + if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr)) + return OldInterruptFunction(); if (Sig == SIGPIPE) if (auto OldOneShotPipeFunction = OneShotPipeSignalFunction.exchange(nullptr)) return OldOneShotPipeFunction(); - if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) - != std::end(IntSigs)) { - if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr)) - return OldInterruptFunction(); - - raise(Sig); // Execute the default handler. - return; - } + raise(Sig); // Execute the default handler. + return; } - // Otherwise if it is a fault (like SEGV) run any handler. - llvm::sys::RunSignalHandlers(); - #ifdef __s390__ // On S/390, certain signals are delivered with PSW Address pointing to // *after* the faulting instruction. Simply returning from the signal Index: llvm/lib/Support/Windows/Signals.inc =================================================================== --- llvm/lib/Support/Windows/Signals.inc +++ llvm/lib/Support/Windows/Signals.inc @@ -521,10 +521,13 @@ extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord); #endif -void llvm::sys::PrintStackTrace(raw_ostream &OS) { - STACKFRAME64 StackFrame = {}; - CONTEXT Context = {}; - ::RtlCaptureContext(&Context); +static void LocalPrintStackTrace(raw_ostream &OS, PCONTEXT C) { + STACKFRAME64 StackFrame{}; + CONTEXT Context{}; + if (!C) { + ::RtlCaptureContext(&Context); + C = &Context; + } #if defined(_M_X64) StackFrame.AddrPC.Offset = Context.Rip; StackFrame.AddrStack.Offset = Context.Rsp; @@ -546,9 +549,12 @@ StackFrame.AddrStack.Mode = AddrModeFlat; StackFrame.AddrFrame.Mode = AddrModeFlat; PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(), - StackFrame, &Context); + StackFrame, C); } +void llvm::sys::PrintStackTrace(raw_ostream &OS) { + LocalPrintStackTrace(OS, nullptr); +} void llvm::sys::SetInterruptFunction(void (*IF)()) { RegisterHandler(); @@ -792,6 +798,10 @@ return std::error_code(); } +void sys::CleanupOnSignal(uintptr_t ExceptContext) { + LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)ExceptContext); +} + static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) { Cleanup(); @@ -810,42 +820,9 @@ << "\n"; } - // Initialize the STACKFRAME structure. - STACKFRAME64 StackFrame = {}; - -#if defined(_M_X64) - StackFrame.AddrPC.Offset = ep->ContextRecord->Rip; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp; - StackFrame.AddrStack.Mode = AddrModeFlat; - StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp; - StackFrame.AddrFrame.Mode = AddrModeFlat; -#elif defined(_M_IX86) - StackFrame.AddrPC.Offset = ep->ContextRecord->Eip; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Esp; - StackFrame.AddrStack.Mode = AddrModeFlat; - StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp; - StackFrame.AddrFrame.Mode = AddrModeFlat; -#elif defined(_M_ARM64) || defined(_M_ARM) - StackFrame.AddrPC.Offset = ep->ContextRecord->Pc; - StackFrame.AddrPC.Mode = AddrModeFlat; - StackFrame.AddrStack.Offset = ep->ContextRecord->Sp; - StackFrame.AddrStack.Mode = AddrModeFlat; -#if defined(_M_ARM64) - StackFrame.AddrFrame.Offset = ep->ContextRecord->Fp; -#else - StackFrame.AddrFrame.Offset = ep->ContextRecord->R11; -#endif - StackFrame.AddrFrame.Mode = AddrModeFlat; -#endif - - HANDLE hProcess = GetCurrentProcess(); - HANDLE hThread = GetCurrentThread(); - PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame, - ep->ContextRecord); + LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr); - _exit(ep->ExceptionRecord->ExceptionCode); + return EXCEPTION_EXECUTE_HANDLER; } static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) { Index: llvm/unittests/Support/CrashRecoveryTest.cpp =================================================================== --- llvm/unittests/Support/CrashRecoveryTest.cpp +++ llvm/unittests/Support/CrashRecoveryTest.cpp @@ -8,6 +8,8 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/CrashRecoveryContext.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Signals.h" #include "gtest/gtest.h" #ifdef _WIN32 @@ -23,6 +25,8 @@ static void nullDeref() { *(volatile int *)0x10 = 0; } static void incrementGlobal() { ++GlobalInt; } static void llvmTrap() { LLVM_BUILTIN_TRAP; } +static void incrementGlobalWithCookie(void*) { ++GlobalInt; } + TEST(CrashRecoveryTest, Basic) { llvm::CrashRecoveryContext::Enable(); @@ -31,6 +35,7 @@ EXPECT_EQ(1, GlobalInt); EXPECT_FALSE(CrashRecoveryContext().RunSafely(nullDeref)); EXPECT_FALSE(CrashRecoveryContext().RunSafely(llvmTrap)); + llvm::CrashRecoveryContext::Disable(); } struct IncrementGlobalCleanup : CrashRecoveryContextCleanup { @@ -58,6 +63,56 @@ EXPECT_FALSE(CRC.RunSafely(nullDeref)); } // run cleanups EXPECT_EQ(1, GlobalInt); + llvm::CrashRecoveryContext::Disable(); +} + +TEST(CrashRecoveryTest, DumpStackCleanup) { + SmallString<128> Filename; + std::error_code EC = sys::fs::createTemporaryFile("crash", "test", Filename); + EXPECT_FALSE(EC); + sys::RemoveFileOnSignal(Filename); + llvm::sys::AddSignalHandler(incrementGlobalWithCookie, nullptr); + GlobalInt = 0; + llvm::CrashRecoveryContext::Enable(); + { + CrashRecoveryContext CRC; + CRC.DumpStackAndCleanupOnFailure = true; + EXPECT_TRUE(CRC.RunSafely(noop)); + } + EXPECT_TRUE(sys::fs::exists(Filename)); + EXPECT_EQ(GlobalInt, 0); + { + CrashRecoveryContext CRC; + CRC.DumpStackAndCleanupOnFailure = true; + EXPECT_FALSE(CRC.RunSafely(nullDeref)); + EXPECT_NE(CRC.RetCode, 0); + } + EXPECT_FALSE(sys::fs::exists(Filename)); + EXPECT_EQ(GlobalInt, 1); + llvm::CrashRecoveryContext::Disable(); +} + +TEST(CrashRecoveryTest, RefCountEnable) { + // CrashRecoveryContext's internal ref count should be zero + CrashRecoveryContext CRC; + EXPECT_DEATH(CRC.RunSafely(nullDeref), ""); + + llvm::CrashRecoveryContext::Enable(); + EXPECT_FALSE(CRC.RunSafely(nullDeref)); + llvm::CrashRecoveryContext::Disable(); + + EXPECT_DEATH(CRC.RunSafely(nullDeref), ""); + + llvm::CrashRecoveryContext::Enable(); + llvm::CrashRecoveryContext::Enable(); + // refcount = 2 + EXPECT_FALSE(CRC.RunSafely(nullDeref)); + llvm::CrashRecoveryContext::Disable(); + // refcount = 1 + EXPECT_FALSE(CRC.RunSafely(nullDeref)); + llvm::CrashRecoveryContext::Disable(); + // refcount = 0 + EXPECT_DEATH(CRC.RunSafely(nullDeref), ""); } #ifdef _WIN32 @@ -68,6 +123,7 @@ TEST(CrashRecoveryTest, RaiseException) { llvm::CrashRecoveryContext::Enable(); EXPECT_FALSE(CrashRecoveryContext().RunSafely(raiseIt)); + llvm::CrashRecoveryContext::Disable(); } static void outputString() { @@ -77,6 +133,7 @@ TEST(CrashRecoveryTest, CallOutputDebugString) { llvm::CrashRecoveryContext::Enable(); EXPECT_TRUE(CrashRecoveryContext().RunSafely(outputString)); + llvm::CrashRecoveryContext::Disable(); } #endif