[Support] Optionally call signal handlers when a function wrapped by the the CrashRecoveryContext fails

This patch allows for handling a failure inside a CrashRecoveryContext in the same way as the global exception/signal handler. A failure will have the same side-effect, such as cleanup of temporarty file, printing callstack, calling relevant signal handlers, and finally returning an exception code. This is an optional feature, disabled by default.
This is a support patch for D69825.

Differential Revision: https://reviews.llvm.org/D70568

include/llvm/Support/CrashRecoveryContext.h

@@ -100,6 +100,14 @@ public:
   /// 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 failures should be done in the same way as
+  /// for regular crashes. 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.

include/llvm/Support/Signals.h

@@ -106,6 +106,15 @@ namespace sys {
   /// 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
+  /// Context is a system-specific failure context: it is the signal type on
+  /// Unix; the ExceptionContext on Windows.
+  void CleanupOnSignal(uintptr_t Context);
 } // End sys namespace
 } // End llvm namespace
 

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 <mutex>
 #include <setjmp.h>
+#ifdef _WIN32
+#include <excpt.h> // for GetExceptionInformation
+#endif
+#if LLVM_ON_UNIX
+#include <sysexits.h> // EX_IOERR
+#endif
+
 using namespace llvm;
 
 namespace {
@@ -54,7 +62,11 @@ public:
 #endif
   }
 
-  void HandleCrash() {
+  // If the function ran by the CrashRecoveryContext crashes or fails, then
+  // 'RetCode' represents the returned error code, as if it was returned by a
+  // process. 'Context' represents the signal type on Unix; on Windows, it is
+  // the ExceptionContext.
+  void HandleCrash(int RetCode, uintptr_t Context) {
     // Eliminate the current context entry, to avoid re-entering in case the
     // cleanup code crashes.
     CurrentContext->set(Next);
@@ -62,7 +74,10 @@ public:
     assert(!Failed && "Crash recovery context already failed!");
     Failed = true;
 
-    // FIXME: Stash the backtrace.
+    if (CRC->DumpStackAndCleanupOnFailure)
+      sys::CleanupOnSignal(Context);
+
+    CRC->RetCode = RetCode;
 
     // Jump back to the RunSafely we were called under.
     longjmp(JumpBuffer, 1);
@@ -171,19 +186,32 @@ CrashRecoveryContext::unregisterCleanup(CrashRecoveryContextCleanup *cleanup) {
 static void installExceptionOrSignalHandlers() {}
 static void uninstallExceptionOrSignalHandlers() {}
 
+// We need this function because the call to GetExceptionInformation() can only
+// occur inside the __except evaluation block
+static int ExceptionFilter(bool DumpStackAndCleanup,
+                           _EXCEPTION_POINTERS *Except) {
+  if (DumpStackAndCleanup)
+    sys::CleanupOnSignal((uintptr_t)Except);
+  return EXCEPTION_EXECUTE_HANDLER;
+}
+
+static bool InvokeFunctionCall(function_ref<void()> Fn,
+                               bool DumpStackAndCleanup, int &RetCode) {
+  __try {
+    Fn();
+  } __except (ExceptionFilter(DumpStackAndCleanup, GetExceptionInformation())) {
+    RetCode = GetExceptionCode();
+    return false;
+  }
+  return true;
+}
+
 bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
   if (!gCrashRecoveryEnabled) {
     Fn();
     return true;
   }
-
-  bool Result = true;
-  __try {
-    Fn();
-  } __except (1) { // Catch any exception.
-    Result = false;
-  }
-  return Result;
+  return InvokeFunctionCall(Fn, DumpStackAndCleanupOnFailure, RetCode);
 }
 
 #else // !_MSC_VER
@@ -237,7 +265,8 @@ static LONG CALLBACK ExceptionHandler(PEXCEPTION_POINTERS ExceptionInfo)
   // implementation if we so choose.
 
   // Handle the crash
-  const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash();
+  const_cast<CrashRecoveryContextImpl *>(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.
@@ -319,8 +348,16 @@ 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;
+
+  // 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<CrashRecoveryContextImpl*>(CRCI)->HandleCrash();
+    const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(RetCode, Signal);
 }
 
 static void installExceptionOrSignalHandlers() {
@@ -364,7 +401,9 @@ bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
 void CrashRecoveryContext::HandleCrash() {
   CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl;
   assert(CRCI && "Crash recovery context never initialized!");
-  CRCI->HandleCrash();
+  // As per convention, -2 indicates a crash or timeout as opposed to failure to
+  // execute (see llvm/include/llvm/Support/Program.h)
+  CRCI->HandleCrash(-2, 0);
 }
 
 // FIXME: Portability.

lib/Support/Unix/Signals.inc

@@ -345,6 +345,22 @@ static void RemoveFilesToRemove() {
   FileToRemoveList::removeAllFiles(FilesToRemove);
 }
 
+void sys::CleanupOnSignal(uintptr_t Context) {
+  int Sig = (int)Context;
+
+  if (llvm::is_contained(InfoSigs, Sig)) {
+    InfoSignalHandler(Sig);
+    return;
+  }
+
+  RemoveFilesToRemove();
+
+  if (llvm::is_contained(IntSigs, Sig) || Sig == SIGPIPE)
+    return;
+
+  llvm::sys::RunSignalHandlers();
+}
+
 // The signal handler that runs.
 static RETSIGTYPE SignalHandler(int Sig) {
   // Restore the signal behavior to default, so that the program actually

lib/Support/Windows/Signals.inc

@@ -521,10 +521,13 @@ void sys::PrintStackTraceOnErrorSignal(StringRef Argv0,
 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 @@ void llvm::sys::PrintStackTrace(raw_ostream &OS) {
   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 @@ WriteWindowsDumpFile(PMINIDUMP_EXCEPTION_INFORMATION ExceptionInfo) {
   return std::error_code();
 }
 
+void sys::CleanupOnSignal(uintptr_t Context) {
+  LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)Context);
+}
+
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
@@ -810,42 +820,9 @@ static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
                    << "\n";
   }
 
-  // Initialize the STACKFRAME structure.
-  STACKFRAME64 StackFrame = {};
+  LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr);
 
-#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);
-
-  _exit(ep->ExceptionRecord->ExceptionCode);
+  return EXCEPTION_EXECUTE_HANDLER;
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {

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,7 @@ static int GlobalInt = 0;
 static void nullDeref() { *(volatile int *)0x10 = 0; }
 static void incrementGlobal() { ++GlobalInt; }
 static void llvmTrap() { LLVM_BUILTIN_TRAP; }
+static void incrementGlobalWithParam(void *) { ++GlobalInt; }
 
 TEST(CrashRecoveryTest, Basic) {
   llvm::CrashRecoveryContext::Enable();
@@ -58,6 +61,33 @@ TEST(CrashRecoveryTest, Cleanup) {
     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(incrementGlobalWithParam, 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();
 }
 
 #ifdef _WIN32