https://github.com/bd1976bris updated https://github.com/llvm/llvm-project/pull/140956
>From 23b762fa8adae76ffe7e85a328aef8710030d831 Mon Sep 17 00:00:00 2001 From: Dunbobbin <ben.dunbob...@sony.com> Date: Mon, 19 May 2025 10:17:00 +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` may emit a stack trace and 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 a stack trace and 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, the existing Linux-specific LTO cache directory test has been replaced with a Windows-only test case that also 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 | 25 ++++++++++++++++++------- llvm/docs/ReleaseNotes.md | 2 ++ llvm/include/llvm/Support/Error.h | 2 +- llvm/include/llvm/Support/Signals.h | 2 +- llvm/lib/Support/Error.cpp | 1 + llvm/lib/Support/ErrorHandling.cpp | 19 ++++++++++++------- llvm/lib/Support/Windows/Signals.inc | 8 ++++---- llvm/lib/TableGen/Error.cpp | 2 +- mlir/tools/mlir-tblgen/OpFormatGen.cpp | 2 +- 11 files changed, 43 insertions(+), 24 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/COFF/lto-cache-errors.ll b/lld/test/COFF/lto-cache-errors.ll index 44719e239d989..dd4bed921dc1f 100644 --- a/lld/test/COFF/lto-cache-errors.ll +++ b/lld/test/COFF/lto-cache-errors.ll @@ -1,15 +1,26 @@ -; 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. +; 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 fatal usage error emitted when the cache dir can't be created. -; RUN: not 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 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 +; 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 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 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/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 2f6d90cdc589f..c19721a6706de 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 ------------------------ diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index 9bb952ceb0dc2..d42fb30079c8d 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -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); /// Report a fatal error that indicates a bug in LLVM. /// See ErrorHandling.h for details. 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/Support/Error.cpp b/llvm/lib/Support/Error.cpp index d168b462a6eb2..677f0a7acfebf 100644 --- a/llvm/lib/Support/Error.cpp +++ b/llvm/lib/Support/Error.cpp @@ -177,6 +177,7 @@ void report_fatal_error(Error Err, bool GenCrashDiag) { void reportFatalInternalError(Error Err) { report_fatal_error(std::move(Err), /*GenCrashDiag=*/true); } + void reportFatalUsageError(Error Err) { report_fatal_error(std::move(Err), /*GenCrashDiag=*/false); } diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp index cc16f2037ea58..3d42b9aae733f 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(); @@ -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); } + void llvm::reportFatalInternalError(StringRef reason) { - report_fatal_error(reason, /*GenCrashDiag=*/true); + report_fatal_error(reason, /*gen_crash_diag=*/true); } + void llvm::reportFatalInternalError(const Twine &reason) { - report_fatal_error(reason, /*GenCrashDiag=*/true); + report_fatal_error(reason, /*gen_crash_diag=*/true); } + void llvm::reportFatalUsageError(const char *reason) { - report_fatal_error(reason, /*GenCrashDiag=*/false); + report_fatal_error(reason, /*gen_crash_diag=*/false); } + void llvm::reportFatalUsageError(StringRef reason) { - report_fatal_error(reason, /*GenCrashDiag=*/false); + report_fatal_error(reason, /*gen_crash_diag=*/false); } + void llvm::reportFatalUsageError(const Twine &reason) { - report_fatal_error(reason, /*GenCrashDiag=*/false); + 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/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/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
