https://github.com/bd1976bris updated https://github.com/llvm/llvm-project/pull/140956
>From 0d9218eda0b99b0d439afb8a84a056581362638e Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Mon, 19 May 2025 10:17:00 +0100 Subject: [PATCH 1/7] [Windows][LLVM] Elide `PrettyStackTrace` output for usage errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/llvm/llvm-project/issues/140953 Internal Tracker: TOOLCHAIN-17744 --- clang/tools/clang-repl/ClangRepl.cpp | 2 +- clang/tools/driver/cc1_main.cpp | 2 +- lld/test/COFF/lto-cache-errors.ll | 23 ++++++++++----- .../llvm/Passes/StandardInstrumentations.h | 2 +- llvm/include/llvm/Support/Error.h | 3 +- llvm/include/llvm/Support/ErrorHandling.h | 9 ++++-- llvm/include/llvm/Support/Signals.h | 6 ++-- llvm/lib/LTO/LTOBackend.cpp | 2 +- llvm/lib/Passes/StandardInstrumentations.cpp | 2 +- llvm/lib/Support/Debug.cpp | 2 +- llvm/lib/Support/Error.cpp | 9 +++--- llvm/lib/Support/ErrorHandling.cpp | 29 +++++++++++-------- llvm/lib/Support/PrettyStackTrace.cpp | 5 +++- llvm/lib/Support/Signals.cpp | 4 +-- llvm/lib/Support/Windows/Signals.inc | 12 ++++---- llvm/lib/TableGen/Error.cpp | 2 +- llvm/unittests/Support/CrashRecoveryTest.cpp | 2 +- 17 files changed, 69 insertions(+), 47 deletions(-) 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(); >From 304189f162c391da67568486bdb6d3c13459773a Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 20:49:13 +0100 Subject: [PATCH 2/7] apply clang-format --- llvm/lib/Support/Error.cpp | 6 ++++-- llvm/lib/Support/ErrorHandling.cpp | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp index 394e9b45d10bc..3c20532a3f76f 100644 --- a/llvm/lib/Support/Error.cpp +++ b/llvm/lib/Support/Error.cpp @@ -175,11 +175,13 @@ void report_fatal_error(Error Err, bool GenCrashDiag, bool IsUsageError) { } void reportFatalInternalError(Error Err) { - report_fatal_error(std::move(Err), /*GenCrashDiag=*/true, /*IsUsageError=*/false); + report_fatal_error(std::move(Err), /*GenCrashDiag=*/true, + /*IsUsageError=*/false); } void reportFatalUsageError(Error Err) { - report_fatal_error(std::move(Err), /*GenCrashDiag=*/false, /*IsUsageError=*/true); + 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 c7aa00415e9e5..7fe00c9d7d643 100644 --- a/llvm/lib/Support/ErrorHandling.cpp +++ b/llvm/lib/Support/ErrorHandling.cpp @@ -80,15 +80,18 @@ void llvm::remove_fatal_error_handler() { ErrorHandlerUserData = nullptr; } -void llvm::report_fatal_error(const char *Reason, bool GenCrashDiag, bool IsUsageError) { +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, bool IsUsageError) { +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, bool IsUsageError) { +void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag, + bool IsUsageError) { llvm::fatal_error_handler_t handler = nullptr; void* handlerData = nullptr; { >From bf6fbe279d4e8d51e04ec005cafe0eab59ab69d0 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 21:14:52 +0100 Subject: [PATCH 3/7] Drop additional parameter and rely on gen_crash_diag --- clang/tools/clang-repl/ClangRepl.cpp | 2 +- clang/tools/driver/cc1_main.cpp | 2 +- llvm/include/llvm/Support/Error.h | 3 +-- llvm/include/llvm/Support/ErrorHandling.h | 9 +++----- llvm/include/llvm/Support/Signals.h | 4 ++-- llvm/lib/Support/Debug.cpp | 2 +- llvm/lib/Support/Error.cpp | 10 ++++----- llvm/lib/Support/ErrorHandling.cpp | 27 ++++++++++------------- llvm/lib/Support/PrettyStackTrace.cpp | 4 ++-- llvm/lib/Support/Signals.cpp | 4 ++-- llvm/lib/Support/Windows/Signals.inc | 12 +++++----- llvm/lib/TableGen/Error.cpp | 2 +- 12 files changed, 36 insertions(+), 45 deletions(-) diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp index 79c2c21b004b6..cf1255a2ab3ad 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(/*IsUsageError=*/false); + llvm::sys::RunInterruptHandlers(/*GenCrashDiag=*/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 ddccaf8bfdbfc..417c7e1b1a006 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(/*IsUsageError=*/false); + llvm::sys::RunInterruptHandlers(/*GenCrashDiag=*/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/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index b07f93d9fa82d..2141d77cb8bfc 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -742,8 +742,7 @@ template <class T> class [[nodiscard]] Expected { /// @deprecated Use reportFatalInternalError() or reportFatalUsageError() /// instead. [[noreturn]] LLVM_ABI void report_fatal_error(Error Err, - bool GenCrashDiag = true, - bool IsUsageError = false); + bool GenCrashDiag = true); /// 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 af024e3c4f150..4c17b6e83acd2 100644 --- a/llvm/include/llvm/Support/ErrorHandling.h +++ b/llvm/include/llvm/Support/ErrorHandling.h @@ -61,14 +61,11 @@ struct ScopedFatalErrorHandler { /// @deprecated Use reportFatalInternalError() or reportFatalUsageError() /// instead. [[noreturn]] LLVM_ABI void report_fatal_error(const char *reason, - bool gen_crash_diag = true, - bool is_usage_error = false); + bool gen_crash_diag = true); [[noreturn]] LLVM_ABI void report_fatal_error(StringRef reason, - bool gen_crash_diag = true, - bool is_usage_error = false); + bool gen_crash_diag = true); [[noreturn]] LLVM_ABI void report_fatal_error(const Twine &reason, - bool gen_crash_diag = true, - bool is_usage_error = false); + bool gen_crash_diag = true); /// 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 278569015e77a..88ba53a58eed9 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(bool IsUsageError); +LLVM_ABI void RunInterruptHandlers(bool GenCrashDiag); /// This function registers signal handlers to ensure that if a signal gets /// delivered that the named file is removed. @@ -58,7 +58,7 @@ LLVM_ABI void DisableSystemDialogsOnCrash(); LLVM_ABI void PrintStackTrace(raw_ostream &OS, int Depth = 0); // Run all registered signal handlers. -LLVM_ABI void RunSignalHandlers(bool IsUsageError); +LLVM_ABI void RunSignalHandlers(bool GenCrashDiag); using SignalHandlerCallback = void (*)(void *, bool); diff --git a/llvm/lib/Support/Debug.cpp b/llvm/lib/Support/Debug.cpp index 2b3e52683fe35..891570437899a 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, bool /*IsUsageError*/) { +static void debug_user_sig_handler(void *Cookie, bool /*GenCrashDiag*/) { // 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 3c20532a3f76f..677f0a7acfebf 100644 --- a/llvm/lib/Support/Error.cpp +++ b/llvm/lib/Support/Error.cpp @@ -164,24 +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, bool IsUsageError) { +void report_fatal_error(Error Err, bool GenCrashDiag) { 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, IsUsageError); + report_fatal_error(Twine(ErrMsg), GenCrashDiag); } void reportFatalInternalError(Error Err) { - report_fatal_error(std::move(Err), /*GenCrashDiag=*/true, - /*IsUsageError=*/false); + report_fatal_error(std::move(Err), /*GenCrashDiag=*/true); } void reportFatalUsageError(Error Err) { - report_fatal_error(std::move(Err), /*GenCrashDiag=*/false, - /*IsUsageError=*/true); + report_fatal_error(std::move(Err), /*GenCrashDiag=*/false); } } // end namespace llvm diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp index 7fe00c9d7d643..3d42b9aae733f 100644 --- a/llvm/lib/Support/ErrorHandling.cpp +++ b/llvm/lib/Support/ErrorHandling.cpp @@ -80,18 +80,15 @@ void llvm::remove_fatal_error_handler() { ErrorHandlerUserData = nullptr; } -void llvm::report_fatal_error(const char *Reason, bool GenCrashDiag, - bool IsUsageError) { - report_fatal_error(Twine(Reason), GenCrashDiag, IsUsageError); +void llvm::report_fatal_error(const char *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(StringRef Reason, bool GenCrashDiag) { + report_fatal_error(Twine(Reason), GenCrashDiag); } -void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag, - bool IsUsageError) { +void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) { llvm::fatal_error_handler_t handler = nullptr; void* handlerData = nullptr; { @@ -121,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(IsUsageError); + sys::RunInterruptHandlers(GenCrashDiag); if (GenCrashDiag) abort(); @@ -130,27 +127,27 @@ void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag, } void llvm::reportFatalInternalError(const char *reason) { - report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false); + report_fatal_error(reason, /*gen_crash_diag=*/true); } void llvm::reportFatalInternalError(StringRef reason) { - report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false); + report_fatal_error(reason, /*gen_crash_diag=*/true); } void llvm::reportFatalInternalError(const Twine &reason) { - report_fatal_error(reason, /*gen_crash_diag=*/true, /*is_usage_error=*/false); + report_fatal_error(reason, /*gen_crash_diag=*/true); } void llvm::reportFatalUsageError(const char *reason) { - report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true); + report_fatal_error(reason, /*gen_crash_diag=*/false); } void llvm::reportFatalUsageError(StringRef reason) { - report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true); + report_fatal_error(reason, /*gen_crash_diag=*/false); } void llvm::reportFatalUsageError(const Twine &reason) { - report_fatal_error(reason, /*gen_crash_diag=*/false, /*is_usage_error=*/true); + report_fatal_error(reason, /*gen_crash_diag=*/false); } 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 4bdd18b0df120..d9ade170e7265 100644 --- a/llvm/lib/Support/PrettyStackTrace.cpp +++ b/llvm/lib/Support/PrettyStackTrace.cpp @@ -150,8 +150,8 @@ 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 *, bool IsUsageError) { - if (IsUsageError) +static void CrashHandler(void *, bool GenCrashDiag) { + if (!GenCrashDiag) return; errs() << BugReportMsg ; diff --git a/llvm/lib/Support/Signals.cpp b/llvm/lib/Support/Signals.cpp index e39cb703987e9..dbeba3c2ac8e3 100644 --- a/llvm/lib/Support/Signals.cpp +++ b/llvm/lib/Support/Signals.cpp @@ -95,13 +95,13 @@ CallBacksToRun() { } // Signal-safe. -void sys::RunSignalHandlers(bool IsUsageError) { +void sys::RunSignalHandlers(bool GenCrashDiag) { 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, IsUsageError); + (*RunMe.Callback)(RunMe.Cookie, GenCrashDiag); 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 cb4b77680dfb6..b77a7e024dce5 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, bool IsUsageError) { +static void Cleanup(bool ExecuteSignalHandlers, bool GenCrashDiag) { if (CleanupExecuted) return; @@ -629,18 +629,18 @@ static void Cleanup(bool ExecuteSignalHandlers, bool IsUsageError) { } if (ExecuteSignalHandlers) - llvm::sys::RunSignalHandlers(IsUsageError); + llvm::sys::RunSignalHandlers(GenCrashDiag); LeaveCriticalSection(&CriticalSection); } -void llvm::sys::RunInterruptHandlers(bool IsUsageError) { +void llvm::sys::RunInterruptHandlers(bool GenCrashDiag) { // 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, IsUsageError); + Cleanup(true, GenCrashDiag); } /// 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(/*ExecuteSignalHandlers=*/true, /*IsUsageError=*/false); + Cleanup(/*ExecuteSignalHandlers=*/true, /*GenCrashDiag=*/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(/*ExecuteSignalHandlers=*/false, /*IsUsageError=*/false); + Cleanup(/*ExecuteSignalHandlers=*/false, /*GenCrashDiag=*/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 9a95c9df9a44f..7f54cf2adba33 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(/*IsUsageError=*/false); + sys::RunInterruptHandlers(/*GenCrashDiag=*/false); std::exit(1); } >From ef2776b7ecb810010be606d61ae53dab83d49b61 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 21:47:20 +0100 Subject: [PATCH 4/7] Don't call signal handler in Windows signal.inc --- clang/tools/clang-repl/ClangRepl.cpp | 2 +- clang/tools/driver/cc1_main.cpp | 2 +- llvm/include/llvm/Passes/StandardInstrumentations.h | 2 +- llvm/include/llvm/Support/Signals.h | 6 +++--- llvm/lib/Passes/StandardInstrumentations.cpp | 2 +- llvm/lib/Support/Debug.cpp | 2 +- llvm/lib/Support/PrettyStackTrace.cpp | 5 +---- llvm/lib/Support/Signals.cpp | 4 ++-- llvm/lib/Support/Windows/Signals.inc | 12 ++++++------ llvm/lib/TableGen/Error.cpp | 2 +- llvm/unittests/Support/CrashRecoveryTest.cpp | 2 +- 11 files changed, 19 insertions(+), 22 deletions(-) diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp index cf1255a2ab3ad..1ef0d27c4b128 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(/*GenCrashDiag=*/false); + llvm::sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/true); // 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 417c7e1b1a006..b229e7fd1d994 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(/*GenCrashDiag=*/false); + llvm::sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/true); // 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/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h index 3ec3c607e2861..f7a65a88ecf5b 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 *, bool); + static void SignalHandler(void *); }; /// This class provides an interface to register all the standard pass diff --git a/llvm/include/llvm/Support/Signals.h b/llvm/include/llvm/Support/Signals.h index 88ba53a58eed9..5e309e6c51604 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(bool GenCrashDiag); +LLVM_ABI void RunInterruptHandlers(bool ExecuteSignalHandlers); /// 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(bool GenCrashDiag); +LLVM_ABI void RunSignalHandlers(); -using SignalHandlerCallback = void (*)(void *, bool); +using SignalHandlerCallback = void (*)(void *); /// 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/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp index 6333a35116f2e..dc1dd5d9c7f4c 100644 --- a/llvm/lib/Passes/StandardInstrumentations.cpp +++ b/llvm/lib/Passes/StandardInstrumentations.cpp @@ -2468,7 +2468,7 @@ void PrintCrashIRInstrumentation::reportCrashIR() { } } -void PrintCrashIRInstrumentation::SignalHandler(void *, bool) { +void PrintCrashIRInstrumentation::SignalHandler(void *) { // 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 891570437899a..5bb04d0c22998 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, bool /*GenCrashDiag*/) { +static void debug_user_sig_handler(void *Cookie) { // 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/PrettyStackTrace.cpp b/llvm/lib/Support/PrettyStackTrace.cpp index d9ade170e7265..9b09384e95bfe 100644 --- a/llvm/lib/Support/PrettyStackTrace.cpp +++ b/llvm/lib/Support/PrettyStackTrace.cpp @@ -150,10 +150,7 @@ 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 *, bool GenCrashDiag) { - if (!GenCrashDiag) - return; - +static void CrashHandler(void *) { errs() << BugReportMsg ; #ifndef __APPLE__ diff --git a/llvm/lib/Support/Signals.cpp b/llvm/lib/Support/Signals.cpp index dbeba3c2ac8e3..9f9030e79d104 100644 --- a/llvm/lib/Support/Signals.cpp +++ b/llvm/lib/Support/Signals.cpp @@ -95,13 +95,13 @@ CallBacksToRun() { } // Signal-safe. -void sys::RunSignalHandlers(bool GenCrashDiag) { +void sys::RunSignalHandlers() { 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, GenCrashDiag); + (*RunMe.Callback)(RunMe.Cookie); 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 b77a7e024dce5..ccc1d5936133d 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, bool GenCrashDiag) { +static void Cleanup(bool ExecuteSignalHandlers) { if (CleanupExecuted) return; @@ -629,18 +629,18 @@ static void Cleanup(bool ExecuteSignalHandlers, bool GenCrashDiag) { } if (ExecuteSignalHandlers) - llvm::sys::RunSignalHandlers(GenCrashDiag); + llvm::sys::RunSignalHandlers(); LeaveCriticalSection(&CriticalSection); } -void llvm::sys::RunInterruptHandlers(bool GenCrashDiag) { +void llvm::sys::RunInterruptHandlers(bool ExecuteSignalHandlers) { // 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, GenCrashDiag); + Cleanup(ExecuteSignalHandlers); } /// 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(/*ExecuteSignalHandlers=*/true, /*GenCrashDiag=*/false); + Cleanup(/*ExecuteSignalHandlers=*/true); // 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(/*ExecuteSignalHandlers=*/false, /*GenCrashDiag=*/false); + Cleanup(/*ExecuteSignalHandlers=*/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 7f54cf2adba33..d76de85361573 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(/*GenCrashDiag=*/false); + sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/false); std::exit(1); } diff --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp index 831d21a15ef9a..ceafba5b36f11 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 *, bool) { ++GlobalInt; } +static void incrementGlobalWithParam(void *) { ++GlobalInt; } TEST(CrashRecoveryTest, Basic) { llvm::CrashRecoveryContext::Enable(); >From 1125fef55298b2dd76ac4582239a40a2f08d0b65 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 22:45:38 +0100 Subject: [PATCH 5/7] Call RunInterruptHandlers correctly everywhere. --- llvm/lib/TableGen/Error.cpp | 2 +- mlir/tools/mlir-tblgen/OpFormatGen.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/TableGen/Error.cpp b/llvm/lib/TableGen/Error.cpp index d76de85361573..b952c6a7f7bc4 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(/*ExecuteSignalHandlers=*/false); + sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/true); std::exit(1); } diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp index 0a9d14d6603a8..4059571689370 100644 --- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp @@ -3836,7 +3836,7 @@ void mlir::tblgen::generateOpFormat(const Operator &constOp, OpClass &opClass, // Exit the process if format errors are treated as fatal. if (formatErrorIsFatal) { // Invoke the interrupt handlers to run the file cleanup handlers. - llvm::sys::RunInterruptHandlers(); + llvm::sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/true); std::exit(1); } return; >From 3233c0d50029971a878503c103bfee9149ef8f83 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 22:57:57 +0100 Subject: [PATCH 6/7] Minor improvements to lld/test/COFF/lto-cache-errors.ll --- lld/test/COFF/lto-cache-errors.ll | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lld/test/COFF/lto-cache-errors.ll b/lld/test/COFF/lto-cache-errors.ll index 093e66cc67c0c..dd4bed921dc1f 100644 --- a/lld/test/COFF/lto-cache-errors.ll +++ b/lld/test/COFF/lto-cache-errors.ll @@ -1,12 +1,12 @@ -;; The output is OS specific, so make this test specific to Windows. +;; The output is OS-specific, so this test is limited 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 -; 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. +;; Use a 261-character path component that typical filesystems will reject. +;; Use a response file to break 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 @@ -15,10 +15,12 @@ ; 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. +; Check that a fatal usage error is emitted when the cache dir can't be created. +; The --implicit-check-not arguments verify that no crash-style output is shown. ; 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)}} + +; CHECK: LLVM ERROR: can't create cache directory {{.*}}/bob/: invalid argument 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" >From 030edda9a6aaa0d60c9ad801f6c32e5808568adc Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Wed, 21 May 2025 23:20:34 +0100 Subject: [PATCH 7/7] Add a release note for this change. --- llvm/docs/ReleaseNotes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 9c8cc599a8daf..e636f28ab7788 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -80,6 +80,8 @@ Changes to LLVM infrastructure * Added the support for ``fmaximum`` and ``fminimum`` in ``atomicrmw`` instruction. The comparison is expected to match the behavior of ``llvm.maximum.*`` and ``llvm.minimum.*`` respectively. +* On Windows, fatal usage errors no longer produce crash-style diagnostics + such as stack traces or bug report prompts. Changes to building LLVM ------------------------ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
