[Windows][LLVM] Elide PrettyStackTrace output for usage errors

bd1976bris · bd1976bris · commit 0d9218eda0b9 · 2025-05-21T19:50:09.000+01:00
LLVM’s `reportFatalUsageError`, introduced recently, may emit a stack trace and bug report prompt due to the `PrettyStackTrace` signal handler (initialized via `InitLLVM`). On Windows, this occurs when `sys::RunInterruptHandlers()` is invoked from `reportFatalUsageError`. This behavior is misleading for usage errors. For example, one of Sony’s customers reported a bug after specifying an invalid LTO cache directory - a clear usage error - because the toolchain output included a stack trace and instructions to file a bug report. This patch suppresses `PrettyStackTrace` output for usage errors by passing a flag to the signal handlers, indicating when the error should not trigger crash-style diagnostics. To test this, LTO cache directory errors have been updated to use `reportFatalUsageError` and the existing Linux-specific test has been replaced with a Windows-only test case that additionally verifies no crash-style diagnostics are emitted. LLVM Issue: #140953 Internal Tracker: TOOLCHAIN-17744
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
@@ -55,7 +55,7 @@ static void LLVMErrorHandler(void *UserData, const char *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();
+  llvm::sys::RunInterruptHandlers(/*IsUsageError=*/false);
 
   // We cannot recover from llvm errors.  When reporting a fatal error, exit
   // with status 70 to generate crash diagnostics.  For BSD systems this is
diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
@@ -72,7 +72,7 @@ static void LLVMErrorHandler(void *UserData, const char *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();
+  llvm::sys::RunInterruptHandlers(/*IsUsageError=*/false);
 
   // We cannot recover from llvm errors.  When reporting a fatal error, exit
   // with status 70 to generate crash diagnostics.  For BSD systems this is
diff --git a/lld/test/COFF/lto-cache-errors.ll b/lld/test/COFF/lto-cache-errors.ll
@@ -1,15 +1,24 @@
-; REQUIRES: x86, non-root-user
-;; Not supported on windows since we use permissions to deny the creation
-; UNSUPPORTED: system-windows
+;; The output is OS specific, so make this test specific to Windows.
+; REQUIRES: x86, system-windows
 
 ; RUN: opt -module-hash -module-summary %s -o %t.o
 ; RUN: opt -module-hash -module-summary %p/Inputs/lto-cache.ll -o %t2.o
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
-; RUN: chmod 444 %t.cache
 
-;; Check emit warnings when we can't create the cache dir
-; RUN: not --crash lld-link /lldltocache:%t.cache/nonexistant/ /out:%t3 /entry:main %t2.o %t.o 2>&1 | FileCheck %s
-; CHECK: LLVM ERROR: can't create cache directory {{.*}}/nonexistant/: Permission denied
+; Use a 261-character path component which filesystems will fail to create. Use
+; a response file to allow breaking up the long path into 50-character chunks.
+; RUN: echo -n "/lldltocache:%t.cache/" > %t_rsp
+; RUN: echo -n "01234567890123456789012345678901234567890123456789" >> %t_rsp
+; RUN: echo -n "01234567890123456789012345678901234567890123456789" >> %t_rsp
+; RUN: echo -n "01234567890123456789012345678901234567890123456789" >> %t_rsp
+; RUN: echo -n "01234567890123456789012345678901234567890123456789" >> %t_rsp
+; RUN: echo -n "01234567890123456789012345678901234567890123456789" >> %t_rsp
+; RUN: echo -n "01234567890/bob/" >> %t_rsp
+
+;; Check fatal usage error emitted when the cache dir can't be created.
+; RUN: not lld-link @%t_rsp /out:%t3 /entry:main %t2.o %t.o 2>&1 | FileCheck %s \
+; RUN:   --ignore-case --implicit-check-not=bug --implicit-check-not=crash
+; CHECK: LLVM ERROR: can't create cache directory {{.*}}/bob/: {{.*(invalid argument)|(too long)}}
 
 target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc"
diff --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -589,7 +589,7 @@ class PrintCrashIRInstrumentation {
   // The crash reporter that will report on a crash.
   static PrintCrashIRInstrumentation *CrashReporter;
   // Crash handler registered when print-on-crash is specified.
-  static void SignalHandler(void *);
+  static void SignalHandler(void *, bool);
 };
 
 /// This class provides an interface to register all the standard pass
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
@@ -742,7 +742,8 @@ template <class T> class [[nodiscard]] Expected {
 /// @deprecated Use reportFatalInternalError() or reportFatalUsageError()
 /// instead.
 [[noreturn]] LLVM_ABI void report_fatal_error(Error Err,
-                                              bool gen_crash_diag = true);
+                                              bool GenCrashDiag = true,
+                                              bool IsUsageError = false);
 
 /// Report a fatal error that indicates a bug in LLVM.
 /// See ErrorHandling.h for details.
diff --git a/llvm/include/llvm/Support/ErrorHandling.h b/llvm/include/llvm/Support/ErrorHandling.h
@@ -61,11 +61,14 @@ struct ScopedFatalErrorHandler {
 /// @deprecated Use reportFatalInternalError() or reportFatalUsageError()
 /// instead.
 [[noreturn]] LLVM_ABI void report_fatal_error(const char *reason,
-                                              bool gen_crash_diag = true);
+                                              bool gen_crash_diag = true,
+                                              bool is_usage_error = false);
 [[noreturn]] LLVM_ABI void report_fatal_error(StringRef reason,
-                                              bool gen_crash_diag = true);
+                                              bool gen_crash_diag = true,
+                                              bool is_usage_error = false);
 [[noreturn]] LLVM_ABI void report_fatal_error(const Twine &reason,
-                                              bool gen_crash_diag = true);
+                                              bool gen_crash_diag = true,
+                                              bool is_usage_error = false);
 
 /// Report a fatal error that likely indicates a bug in LLVM. It serves a
 /// similar purpose as an assertion, but is always enabled, regardless of the
diff --git a/llvm/include/llvm/Support/Signals.h b/llvm/include/llvm/Support/Signals.h
@@ -26,7 +26,7 @@ namespace sys {
 
 /// This function runs all the registered interrupt handlers, including the
 /// removal of files registered by RemoveFileOnSignal.
-LLVM_ABI void RunInterruptHandlers();
+LLVM_ABI void RunInterruptHandlers(bool IsUsageError);
 
 /// This function registers signal handlers to ensure that if a signal gets
 /// delivered that the named file is removed.
@@ -58,9 +58,9 @@ LLVM_ABI void DisableSystemDialogsOnCrash();
 LLVM_ABI void PrintStackTrace(raw_ostream &OS, int Depth = 0);
 
 // Run all registered signal handlers.
-LLVM_ABI void RunSignalHandlers();
+LLVM_ABI void RunSignalHandlers(bool IsUsageError);
 
-using SignalHandlerCallback = void (*)(void *);
+using SignalHandlerCallback = void (*)(void *, bool);
 
 /// Add a function to be called when an abort/kill signal is delivered to the
 /// process. The handler can have a cookie passed to it to identify what
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
@@ -435,7 +435,7 @@ static void codegen(const Config &Conf, TargetMachine *TM,
   Expected<std::unique_ptr<CachedFileStream>> StreamOrErr =
       AddStream(Task, Mod.getModuleIdentifier());
   if (Error Err = StreamOrErr.takeError())
-    report_fatal_error(std::move(Err));
+    reportFatalUsageError(std::move(Err));
   std::unique_ptr<CachedFileStream> &Stream = *StreamOrErr;
   TM->Options.ObjectFilenameForDebug = Stream->ObjectPathName;
 
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -2468,7 +2468,7 @@ void PrintCrashIRInstrumentation::reportCrashIR() {
   }
 }
 
-void PrintCrashIRInstrumentation::SignalHandler(void *) {
+void PrintCrashIRInstrumentation::SignalHandler(void *, bool) {
   // Called by signal handlers so do not lock here
   // Is the PrintCrashIRInstrumentation still alive?
   if (!CrashReporter)
diff --git a/llvm/lib/Support/Debug.cpp b/llvm/lib/Support/Debug.cpp
@@ -148,7 +148,7 @@ void llvm::initDebugOptions() {
 }
 
 // Signal handlers - dump debug output on termination.
-static void debug_user_sig_handler(void *Cookie) {
+static void debug_user_sig_handler(void *Cookie, bool /*IsUsageError*/) {
   // This is a bit sneaky.  Since this is under #ifndef NDEBUG, we
   // know that debug mode is enabled and dbgs() really is a
   // circular_raw_ostream.  If NDEBUG is defined, then dbgs() ==
diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
@@ -164,21 +164,22 @@ Error createStringError(std::string &&Msg, std::error_code EC) {
   return make_error<StringError>(Msg, EC);
 }
 
-void report_fatal_error(Error Err, bool GenCrashDiag) {
+void report_fatal_error(Error Err, bool GenCrashDiag, bool IsUsageError) {
   assert(Err && "report_fatal_error called with success value");
   std::string ErrMsg;
   {
     raw_string_ostream ErrStream(ErrMsg);
     logAllUnhandledErrors(std::move(Err), ErrStream);
   }
-  report_fatal_error(Twine(ErrMsg), GenCrashDiag);
+  report_fatal_error(Twine(ErrMsg), GenCrashDiag, IsUsageError);
 }
 
 void reportFatalInternalError(Error Err) {
-  report_fatal_error(std::move(Err), /*GenCrashDiag=*/true);
+  report_fatal_error(std::move(Err), /*GenCrashDiag=*/true, /*IsUsageError=*/false);
 }
+
 void reportFatalUsageError(Error Err) {
-  report_fatal_error(std::move(Err), /*GenCrashDiag=*/false);
+  report_fatal_error(std::move(Err), /*GenCrashDiag=*/false, /*IsUsageError=*/true);
 }
 
 } // end namespace llvm
diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp
@@ -80,15 +80,15 @@ void llvm::remove_fatal_error_handler() {
   ErrorHandlerUserData = nullptr;
 }
 
-void llvm::report_fatal_error(const char *Reason, bool GenCrashDiag) {
-  report_fatal_error(Twine(Reason), GenCrashDiag);
+void llvm::report_fatal_error(const char *Reason, bool GenCrashDiag, bool IsUsageError) {
+  report_fatal_error(Twine(Reason), GenCrashDiag, IsUsageError);
 }
 
-void llvm::report_fatal_error(StringRef Reason, bool GenCrashDiag) {
-  report_fatal_error(Twine(Reason), GenCrashDiag);
+void llvm::report_fatal_error(StringRef Reason, bool GenCrashDiag, bool IsUsageError) {
+  report_fatal_error(Twine(Reason), GenCrashDiag, IsUsageError);
 }
 
-void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
+void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag, bool IsUsageError) {
   llvm::fatal_error_handler_t handler = nullptr;
   void* handlerData = nullptr;
   {
@@ -118,7 +118,7 @@ void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
   // 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();
+  sys::RunInterruptHandlers(IsUsageError);
 
   if (GenCrashDiag)
     abort();
@@ -127,22 +127,27 @@ void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
 }
 
 void llvm::reportFatalInternalError(const char *reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/true);
+  report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false);
 }
+
 void llvm::reportFatalInternalError(StringRef reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/true);
+  report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false);
 }
+
 void llvm::reportFatalInternalError(const Twine &reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/true);
+  report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false);
 }
+
 void llvm::reportFatalUsageError(const char *reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/false);
+  report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true);
 }
+
 void llvm::reportFatalUsageError(StringRef reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/false);
+  report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true);
 }
+
 void llvm::reportFatalUsageError(const Twine &reason) {
-  report_fatal_error(reason, /*GenCrashDiag=*/false);
+  report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true);
 }
 
 void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler,
diff --git a/llvm/lib/Support/PrettyStackTrace.cpp b/llvm/lib/Support/PrettyStackTrace.cpp
@@ -150,7 +150,10 @@ alignas(CrashHandlerString) static CrashHandlerStringStorage
 
 /// This callback is run if a fatal signal is delivered to the process, it
 /// prints the pretty stack trace.
-static void CrashHandler(void *) {
+static void CrashHandler(void *, bool IsUsageError) {
+  if (IsUsageError)
+    return;
+
   errs() << BugReportMsg ;
 
 #ifndef __APPLE__
diff --git a/llvm/lib/Support/Signals.cpp b/llvm/lib/Support/Signals.cpp
@@ -95,13 +95,13 @@ CallBacksToRun() {
 }
 
 // Signal-safe.
-void sys::RunSignalHandlers() {
+void sys::RunSignalHandlers(bool IsUsageError) {
   for (CallbackAndCookie &RunMe : CallBacksToRun()) {
     auto Expected = CallbackAndCookie::Status::Initialized;
     auto Desired = CallbackAndCookie::Status::Executing;
     if (!RunMe.Flag.compare_exchange_strong(Expected, Desired))
       continue;
-    (*RunMe.Callback)(RunMe.Cookie);
+    (*RunMe.Callback)(RunMe.Cookie, IsUsageError);
     RunMe.Callback = nullptr;
     RunMe.Cookie = nullptr;
     RunMe.Flag.store(CallbackAndCookie::Status::Empty);
diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc
@@ -611,7 +611,7 @@ void llvm::sys::AddSignalHandler(sys::SignalHandlerCallback FnPtr,
   LeaveCriticalSection(&CriticalSection);
 }
 
-static void Cleanup(bool ExecuteSignalHandlers) {
+static void Cleanup(bool ExecuteSignalHandlers, bool IsUsageError) {
   if (CleanupExecuted)
     return;
 
@@ -629,18 +629,18 @@ static void Cleanup(bool ExecuteSignalHandlers) {
     }
 
   if (ExecuteSignalHandlers)
-    llvm::sys::RunSignalHandlers();
+    llvm::sys::RunSignalHandlers(IsUsageError);
 
   LeaveCriticalSection(&CriticalSection);
 }
 
-void llvm::sys::RunInterruptHandlers() {
+void llvm::sys::RunInterruptHandlers(bool IsUsageError) {
   // The interrupt handler may be called from an interrupt, but it may also be
   // called manually (such as the case of report_fatal_error with no registered
   // error handler). We must ensure that the critical section is properly
   // initialized.
   InitializeThreading();
-  Cleanup(true);
+  Cleanup(true, IsUsageError);
 }
 
 /// Find the Windows Registry Key for a given location.
@@ -847,7 +847,7 @@ void sys::CleanupOnSignal(uintptr_t Context) {
 }
 
 static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
-  Cleanup(true);
+  Cleanup(/*ExecuteSignalHandlers=*/true, /*IsUsageError=*/false);
 
   // Write out the exception code.
   if (ep && ep->ExceptionRecord)
@@ -887,7 +887,7 @@ static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
   // This function is only ever called when a CTRL-C or similar control signal
   // is fired. Killing a process in this way is normal, so don't trigger the
   // signal handlers.
-  Cleanup(false);
+  Cleanup(/*ExecuteSignalHandlers=*/false, /*IsUsageError=*/false);
 
   // If an interrupt function has been set, go and run one it; otherwise,
   // the process dies.
diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp
@@ -43,7 +43,7 @@ static void PrintMessage(ArrayRef<SMLoc> Loc, SourceMgr::DiagKind Kind,
 // Run file cleanup handlers and then exit fatally (with non-zero exit code).
 [[noreturn]] inline static void fatal_exit() {
   // The following call runs the file cleanup handlers.
-  sys::RunInterruptHandlers();
+  sys::RunInterruptHandlers(/*IsUsageError=*/false);
   std::exit(1);
 }
 
diff --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp
@@ -36,7 +36,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; }
+static void incrementGlobalWithParam(void *, bool) { ++GlobalInt; }
 
 TEST(CrashRecoveryTest, Basic) {
   llvm::CrashRecoveryContext::Enable();

Original file line number	Diff line number	Diff line change
`@@ -2468,7 +2468,7 @@ void PrintCrashIRInstrumentation::reportCrashIR() {`
`2468`	`2468`	`}`
`2469`	`2469`	`}`
`2470`	`2470`
`2471`		`-void PrintCrashIRInstrumentation::SignalHandler(void *) {`
	`2471`	`+void PrintCrashIRInstrumentation::SignalHandler(void *, bool) {`
`2472`	`2472`	`// Called by signal handlers so do not lock here`
`2473`	`2473`	`// Is the PrintCrashIRInstrumentation still alive?`
`2474`	`2474`	`if (!CrashReporter)`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ void llvm::initDebugOptions() {`
`148`	`148`	`}`
`149`	`149`
`150`	`150`	`// Signal handlers - dump debug output on termination.`
`151`		`-static void debug_user_sig_handler(void *Cookie) {`
	`151`	`+static void debug_user_sig_handler(void Cookie, bool /IsUsageError*/) {`
`152`	`152`	`// This is a bit sneaky. Since this is under #ifndef NDEBUG, we`
`153`	`153`	`// know that debug mode is enabled and dbgs() really is a`
`154`	`154`	`// circular_raw_ostream. If NDEBUG is defined, then dbgs() ==`