aganea updated this revision to Diff 229333.
aganea marked an inline comment as done.
aganea added a comment.

Call into `ExecuteCC1Tool()` directly, as suggested by @hans
Add more comments.
Remove `pThis` in `CrashRecoveryContext.cpp:RunSafely()` as suggested by 
@zturner


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69825/new/

https://reviews.llvm.org/D69825

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===================================================================
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -187,7 +187,7 @@
 using namespace llvm;
 
 // Forward declare.
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
 // The function to call if ctrl-c is pressed.
@@ -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();
@@ -785,7 +791,7 @@
   return std::error_code();
 }
 
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
   // We'll automatically write a Minidump file here to help diagnose
@@ -803,42 +809,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/lib/Support/Unix/Signals.inc
===================================================================
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -79,7 +79,7 @@
 
 using namespace llvm;
 
-static RETSIGTYPE SignalHandler(int Sig);  // defined below.
+RETSIGTYPE SignalHandler(int Sig);  // defined below.
 static RETSIGTYPE InfoSignalHandler(int Sig);  // defined below.
 
 using SignalHandlerFunctionType = void (*)();
@@ -341,7 +341,7 @@
 }
 
 // The signal handler that runs.
-static RETSIGTYPE SignalHandler(int Sig) {
+RETSIGTYPE SignalHandler(int Sig) {
   // Restore the signal behavior to default, so that the program actually
   // 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
@@ -356,8 +356,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();
 
@@ -365,9 +365,9 @@
       if (Sig == SIGPIPE)
         exit(EX_IOERR);
 
-      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/CrashRecoveryContext.cpp
===================================================================
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -13,8 +13,20 @@
 #include "llvm/Support/ThreadLocal.h"
 #include <mutex>
 #include <setjmp.h>
+#ifdef _WIN32
+#include <windows.h>
+#endif
 using namespace llvm;
 
+#ifdef _WIN32
+// In llvm/lib/Support/Windows/Signals.inc
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+#endif
+#ifdef LLVM_ON_UNIX
+// In llvm/lib/Support/Unix/Signals.inc
+void SignalHandler(int Sig);
+#endif
+
 namespace {
 
 struct CrashRecoveryContextImpl;
@@ -54,7 +66,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 +74,20 @@
     assert(!Failed && "Crash recovery context already failed!");
     Failed = true;
 
-    // FIXME: Stash the backtrace.
+    if (CRC->EnableExceptionHandler) {
+#ifdef _WIN32
+      LLVMUnhandledExceptionFilter((LPEXCEPTION_POINTERS)ExceptionInfo);
+#endif
+#ifdef LLVM_ON_UNIX
+      SignalHandler(RetCode);
+      // llvm/lib/Support/Unix/Program.inc:Wait() returns -2 if a crash occurs,
+      // not the actual error code.
+      if (WIFSIGNALED(RetCode))
+        RetCode = -2;
+#endif
+    }
+
+    CRC->RetCode = RetCode;
 
     // Jump back to the RunSafely we were called under.
     longjmp(JumpBuffer, 1);
@@ -171,19 +196,25 @@
 static void installExceptionOrSignalHandlers() {}
 static void uninstallExceptionOrSignalHandlers() {}
 
-bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
-  if (!gCrashRecoveryEnabled) {
+static bool InvokeFunctionCall(function_ref<void()> Fn, bool ExceptionHandler,
+                               int &RetCode) {
+  __try {
     Fn();
-    return true;
+  } __except (ExceptionHandler
+                  ? LLVMUnhandledExceptionFilter(GetExceptionInformation())
+                  : 1) {
+    RetCode = GetExceptionCode();
+    return false;
   }
+  return true;
+}
 
-  bool Result = true;
-  __try {
+bool CrashRecoveryContext::RunSafely(function_ref<void()> 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 +268,8 @@
   // 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.
@@ -320,7 +352,7 @@
   sigprocmask(SIG_UNBLOCK, &SigMask, nullptr);
 
   if (CRCI)
-    const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash();
+    const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(Signal, nullptr);
 }
 
 static void installExceptionOrSignalHandlers() {
@@ -364,7 +396,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/include/llvm/Support/CrashRecoveryContext.h
===================================================================
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -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{};
+
+  /// 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{};
 };
 
 /// Abstract base class of cleanup handlers.
Index: clang/tools/driver/driver.cpp
===================================================================
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -30,6 +30,7 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
@@ -303,7 +304,11 @@
     TheDriver.setInstalledDir(InstalledPathParent);
 }
 
-static int ExecuteCC1Tool(ArrayRef<const char *> argv, StringRef Tool) {
+int ExecuteCC1Tool(ArrayRef<const char *> argv) {
+  // If we re-enter the cc1 tool from the driver process, we should cleanup the
+  // usage count for the driver options (which might be used in the cc1 tool)
+  llvm::cl::ResetAllOptionOccurrences();
+  StringRef Tool = argv[1] + 4;
   void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath;
   if (Tool == "")
     return cc1_main(argv.slice(2), argv[0], GetExecutablePathVP);
@@ -327,6 +332,13 @@
     return 1;
 
   llvm::InitializeAllTargets();
+
+  // When calling any clang driver other than -cc1, we optimize the
+  // clangDriver lib to call back into this function, instead of starting a new
+  // clang.exe. This saves a huge amount of time of Windows, as process creation
+  // can be expensive on that platform.
+  Driver::CC1Main = &ExecuteCC1Tool;
+
   auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
 
   llvm::BumpPtrAllocator A;
@@ -379,7 +391,7 @@
       auto newEnd = std::remove(argv.begin(), argv.end(), nullptr);
       argv.resize(newEnd - argv.begin());
     }
-    return ExecuteCC1Tool(argv, argv[1] + 4);
+    return ExecuteCC1Tool(argv);
   }
 
   bool CanonicalPrefixes = true;
@@ -503,7 +515,7 @@
 
 #ifdef _WIN32
   // Exit status should not be negative on Win32, unless abnormal termination.
-  // Once abnormal termiation was caught, negative status should not be
+  // Once abnormal termination was caught, negative status should not be
   // propagated.
   if (Res < 0)
     Res = 1;
Index: clang/lib/Driver/Job.cpp
===================================================================
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -19,8 +19,10 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
@@ -313,6 +315,8 @@
   Environment.push_back(nullptr);
 }
 
+LLVM_THREAD_LOCAL bool Command::ExecuteCC1Tool = true;
+
 int Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
                      std::string *ErrMsg, bool *ExecutionFailed) const {
   if (PrintInputFilenames) {
@@ -321,7 +325,7 @@
     llvm::outs().flush();
   }
 
-  SmallVector<const char*, 128> Argv;
+  SmallVector<const char *, 128> Argv;
 
   Optional<ArrayRef<StringRef>> Env;
   std::vector<StringRef> ArgvVectorStorage;
@@ -332,42 +336,79 @@
     Env = makeArrayRef(ArgvVectorStorage);
   }
 
-  if (ResponseFile == nullptr) {
+  Driver::CC1ToolFunc CC1Main{};
+
+  // If we can re-use the same process, we reenter a variant of main() through a
+  // callback. This is an optimization for speed on Windows.
+  if (ExecuteCC1Tool) {
+    const Driver &D = getCreator().getToolChain().getDriver();
+    StringRef CommandExe = llvm::sys::path::stem(Executable);
+    StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+    if (CommandExe.equals_lower(DriverExe))
+        CC1Main = Driver::CC1Main;
+  }
+
+  if (ResponseFile == nullptr || CC1Main) {
     Argv.push_back(Executable);
     Argv.append(Arguments.begin(), Arguments.end());
-    Argv.push_back(nullptr);
-
-    auto Args = llvm::toStringRefArray(Argv.data());
-    return llvm::sys::ExecuteAndWait(
-        Executable, Args, Env, Redirects, /*secondsToWait*/ 0,
-        /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
+    if (!CC1Main)
+      Argv.push_back(nullptr);
+  } else {
+    // If the command is too large, we need to put arguments in a response file.
+    std::string RespContents;
+    llvm::raw_string_ostream SS(RespContents);
+
+    // Write file contents and build the Argv vector
+    writeResponseFile(SS);
+    buildArgvForResponseFile(Argv);
+    if (!CC1Main)
+      Argv.push_back(nullptr);
+    SS.flush();
+
+    // Save the response file in the appropriate encoding
+    if (std::error_code EC = writeFileWithEncoding(
+            ResponseFile, RespContents, Creator.getResponseFileEncoding())) {
+      if (ErrMsg)
+        *ErrMsg = EC.message();
+      if (ExecutionFailed)
+        *ExecutionFailed = true;
+      return -1;
+    }
   }
 
-  // We need to put arguments in a response file (command is too large)
-  // Open stream to store the response file contents
-  std::string RespContents;
-  llvm::raw_string_ostream SS(RespContents);
-
-  // Write file contents and build the Argv vector
-  writeResponseFile(SS);
-  buildArgvForResponseFile(Argv);
-  Argv.push_back(nullptr);
-  SS.flush();
-
-  // Save the response file in the appropriate encoding
-  if (std::error_code EC = writeFileWithEncoding(
-          ResponseFile, RespContents, Creator.getResponseFileEncoding())) {
-    if (ErrMsg)
-      *ErrMsg = EC.message();
+  if (CC1Main) {
+    // This simply indicates that the program couldn't start, which isn't
+    // applicable here.
     if (ExecutionFailed)
-      *ExecutionFailed = true;
-    return -1;
-  }
+      *ExecutionFailed = false;
 
-  auto Args = llvm::toStringRefArray(Argv.data());
-  return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects,
-                                   /*secondsToWait*/ 0,
-                                   /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
+    static bool CRCEnabled{};
+    if (!CRCEnabled) {
+      llvm::CrashRecoveryContext::Enable();
+      CRCEnabled = true;
+    }
+
+    llvm::CrashRecoveryContext CRC;
+    CRC.EnableExceptionHandler = true;
+
+    const void *PrettyState = llvm::SavePrettyStackState();
+
+    int Ret = 0;
+    auto ExecuteCC1Main = [&]() { Ret = CC1Main(Argv); };
+
+    // Reenter a variant of main() instead of starting up a new process
+    if (!CRC.RunSafely(ExecuteCC1Main)) {
+      llvm::RestorePrettyStackState(PrettyState);
+      return CRC.RetCode;
+    }
+    return Ret;
+  } else {
+    auto Args = llvm::toStringRefArray(Argv.data());
+    return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects,
+                                     /*secondsToWait*/ 0,
+                                     /*memoryLimit*/ 0, ErrMsg,
+                                     ExecutionFailed);
+  }
 }
 
 FallbackCommand::FallbackCommand(const Action &Source_, const Tool &Creator_,
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -91,6 +91,8 @@
 using namespace clang;
 using namespace llvm::opt;
 
+LLVM_THREAD_LOCAL Driver::CC1ToolFunc Driver::CC1Main;
+
 // static
 std::string Driver::GetResourcesPath(StringRef BinaryPath,
                                      StringRef CustomResourceDir) {
@@ -1333,6 +1335,11 @@
 
   BuildJobs(C);
 
+  // Ensure we don't re-enter this process, and instead call a new clang.exe for
+  // rendering the preprocessed output. This is to minimize the chances of a
+  // secondary crash (in case this function was called due to an exception)
+  Command::ExecuteCC1Tool = false;
+
   // If there were errors building the compilation, quit now.
   if (Trap.hasErrorOccurred()) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
Index: clang/include/clang/Driver/Job.h
===================================================================
--- clang/include/clang/Driver/Job.h
+++ clang/include/clang/Driver/Job.h
@@ -130,6 +130,10 @@
 
   /// Set whether to print the input filenames when executing.
   void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
+
+  /// When set, we will try executing the CC1 tool in the current process,
+  /// instead of creating a new process. This is an optimization for speed.
+  static LLVM_THREAD_LOCAL bool ExecuteCC1Tool;
 };
 
 /// Like Command, but with a fallback which is executed in case
Index: clang/include/clang/Driver/Driver.h
===================================================================
--- clang/include/clang/Driver/Driver.h
+++ clang/include/clang/Driver/Driver.h
@@ -204,6 +204,10 @@
   /// Whether the driver is generating diagnostics for debugging purposes.
   unsigned CCGenDiagnostics : 1;
 
+  /// Callback to the CC1 tool, if available
+  typedef int(*CC1ToolFunc)(ArrayRef<const char *> argv);
+  static LLVM_THREAD_LOCAL CC1ToolFunc CC1Main;
+
 private:
   /// Raw target triple.
   std::string TargetTriple;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to