Index: clang/test/Modules/signal.m =================================================================== --- clang/test/Modules/signal.m +++ clang/test/Modules/signal.m @@ -2,10 +2,10 @@ // RUN: rm -rf %t // Crash building module. -// RUN: not --crash %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs %s +// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs %s -// The dead symlink is still around, but the underlying lock file is gone. -// RUN: find %t -name "crash-*.pcm.lock" | count 1 +// All the temp files should be gone. +// RUN: find %t -name "crash-*.pcm.lock" | count 0 // RUN: find %t -name "crash-*.pcm.lock-*" | count 0 @import crash; Index: clang/tools/libclang/CIndex.cpp =================================================================== --- clang/tools/libclang/CIndex.cpp +++ clang/tools/libclang/CIndex.cpp @@ -3290,8 +3290,8 @@ int displayDiagnostics) { // We use crash recovery to make some of our APIs more reliable, implicitly // enable it. - if (!getenv("LIBCLANG_DISABLE_CRASH_RECOVERY")) - llvm::CrashRecoveryContext::Enable(); + if (getenv("LIBCLANG_DISABLE_CRASH_RECOVERY")) + llvm::CrashRecoveryContext::Disable(); // Look through the managed static to trigger construction of the managed // static which registers our fatal error handler. This ensures it is only Index: llvm/include/llvm/Support/CrashRecoveryContext.h =================================================================== --- llvm/include/llvm/Support/CrashRecoveryContext.h +++ llvm/include/llvm/Support/CrashRecoveryContext.h @@ -48,7 +48,7 @@ CrashRecoveryContextCleanup *head; public: - CrashRecoveryContext() : Impl(nullptr), head(nullptr) {} + CrashRecoveryContext(); ~CrashRecoveryContext(); /// Register cleanup handler, which is used when the recovery context is @@ -100,6 +100,15 @@ /// 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 the global exception handler should be called. When this + /// is active, a crash has the same side-effect as uncatched code (callstack + /// print and coredump/minidump), except that here we recover the execution + /// flow after the call to RunSafely(). + bool EnableExceptionHandler = 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,14 @@ /// On Unix systems, this function exits with an "IO error" exit code. /// This is a no-op on Windows. void DefaultOneShotPipeSignalHandler(); + + /// Forcibly triggers the exception processing, but does not throw the + /// exception afterwards. Let the handler modify the RetCode to properly + /// handle diagnostics in the clang tool. + /// The return value is platform-specific: on Windows it indicates what action + /// the exception handler should take next; on Linux, the return value is + /// ignored and is always zero. + signed InvokeExceptionHandler(int &RetCode, void *ExceptionInfo); } // 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,13 @@ #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 using namespace llvm; namespace { @@ -54,7 +58,7 @@ #endif } - void HandleCrash() { + void HandleCrash(int RetCode, void *ExceptionInfo) { // Eliminate the current context entry, to avoid re-entering in case the // cleanup code crashes. CurrentContext->set(Next); @@ -62,7 +66,11 @@ assert(!Failed && "Crash recovery context already failed!"); Failed = true; - // FIXME: Stash the backtrace. + if (CRC->EnableExceptionHandler) + sys::InvokeExceptionHandler(RetCode, ExceptionInfo); + + // Should come after InvokeExceptionHandler(), it could modify RetCode. + CRC->RetCode = RetCode; // Jump back to the RunSafely we were called under. longjmp(JumpBuffer, 1); @@ -71,8 +79,9 @@ } +static std::atomic gCrashRecoveryEnabled{ true }; static ManagedStatic gCrashRecoveryContextMutex; -static bool gCrashRecoveryEnabled = false; +static bool gCrashRecoveryInstalled; static ManagedStatic> tlIsRecoveringFromCrash; @@ -80,8 +89,32 @@ static void installExceptionOrSignalHandlers(); static void uninstallExceptionOrSignalHandlers(); +static void Install() { + if (!gCrashRecoveryEnabled) + return; + std::lock_guard L(*gCrashRecoveryContextMutex); + if (gCrashRecoveryInstalled) + return; + installExceptionOrSignalHandlers(); + gCrashRecoveryInstalled = true; +} + +static void Uninstall() { + if (!gCrashRecoveryEnabled) + return; + std::lock_guard L(*gCrashRecoveryContextMutex); + if (!gCrashRecoveryInstalled) + return; + uninstallExceptionOrSignalHandlers(); + gCrashRecoveryInstalled = false; +} + CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {} +CrashRecoveryContext::CrashRecoveryContext() : Impl(nullptr), head(nullptr) { + Install(); +} + CrashRecoveryContext::~CrashRecoveryContext() { // Reclaim registered resources. CrashRecoveryContextCleanup *i = head; @@ -115,21 +148,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() { gCrashRecoveryEnabled = true; } void CrashRecoveryContext::Disable() { - std::lock_guard L(*gCrashRecoveryContextMutex); - if (!gCrashRecoveryEnabled) - return; + Uninstall(); gCrashRecoveryEnabled = false; - uninstallExceptionOrSignalHandlers(); } void CrashRecoveryContext::registerCleanup(CrashRecoveryContextCleanup *cleanup) @@ -171,19 +194,25 @@ static void installExceptionOrSignalHandlers() {} static void uninstallExceptionOrSignalHandlers() {} -bool CrashRecoveryContext::RunSafely(function_ref Fn) { - if (!gCrashRecoveryEnabled) { +static bool InvokeFunctionCall(function_ref Fn, bool ExceptionHandler, + int &RetCode) { + __try { Fn(); - return true; + } __except (ExceptionHandler ? sys::InvokeExceptionHandler( + RetCode, GetExceptionInformation()) + : 1) { + RetCode = GetExceptionCode(); + return false; } + return true; +} - bool Result = true; - __try { +bool CrashRecoveryContext::RunSafely(function_ref Fn) { + if (!gCrashRecoveryEnabled) { Fn(); - } __except (1) { // Catch any exception. - Result = false; + return true; } - return Result; + return InvokeFunctionCall(Fn, EnableExceptionHandler, RetCode); } #else // !_MSC_VER @@ -237,7 +266,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,8 +334,6 @@ // 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(); raise(Signal); @@ -320,7 +348,7 @@ sigprocmask(SIG_UNBLOCK, &SigMask, nullptr); if (CRCI) - const_cast(CRCI)->HandleCrash(); + const_cast(CRCI)->HandleCrash(Signal, nullptr); } static void installExceptionOrSignalHandlers() { @@ -345,7 +373,7 @@ bool CrashRecoveryContext::RunSafely(function_ref Fn) { // If crash recovery is disabled, do nothing. - if (gCrashRecoveryEnabled) { + if (IsEnabled()) { assert(!Impl && "Crash recovery context already initialized!"); CrashRecoveryContextImpl *CRCI = new CrashRecoveryContextImpl(this); Impl = CRCI; @@ -364,7 +392,7 @@ void CrashRecoveryContext::HandleCrash() { CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl; assert(CRCI && "Crash recovery context never initialized!"); - CRCI->HandleCrash(); + CRCI->HandleCrash(-1, nullptr); } // FIXME: Portability. Index: llvm/lib/Support/Unix/Signals.inc =================================================================== --- llvm/lib/Support/Unix/Signals.inc +++ llvm/lib/Support/Unix/Signals.inc @@ -345,6 +345,17 @@ FileToRemoveList::removeAllFiles(FilesToRemove); } +signed sys::InvokeExceptionHandler(int &RetCode, void *) { + SignalHandler(RetCode); + // llvm/lib/Support/Unix/Program.inc:Wait() returns -2 if a crash occurs, + // not the actual error code. If we want to diagnose a crash in the same + // way as invoking/forking a new process (in + // clang/tools/driver/driver.cpp), we need to do this here. + if (WIFSIGNALED(RetCode)) + RetCode = -2; + return 0; +} + // The signal handler that runs. static RETSIGTYPE SignalHandler(int Sig) { // Restore the signal behavior to default, so that the program actually @@ -361,8 +372,8 @@ { RemoveFilesToRemove(); - if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) - != std::end(IntSigs)) { + if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig) != + std::end(IntSigs)) { if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr)) return OldInterruptFunction(); @@ -371,9 +382,9 @@ OneShotPipeSignalFunction.exchange(nullptr)) return OldOneShotPipeFunction(); - raise(Sig); // Execute the default handler. + raise(Sig); // Execute the default handler. return; - } + } } // Otherwise if it is a fault (like SEGV) run any handler. 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(); } +signed sys::InvokeExceptionHandler(int &, void *ExceptionInfo) { + return LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)ExceptionInfo); +} + 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) {