[Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

[aganea]

Summary

As reported by @arichardson , this patch now makes llvm::report_fatal_error() to generate a preprocessed source + reproducer script again. Tested with/without the cmake flag CLANG_SPAWN_CC1. Tested in various configs, Debug/Release, with MSVC 19.24 or clang-cl 9.0.1, on Windows and Linux (with both gcc 9.2.1 and clang 9.0).

Added a test for #pragma clang __debug llvm_fatal_error to test for the original issue.
Added llvm::sys::Process::Exit() and replaced exit() in places where it was appropriate. This new function would call the current CrashRecoveryContext if one is running on the same thread; or call exit() otherwise.

This fixes PR44705.

[hans]

Evidently I can cut the patch in smaller pieces, let me know.

Yes, I think this would be good. There's a lot going on in this patch, and it would be good to separate the simple stuff from the tricky parts.

[aganea]

In D73742#1854773, @hans wrote:

Evidently I can cut the patch in smaller pieces, let me know.

Yes, I think this would be good. There's a lot going on in this patch, and it would be good to separate the simple stuff from the tricky parts.

@hans It was only so you can have the big picture. Does it make sense overall?

[hans]

I think so, but I need to look closer at the CrashRecoveryContext changes, and it would help to do that separately.

[aganea]

In D73742#1854810, @hans wrote:

I think so, but I need to look closer at the CrashRecoveryContext changes, and it would help to do that separately.

I split this into smaller pieces. Please take a look at D74063, D74070, D74076 and D74078.

[arichardson]

clang/tools/driver/cc1_main.cpp

73

I don't think it matters (yet?) but we should probably also use llvm::sys::Process::Exit() in cc1as_main.cpp?

[aganea]

Simplified patch after landing previous incremental changes mentioned above.

@arichardson If you have the chance, could you please apply this, see if that fixes your use-case?

[aganea]

Minor update -- added support & tested the VEH codepath as well.

clang/tools/driver/cc1_main.cpp

73

Changed cc1as_main.cpp as well.

[hans]

Looks good to me, just a nit about one of the comments.

Thanks for making this easier to review by splitting it up!

llvm/include/llvm/Support/Process.h

205

Could the comment be phrased in terms of CrashRecoveryContext instead of integrated-cc1, which is a Clang thing?

[aganea]

Side-note: Perhaps we would need to do the same as this patch, for calls to ::abort()?

llvm/include/llvm/Support/Process.h

205

Done in the commit.

[smeenai]

I'm assuming this needs to be picked to 10.0?

[aganea]

In D73742#1870961, @smeenai wrote:

I'm assuming this needs to be picked to 10.0?

Yes! Is it up to the authors to integrate their patches to the release branch? I'm seeing @hans is merging a lot of the patches.

[smeenai]

In D73742#1871012, @aganea wrote:

In D73742#1870961, @smeenai wrote:

I'm assuming this needs to be picked to 10.0?

Yes! Is it up to the authors to integrate their patches to the release branch? I'm seeing @hans is merging a lot of the patches.

I believe @hans takes care of the merging; I just wanted to make sure this hadn't been missed :) I'm gonna assume this is on his radar.

[hans]

In D73742#1871037, @smeenai wrote:

In D73742#1871012, @aganea wrote:

In D73742#1870961, @smeenai wrote:

I'm assuming this needs to be picked to 10.0?

Yes! Is it up to the authors to integrate their patches to the release branch? I'm seeing @hans is merging a lot of the patches.

I believe @hans takes care of the merging; I just wanted to make sure this hadn't been missed :) I'm gonna assume this is on his radar.

Yup :) Pushed to 10.x as fd04cb43e1d83c6f18c932de94c1e341272ed160

clang/test/Driver/crash-report.c

 // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
 
 // RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1                  \
 // RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1                         \
 // RUN:  not %clang %s @%t.rsp -DCRASH 2>&1 | FileCheck %s
 // RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
 // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
 
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1                  \
+// RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1                         \
+// RUN:  not %clang %s @%t.rsp -DFATAL 2>&1 | FileCheck %s
+// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
+// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
+
 // REQUIRES: crash-recovery
 
 #ifdef PARSER
 #pragma clang __debug parser_crash
 #elif CRASH
 #pragma clang __debug crash
+#elif FATAL
+#pragma clang __debug llvm_fatal_error
 #endif
 
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
 // CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-{{.*}}.c
 FOO
 // CHECKSRC: FOO
 // CHECKSH: # Crash reproducer
 // CHECKSH-NEXT: # Driver args: {{.*}}"-fsyntax-only"

clang/tools/driver/cc1_main.cpp

 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include <cstdio>
static void LLVMErrorHandler(void *UserData, const std::string &Message,
 
   // Run the interrupt handlers to make sure any special cleanups get done, in
   // particular that we remove files registered with RemoveFileOnSignal.
   llvm::sys::RunInterruptHandlers();
 
   // We cannot recover from llvm errors.  When reporting a fatal error, exit
   // with status 70 to generate crash diagnostics.  For BSD systems this is
   // defined as an internal software error.  Otherwise, exit with status 1.
-  exit(GenCrashDiag ? 70 : 1);
+  llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1);
 }
 
 #ifdef CLANG_HAVE_RLIMITS
 #if defined(__linux__) && defined(__PIE__)
 static size_t getCurrentStackAllocation() {
   // If we can't compute the current stack usage, allow for 512K of command
   // line arguments and environment.
   size_t Usage = 512 * 1024;

clang/tools/driver/cc1as_main.cpp

 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 #include <system_error>

 
 static void LLVMErrorHandler(void *UserData, const std::string &Message,
                              bool GenCrashDiag) {
   DiagnosticsEngine &Diags = *static_cast<DiagnosticsEngine*>(UserData);
 
   Diags.Report(diag::err_fe_error_backend) << Message;
 
   // We cannot recover from llvm errors.
-  exit(1);
+  sys::Process::Exit(1);
 }
 
 int cc1as_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
   // Initialize targets and assembly printers/parsers.
   InitializeAllTargetInfos();
   InitializeAllTargetMCs();
   InitializeAllAsmParsers();
 

llvm/include/llvm/Support/CrashRecoveryContext.h

public:
   /// On Darwin, if PRIO_DARWIN_BG is set on the calling thread, it will be
   /// propagated to the new thread as well.
   bool RunSafelyOnThread(function_ref<void()>, unsigned RequestedStackSize = 0);
   bool RunSafelyOnThread(void (*Fn)(void*), void *UserData,
                          unsigned RequestedStackSize = 0) {
     return RunSafelyOnThread([&]() { Fn(UserData); }, RequestedStackSize);
   }
 
+  /// Explicitly trigger a crash recovery in the current process, and
+  /// return failure from RunSafely(). This function does not return.
+  LLVM_ATTRIBUTE_NORETURN
+  void HandleExit(int RetCode);
+
   /// 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;
 };

llvm/include/llvm/Support/Process.h

public:
   static const char *OutputReverse();
 
   /// Resets the terminals colors, or returns an escape sequence to do so.
   static const char *ResetColor();
 
   /// Get the result of a process wide random number generator. The
   /// generator will be automatically seeded in non-deterministic fashion.
   static unsigned GetRandomNumber();
+
+  /// Equivalent to ::exit(), except when running inside a CrashRecoveryContext.
+  /// In that case, the control flow will resume after RunSafely(), like for a
+  /// crash, rather than exiting the current process.
+  LLVM_ATTRIBUTE_NORETURN
+  static void Exit(int RetCode);
 };
 
 }
 }
 
 #endif

llvm/lib/Support/CrashRecoveryContext.cpp

 //===--- CrashRecoveryContext.cpp - Crash Recovery ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/CrashRecoveryContext.h"
 #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 <windows.h> // for GetExceptionInformation
-#endif
 #if LLVM_ON_UNIX
 #include <sysexits.h> // EX_IOERR
 #endif
 
 using namespace llvm;
 
 namespace {
 
else {
     cleanup->prev->next = cleanup->next;
     if (cleanup->next)
       cleanup->next->prev = cleanup->prev;
   }
   delete cleanup;
 }
 
 #if defined(_MSC_VER)
+
+#include <windows.h> // for GetExceptionInformation
+
 // If _MSC_VER is defined, we must have SEH. Use it if it's available. It's way
 // better than VEH. Vectored exception handling catches all exceptions happening
 // on the thread with installed exception handlers, so it can interfere with
 // internal exception handling of other libraries on that thread. SEH works
 // exactly as you would expect normal exception handling to work: it only
 // catches exceptions if they would bubble out from the stack frame with __try /
 // __except.
 
static int ExceptionFilter(_EXCEPTION_POINTERS *Except) {
   if (!CRCI) {
     // Something has gone horribly wrong, so let's just tell everyone
     // to keep searching
     CrashRecoveryContext::Disable();
     return EXCEPTION_CONTINUE_SEARCH;
   }
 
   int RetCode = (int)Except->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF0000000) == 0xE0000000)
+    RetCode &= ~0xF0000000; // this crash was generated by sys::Process::Exit
 
   // Handle the crash
   const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(
       RetCode, reinterpret_cast<uintptr_t>(Except));
 
   return EXCEPTION_EXECUTE_HANDLER;
 }
 
if (!CRCI) {
     // to keep searching
     CrashRecoveryContext::Disable();
     return EXCEPTION_CONTINUE_SEARCH;
   }
 
   // TODO: We can capture the stack backtrace here and store it on the
   // implementation if we so choose.
 
+  int RetCode = (int)ExceptionInfo->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF0000000) == 0xE0000000)
+    RetCode &= ~0xF0000000; // this crash was generated by sys::Process::Exit
+
   // Handle the crash
   const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(
-      (int)ExceptionInfo->ExceptionRecord->ExceptionCode,
-      reinterpret_cast<uintptr_t>(ExceptionInfo));
+      RetCode, reinterpret_cast<uintptr_t>(ExceptionInfo));
 
   // Note that we don't actually get here because HandleCrash calls
   // longjmp, which means the HandleCrash function never returns.
   llvm_unreachable("Handled the crash, should have longjmp'ed out of here");
 }
 
 // Because the Enable and Disable calls are static, it means that
 // there may not actually be an Impl available, or even a current
bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
   }
 
   Fn();
   return true;
 }
 
 #endif // !_MSC_VER
 
+LLVM_ATTRIBUTE_NORETURN
+void CrashRecoveryContext::HandleExit(int RetCode) {
+#if defined(_WIN32)
+  // SEH and VEH
+  ::RaiseException(0xE0000000 | RetCode, 0, 0, NULL);
+#else
+  // On Unix we don't need to raise an exception, we go directly to
+  // HandleCrash(), then longjmp will unwind the stack for us.
+  CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *)Impl;
+  assert(CRCI && "Crash recovery context never initialized!");
+  CRCI->HandleCrash(RetCode, 0 /*no sig num*/);
+#endif
+  llvm_unreachable("Most likely setjmp wasn't called!");
+}
+
 // FIXME: Portability.
 static void setThreadBackgroundPriority() {
 #ifdef __APPLE__
   setpriority(PRIO_DARWIN_THREAD, 0, PRIO_DARWIN_BG);
 #endif
 }
 
 static bool hasThreadBackgroundPriority() {

llvm/lib/Support/ErrorHandling.cpp

 #include "llvm/Support/ErrorHandling.h"
 #include "llvm-c/ErrorHandling.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/WindowsError.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
 #include <mutex>
 #include <new>
if (handler) {
     (void)written; // If something went wrong, we deliberately just give up.
   }
 
   // If we reached here, we are failing ungracefully. Run the interrupt handlers
   // to make sure any special cleanups get done, in particular that we remove
   // files registered with RemoveFileOnSignal.
   sys::RunInterruptHandlers();
 
-  exit(1);
+  sys::Process::Exit(1);
 }
 
 void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler,
                                            void *user_data) {
 #if LLVM_ENABLE_THREADS == 1
   std::lock_guard<std::mutex> Lock(BadAllocErrorHandlerMutex);
 #endif
   assert(!ErrorHandler && "Bad alloc error handler already registered!\n");

llvm/lib/Support/Process.cpp

 //===-- Process.cpp - Implement OS Process Concept --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the operating system Process concept.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Process.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/Config/llvm-config.h"
 #include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 
 using namespace llvm;
 using namespace sys;
 
 //===----------------------------------------------------------------------===//

 };
 
 // A CMake option controls wheter we emit core dumps by default. An application
 // may disable core dumps by calling Process::PreventCoreFiles().
 static bool coreFilesPrevented = !LLVM_ENABLE_CRASH_DUMPS;
 
 bool Process::AreCoreFilesPrevented() { return coreFilesPrevented; }
 
+LLVM_ATTRIBUTE_NORETURN
+void Process::Exit(int RetCode) {
+  if (CrashRecoveryContext *CRC = CrashRecoveryContext::GetCurrent())
+    CRC->HandleExit(RetCode);
+  ::exit(RetCode);
+}
+
 // Include the platform-specific parts of this class.
 #ifdef LLVM_ON_UNIX
 #include "Unix/Process.inc"
 #endif
 #ifdef _WIN32
 #include "Windows/Process.inc"
 #endif