https://github.com/bd1976bris updated https://github.com/llvm/llvm-project/pull/140956
>From 0605ef796f300f760c62a0f42ae428481847d2b1 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Thu, 22 May 2025 16:49:49 +0100 Subject: [PATCH] [LLVM][Windows] Elide `PrettyStackTrace` output for usage errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, LLVM’s `reportFatalUsageError` (#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: 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/ELF/lto/ltopasses-custom.ll | 12 ++++++++---- llvm/docs/ReleaseNotes.md | 3 +++ llvm/include/llvm/Support/Signals.h | 2 +- llvm/lib/LTO/LTOBackend.cpp | 9 +++++---- llvm/lib/Support/ErrorHandling.cpp | 2 +- llvm/lib/Support/Unix/Signals.inc | 4 +++- llvm/lib/Support/Windows/Signals.inc | 8 ++++---- llvm/lib/TableGen/Error.cpp | 2 +- llvm/test/tools/llvm-lto2/X86/pipeline.ll | 4 ++-- mlir/tools/mlir-tblgen/OpFormatGen.cpp | 2 +- 12 files changed, 31 insertions(+), 21 deletions(-) diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp index 7af8e4f25d99e..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(); + 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 6638a15ff7e12..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(); + 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/lld/test/ELF/lto/ltopasses-custom.ll b/lld/test/ELF/lto/ltopasses-custom.ll index e37083ca8b8c7..3e680dbb59da3 100644 --- a/lld/test/ELF/lto/ltopasses-custom.ll +++ b/lld/test/ELF/lto/ltopasses-custom.ll @@ -24,14 +24,18 @@ define void @barrier() { ; ATOMIC-NEXT: ret void ; Check that invalid passes are rejected gracefully. -; RUN: env LLD_IN_TEST=1 not --crash ld.lld -m elf_x86_64 %t.o -o /dev/null \ +; The --implicit-check-not arguments verify that no crash-style output is shown. +; RUN: env LLD_IN_TEST=1 not ld.lld -m elf_x86_64 %t.o -o /dev/null \ ; RUN: --lto-newpm-passes=iamnotapass -shared 2>&1 | \ -; RUN: FileCheck %s --check-prefix=INVALID +; RUN: FileCheck %s --check-prefix=INVALID \ +; RUN: --ignore-case --implicit-check-not=bug --implicit-check-not=crash ; INVALID: unable to parse pass pipeline description 'iamnotapass': unknown pass name 'iamnotapass' ; Check that invalid AA pipelines are rejected gracefully. -; RUN: env LLD_IN_TEST=1 not --crash ld.lld -m elf_x86_64 %t.o -o /dev/null \ +; The --implicit-check-not arguments verify that no crash-style output is shown. +; RUN: env LLD_IN_TEST=1 not ld.lld -m elf_x86_64 %t.o -o /dev/null \ ; RUN: --lto-newpm-passes=globaldce --lto-aa-pipeline=patatino \ ; RUN: -shared 2>&1 | \ -; RUN: FileCheck %s --check-prefix=INVALIDAA +; RUN: FileCheck %s --check-prefix=INVALIDAA \ +; RUN: --ignore-case --implicit-check-not=bug --implicit-check-not=crash ; INVALIDAA: unknown alias analysis name 'patatino' diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 2f6d90cdc589f..2ebaa7d13a6c8 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -80,6 +80,9 @@ 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 invoke LLVM's signal handlers. + As a result, the default "please report a bug" message from the `PrettyStackTrace` + signal handler is no longer emitted. Changes to building LLVM ------------------------ diff --git a/llvm/include/llvm/Support/Signals.h b/llvm/include/llvm/Support/Signals.h index 6ce26acdd458e..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(); +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. diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index b7db70b99bcbc..14398829fea53 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -290,8 +290,8 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM, if (!Conf.AAPipeline.empty()) { AAManager AA; if (auto Err = PB.parseAAPipeline(AA, Conf.AAPipeline)) { - report_fatal_error(Twine("unable to parse AA pipeline description '") + - Conf.AAPipeline + "': " + toString(std::move(Err))); + reportFatalUsageError(Twine("unable to parse AA pipeline description '") + + Conf.AAPipeline + "': " + toString(std::move(Err))); } // Register the AA manager first so that our version is the one used. FAM.registerPass([&] { return std::move(AA); }); @@ -331,8 +331,9 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM, // Parse a custom pipeline if asked to. if (!Conf.OptPipeline.empty()) { if (auto Err = PB.parsePassPipeline(MPM, Conf.OptPipeline)) { - report_fatal_error(Twine("unable to parse pass pipeline description '") + - Conf.OptPipeline + "': " + toString(std::move(Err))); + reportFatalUsageError( + Twine("unable to parse pass pipeline description '") + + Conf.OptPipeline + "': " + toString(std::move(Err))); } } else if (IsThinLTO) { MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary)); diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp index cc16f2037ea58..17a08dc2d2c01 100644 --- a/llvm/lib/Support/ErrorHandling.cpp +++ b/llvm/lib/Support/ErrorHandling.cpp @@ -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(GenCrashDiag); if (GenCrashDiag) abort(); diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc index 6668a2953b3b2..9065e1068156a 100644 --- a/llvm/lib/Support/Unix/Signals.inc +++ b/llvm/lib/Support/Unix/Signals.inc @@ -425,7 +425,9 @@ static void InfoSignalHandler(int Sig) { CurrentInfoFunction(); } -void llvm::sys::RunInterruptHandlers() { RemoveFilesToRemove(); } +void llvm::sys::RunInterruptHandlers(bool /*ExecuteSignalHandlers*/) { + RemoveFilesToRemove(); +} void llvm::sys::SetInterruptFunction(void (*IF)()) { InterruptFunction.exchange(IF); diff --git a/llvm/lib/Support/Windows/Signals.inc b/llvm/lib/Support/Windows/Signals.inc index f11ad09f37139..ccc1d5936133d 100644 --- a/llvm/lib/Support/Windows/Signals.inc +++ b/llvm/lib/Support/Windows/Signals.inc @@ -634,13 +634,13 @@ static void Cleanup(bool ExecuteSignalHandlers) { LeaveCriticalSection(&CriticalSection); } -void llvm::sys::RunInterruptHandlers() { +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); + 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(true); + 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(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 91423664a84cc..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(); + sys::RunInterruptHandlers(/*ExecuteSignalHandlers=*/true); std::exit(1); } diff --git a/llvm/test/tools/llvm-lto2/X86/pipeline.ll b/llvm/test/tools/llvm-lto2/X86/pipeline.ll index 93f30d0ee61bc..9416ac5fd9fff 100644 --- a/llvm/test/tools/llvm-lto2/X86/pipeline.ll +++ b/llvm/test/tools/llvm-lto2/X86/pipeline.ll @@ -28,13 +28,13 @@ define void @patatino() { ; CUSTOM-NEXT: } ; Check that invalid pipelines are caught as errors. -; RUN: not --crash llvm-lto2 run %t1.bc -o %t.o \ +; RUN: not llvm-lto2 run %t1.bc -o %t.o \ ; RUN: -r %t1.bc,patatino,px -opt-pipeline foogoo 2>&1 | \ ; RUN: FileCheck %s --check-prefix=ERR ; ERR: LLVM ERROR: unable to parse pass pipeline description 'foogoo': unknown pass name 'foogoo' -; RUN: not --crash llvm-lto2 run %t1.bc -o %t.o \ +; RUN: not llvm-lto2 run %t1.bc -o %t.o \ ; RUN: -r %t1.bc,patatino,px -aa-pipeline patatino \ ; RUN: -opt-pipeline lower-atomic 2>&1 | \ ; RUN: FileCheck %s --check-prefix=AAERR 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
