Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bd1976bris
Copy link
Collaborator

@bd1976bris bd1976bris commented May 21, 2025

On Windows, LLVM’s reportFatalUsageError, introduced recently (#138251), emits bug report prompt due to the PrettyStackTrace signal handler, initialized via InitLLVM. This occurs when sys::RunInterruptHandlers() is called from reportFatalUsageError.

This behavior is misleading for usage errors. For example, one of Sony’s customers filed a bug after specifying an invalid LTO cache directory - a clear usage error - because the toolchain output included instructions to report a bug.

This patch suppresses PrettyStackTrace output for usage errors by adding a flag to sys::RunInterruptHandlers() to indicate whether signal handlers should be executed.

LLVM Issue: #140953
Internal Tracker: TOOLCHAIN-17744

@bd1976bris bd1976bris requested review from nikic, asb and tru and removed request for nikic and asb May 21, 2025 19:20
@llvmbot llvmbot added clang Clang issues not falling into any other category lld tablegen lld:COFF platform:windows LTO Link time optimization (regular/full LTO or ThinLTO) llvm:support labels May 21, 2025
@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-clang

Author: bd1976bris (bd1976bris)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/140956.diff

17 Files Affected:

  • (modified) clang/tools/clang-repl/ClangRepl.cpp (+1-1)
  • (modified) clang/tools/driver/cc1_main.cpp (+1-1)
  • (modified) lld/test/COFF/lto-cache-errors.ll (+16-7)
  • (modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1-1)
  • (modified) llvm/include/llvm/Support/Error.h (+2-1)
  • (modified) llvm/include/llvm/Support/ErrorHandling.h (+6-3)
  • (modified) llvm/include/llvm/Support/Signals.h (+3-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
  • (modified) llvm/lib/Support/Debug.cpp (+1-1)
  • (modified) llvm/lib/Support/Error.cpp (+5-4)
  • (modified) llvm/lib/Support/ErrorHandling.cpp (+17-12)
  • (modified) llvm/lib/Support/PrettyStackTrace.cpp (+4-1)
  • (modified) llvm/lib/Support/Signals.cpp (+2-2)
  • (modified) llvm/lib/Support/Windows/Signals.inc (+6-6)
  • (modified) llvm/lib/TableGen/Error.cpp (+1-1)
  • (modified) llvm/unittests/Support/CrashRecoveryTest.cpp (+1-1)
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 7af8e4f25d99e..79c2c21b004b6 100644
--- 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
index 6638a15ff7e12..ddccaf8bfdbfc 100644
--- 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
index a46190a81b623..093e66cc67c0c 100644
--- 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
index f7a65a88ecf5b..3ec3c607e2861 100644
--- 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
index b0bcdd55e4831..b07f93d9fa82d 100644
--- 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
index 4c17b6e83acd2..af024e3c4f150 100644
--- 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
index 6ce26acdd458e..278569015e77a 100644
--- 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
index b7db70b99bcbc..d42363f80c482 100644
--- 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
index dc1dd5d9c7f4c..6333a35116f2e 100644
--- 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
index 5bb04d0c22998..2b3e52683fe35 100644
--- 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
index d168b462a6eb2..394e9b45d10bc 100644
--- 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
index cc16f2037ea58..c7aa00415e9e5 100644
--- 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
index 9b09384e95bfe..4bdd18b0df120 100644
--- 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
index 9f9030e79d104..e39cb703987e9 100644
--- 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
index f11ad09f37139..cb4b77680dfb6 100644
--- 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
index 91423664a84cc..9a95c9df9a44f 100644
--- 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
index ceafba5b36f11..831d21a15ef9a 100644
--- 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();

@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-lld-coff

Author: bd1976bris (bd1976bris)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/140956.diff

17 Files Affected:

  • (modified) clang/tools/clang-repl/ClangRepl.cpp (+1-1)
  • (modified) clang/tools/driver/cc1_main.cpp (+1-1)
  • (modified) lld/test/COFF/lto-cache-errors.ll (+16-7)
  • (modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1-1)
  • (modified) llvm/include/llvm/Support/Error.h (+2-1)
  • (modified) llvm/include/llvm/Support/ErrorHandling.h (+6-3)
  • (modified) llvm/include/llvm/Support/Signals.h (+3-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
  • (modified) llvm/lib/Support/Debug.cpp (+1-1)
  • (modified) llvm/lib/Support/Error.cpp (+5-4)
  • (modified) llvm/lib/Support/ErrorHandling.cpp (+17-12)
  • (modified) llvm/lib/Support/PrettyStackTrace.cpp (+4-1)
  • (modified) llvm/lib/Support/Signals.cpp (+2-2)
  • (modified) llvm/lib/Support/Windows/Signals.inc (+6-6)
  • (modified) llvm/lib/TableGen/Error.cpp (+1-1)
  • (modified) llvm/unittests/Support/CrashRecoveryTest.cpp (+1-1)
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 7af8e4f25d99e..79c2c21b004b6 100644
--- 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
index 6638a15ff7e12..ddccaf8bfdbfc 100644
--- 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
index a46190a81b623..093e66cc67c0c 100644
--- 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
index f7a65a88ecf5b..3ec3c607e2861 100644
--- 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
index b0bcdd55e4831..b07f93d9fa82d 100644
--- 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
index 4c17b6e83acd2..af024e3c4f150 100644
--- 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
index 6ce26acdd458e..278569015e77a 100644
--- 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
index b7db70b99bcbc..d42363f80c482 100644
--- 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
index dc1dd5d9c7f4c..6333a35116f2e 100644
--- 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
index 5bb04d0c22998..2b3e52683fe35 100644
--- 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
index d168b462a6eb2..394e9b45d10bc 100644
--- 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
index cc16f2037ea58..c7aa00415e9e5 100644
--- 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
index 9b09384e95bfe..4bdd18b0df120 100644
--- 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
index 9f9030e79d104..e39cb703987e9 100644
--- 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
index f11ad09f37139..cb4b77680dfb6 100644
--- 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
index 91423664a84cc..9a95c9df9a44f 100644
--- 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
index ceafba5b36f11..831d21a15ef9a 100644
--- 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();

@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-tablegen

Author: bd1976bris (bd1976bris)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/140956.diff

17 Files Affected:

  • (modified) clang/tools/clang-repl/ClangRepl.cpp (+1-1)
  • (modified) clang/tools/driver/cc1_main.cpp (+1-1)
  • (modified) lld/test/COFF/lto-cache-errors.ll (+16-7)
  • (modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1-1)
  • (modified) llvm/include/llvm/Support/Error.h (+2-1)
  • (modified) llvm/include/llvm/Support/ErrorHandling.h (+6-3)
  • (modified) llvm/include/llvm/Support/Signals.h (+3-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
  • (modified) llvm/lib/Support/Debug.cpp (+1-1)
  • (modified) llvm/lib/Support/Error.cpp (+5-4)
  • (modified) llvm/lib/Support/ErrorHandling.cpp (+17-12)
  • (modified) llvm/lib/Support/PrettyStackTrace.cpp (+4-1)
  • (modified) llvm/lib/Support/Signals.cpp (+2-2)
  • (modified) llvm/lib/Support/Windows/Signals.inc (+6-6)
  • (modified) llvm/lib/TableGen/Error.cpp (+1-1)
  • (modified) llvm/unittests/Support/CrashRecoveryTest.cpp (+1-1)
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 7af8e4f25d99e..79c2c21b004b6 100644
--- 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
index 6638a15ff7e12..ddccaf8bfdbfc 100644
--- 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
index a46190a81b623..093e66cc67c0c 100644
--- 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
index f7a65a88ecf5b..3ec3c607e2861 100644
--- 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
index b0bcdd55e4831..b07f93d9fa82d 100644
--- 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
index 4c17b6e83acd2..af024e3c4f150 100644
--- 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
index 6ce26acdd458e..278569015e77a 100644
--- 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
index b7db70b99bcbc..d42363f80c482 100644
--- 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
index dc1dd5d9c7f4c..6333a35116f2e 100644
--- 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
index 5bb04d0c22998..2b3e52683fe35 100644
--- 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
index d168b462a6eb2..394e9b45d10bc 100644
--- 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
index cc16f2037ea58..c7aa00415e9e5 100644
--- 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
index 9b09384e95bfe..4bdd18b0df120 100644
--- 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
index 9f9030e79d104..e39cb703987e9 100644
--- 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
index f11ad09f37139..cb4b77680dfb6 100644
--- 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
index 91423664a84cc..9a95c9df9a44f 100644
--- 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
index ceafba5b36f11..831d21a15ef9a 100644
--- 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();

@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-lto

Author: bd1976bris (bd1976bris)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/140956.diff

17 Files Affected:

  • (modified) clang/tools/clang-repl/ClangRepl.cpp (+1-1)
  • (modified) clang/tools/driver/cc1_main.cpp (+1-1)
  • (modified) lld/test/COFF/lto-cache-errors.ll (+16-7)
  • (modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1-1)
  • (modified) llvm/include/llvm/Support/Error.h (+2-1)
  • (modified) llvm/include/llvm/Support/ErrorHandling.h (+6-3)
  • (modified) llvm/include/llvm/Support/Signals.h (+3-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
  • (modified) llvm/lib/Support/Debug.cpp (+1-1)
  • (modified) llvm/lib/Support/Error.cpp (+5-4)
  • (modified) llvm/lib/Support/ErrorHandling.cpp (+17-12)
  • (modified) llvm/lib/Support/PrettyStackTrace.cpp (+4-1)
  • (modified) llvm/lib/Support/Signals.cpp (+2-2)
  • (modified) llvm/lib/Support/Windows/Signals.inc (+6-6)
  • (modified) llvm/lib/TableGen/Error.cpp (+1-1)
  • (modified) llvm/unittests/Support/CrashRecoveryTest.cpp (+1-1)
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 7af8e4f25d99e..79c2c21b004b6 100644
--- 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
index 6638a15ff7e12..ddccaf8bfdbfc 100644
--- 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
index a46190a81b623..093e66cc67c0c 100644
--- 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
index f7a65a88ecf5b..3ec3c607e2861 100644
--- 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
index b0bcdd55e4831..b07f93d9fa82d 100644
--- 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
index 4c17b6e83acd2..af024e3c4f150 100644
--- 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
index 6ce26acdd458e..278569015e77a 100644
--- 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
index b7db70b99bcbc..d42363f80c482 100644
--- 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
index dc1dd5d9c7f4c..6333a35116f2e 100644
--- 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
index 5bb04d0c22998..2b3e52683fe35 100644
--- 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
index d168b462a6eb2..394e9b45d10bc 100644
--- 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
index cc16f2037ea58..c7aa00415e9e5 100644
--- 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
index 9b09384e95bfe..4bdd18b0df120 100644
--- 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
index 9f9030e79d104..e39cb703987e9 100644
--- 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
index f11ad09f37139..cb4b77680dfb6 100644
--- 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
index 91423664a84cc..9a95c9df9a44f 100644
--- 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
index ceafba5b36f11..831d21a15ef9a 100644
--- 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();

@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-llvm-support

Author: bd1976bris (bd1976bris)

Changes

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


Full diff: https://github.com/llvm/llvm-project/pull/140956.diff

17 Files Affected:

  • (modified) clang/tools/clang-repl/ClangRepl.cpp (+1-1)
  • (modified) clang/tools/driver/cc1_main.cpp (+1-1)
  • (modified) lld/test/COFF/lto-cache-errors.ll (+16-7)
  • (modified) llvm/include/llvm/Passes/StandardInstrumentations.h (+1-1)
  • (modified) llvm/include/llvm/Support/Error.h (+2-1)
  • (modified) llvm/include/llvm/Support/ErrorHandling.h (+6-3)
  • (modified) llvm/include/llvm/Support/Signals.h (+3-3)
  • (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
  • (modified) llvm/lib/Passes/StandardInstrumentations.cpp (+1-1)
  • (modified) llvm/lib/Support/Debug.cpp (+1-1)
  • (modified) llvm/lib/Support/Error.cpp (+5-4)
  • (modified) llvm/lib/Support/ErrorHandling.cpp (+17-12)
  • (modified) llvm/lib/Support/PrettyStackTrace.cpp (+4-1)
  • (modified) llvm/lib/Support/Signals.cpp (+2-2)
  • (modified) llvm/lib/Support/Windows/Signals.inc (+6-6)
  • (modified) llvm/lib/TableGen/Error.cpp (+1-1)
  • (modified) llvm/unittests/Support/CrashRecoveryTest.cpp (+1-1)
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 7af8e4f25d99e..79c2c21b004b6 100644
--- 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
index 6638a15ff7e12..ddccaf8bfdbfc 100644
--- 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
index a46190a81b623..093e66cc67c0c 100644
--- 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
index f7a65a88ecf5b..3ec3c607e2861 100644
--- 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
index b0bcdd55e4831..b07f93d9fa82d 100644
--- 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
index 4c17b6e83acd2..af024e3c4f150 100644
--- 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
index 6ce26acdd458e..278569015e77a 100644
--- 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
index b7db70b99bcbc..d42363f80c482 100644
--- 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
index dc1dd5d9c7f4c..6333a35116f2e 100644
--- 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
index 5bb04d0c22998..2b3e52683fe35 100644
--- 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
index d168b462a6eb2..394e9b45d10bc 100644
--- 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
index cc16f2037ea58..c7aa00415e9e5 100644
--- 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
index 9b09384e95bfe..4bdd18b0df120 100644
--- 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
index 9f9030e79d104..e39cb703987e9 100644
--- 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
index f11ad09f37139..cb4b77680dfb6 100644
--- 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
index 91423664a84cc..9a95c9df9a44f 100644
--- 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
index ceafba5b36f11..831d21a15ef9a 100644
--- 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();

Copy link

github-actions bot commented May 21, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This has bitten us before as well.

@bd1976bris bd1976bris requested review from asb, bogner and nikic May 21, 2025 19:29
[[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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes don't make sense: gen_crash_diag is already the flag that indicates whether this is a usage error and controls whether a stack trace is displayed.

Calling reportFatalUsageError() will correctly not display a stack trace and not ask for a bug report on Linux. If this is not the case on Windows, you need to identify why there is a difference in behavior. Adding an extra flag here is certainly the wrong thing to do though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I thought that adding a new flag would mean that the behaviours of 1. report_fatal_error calling abort() and 2. The signal handler having usage or internal error behaviour were independent - which might be required. I'll update to just use gen_crash_diag - that will make for a smaller patch anyway.

As to why Windows has different behaviour - on Linux LLVM once called the signal handler for report_fatal_error IIUC but this seems to have changed in: 288999b.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic I'm just using gen_crash_diag now. Any good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As to why Windows has different behaviour - on Linux LLVM once called the signal handler for report_fatal_error IIUC but this seems to have changed in: 288999b.

So, can we do the same change for Windows? Basically pass false to Cleanup in the Windows RunInterruptHandlers, or something along those lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic I have just realised that I might be able to be a bit braver and not pass anything into the signal handlers but instead just not call them in Windows signal.inc. Let me try that...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As to why Windows has different behaviour - on Linux LLVM once called the signal handler for report_fatal_error IIUC but this seems to have changed in: 288999b.

So, can we do the same change for Windows? Basically pass false to Cleanup in the Windows RunInterruptHandlers, or something along those lines?

Seems to work and much better. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My remaining question here would be whether we actually need the ExecuteSignalHandlers argument on RunInterruptHandlers, or can always assume it is false like on Linux. If I understood things correctly, the way this is supposed to work is that if GenCrashDiag is true then the following abort() is going to run the signal handlers anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good questions.

If I understood things correctly, the way this is supposed to work is that if GenCrashDiag is true then the following abort() is going to run the signal handlers anyway?

Yes. To keep the previous behaviour where the signal handlers are always run. I need to poke at it a bit. Perhaps the output is OK on Windows without running the signal handlers. I don't understand why the difference exists between the two platforms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I believe to be the case. On Windows the signal handlers (PrettyStackTrace in this case) are not invoked via abort() which is why they need to be invoked via RunInterruptHandlers. On Linux they are invoked via abort() which is why RunInterruptHandlers does not need to invoke the interrupt handlers.

Also note that I made a mistake: Despite it's name, PrettyStackTrace does not print a stack trace. It only prints the "please report a bug" message. The stack trace is emitted when abort() is called. Therefore, on both Windows and Linux passing gen_crash_diag does prevent the stack trace in the output, it just doesn't prevent the "please report a bug" message on Windows. Apologies for the misinformation/confusion on this.

@bd1976bris bd1976bris requested a review from rnk May 21, 2025 19:56
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels May 21, 2025
@bd1976bris bd1976bris requested a review from optimisan May 21, 2025 22:08
@bd1976bris
Copy link
Collaborator Author

bd1976bris commented May 21, 2025

I wonder if we need a release note for this behaviour change? I've added one in case.

@bd1976bris bd1976bris force-pushed the no_st_and_bug_for_usage_errs branch from 030edda to 23b762f Compare May 21, 2025 22:57
; 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 this test is limited to Windows.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching a test from "not windows" to "only windows" is suspicious. Is it just the error message phrasing? Can you follow the pattern of other tests that depend on error strings, e.g. llvm/test/tools/llvm-ar/error-opening-permission.test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new failure condition should probably be a separate test and not a change to the old one

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I made the test Windows specific because there is additional output on Windows (that I'm trying to eliminate in this patch) so that seems a better platform to run this test on than Linux. It's stell testing the same failure condition. I'll try to either make the test work on both platforms or add a separate Windows specific test.

Copy link
Collaborator Author

@bd1976bris bd1976bris May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have had to change the test strategy for this to use the invalid pass pipeline errors, as they occur in a single-threaded context*. The test for that in LLD works on both platforms.

* #140955 was reverted. Digging into why the test failed, I suspect that reportFatalUsageError can cause a crash if threads call reportFatalUsageError simultaneously (the ThinLTO cache dir is used from a multi-threaded context). I have observed this on Linux. It may also apply on Windows.

@@ -744,7 +744,7 @@ 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated rename

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll remove anything that is just a tidy-up and not required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

On Windows, LLVM’s `reportFatalUsageError` (llvm#138251) may emit a
bug report prompt due to the `PrettyStackTrace` signal handler,
initialized via `InitLLVM`. This occurs when `RunInterruptHandlers()`
is called from `reportFatalUsageError`.

This behavior is misleading for usage errors. For example, one
of Sony’s customers filed a bug after specifying an invalid LTO
cache directory - a clear usage error - because the toolchain
output included instructions to report a bug.

This patch suppresses `PrettyStackTrace` output for usage errors by
adding a flag to `sys::RunInterruptHandlers()` to indicate whether
signal handlers should be executed.

To test this, I have modified the invalid LTO pipeline errors to call
`reportFatalUsageError`, and I have updated the existing LLD test to
additionally verify that no bug report message has been emitted.

LLVM Issue: llvm#140953
Internal Tracker: TOOLCHAIN-17744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category lld:COFF lld:ELF lld llvm:support LTO Link time optimization (regular/full LTO or ThinLTO) mlir:core MLIR Core Infrastructure mlir platform:windows tablegen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants