aganea created this revision.
aganea added reviewers: arichardson, rnk, hans.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
As reported by @arichardson , this patch now makes `llvm::report_fatal_error()`
to generate a preprocessed source + reproducer script again. Tested
with/without the cmake flag CLANG_SPAWN_CC1. Tested in various configs,
Debug/Release, with MSVC 19.24 or clang-cl 9.0.1, on Windows and Linux (with
both gcc 9.2.1 and clang 9.0).
1. Added a CC1 flag, `-disable-pragma-debug-crash` to give a chance the crash
diagnostic to generate a preprocessed output. Without this flag, the process
started by `Driver::generateCompilationDiagnostics()` would fail execution in
some cases, when `#pragma clang __debug` is used in the input.
2. Some CC1 M_group flags weren't cleared for diagnostics, which led to
dangling files on the disk, after the crash diagnostics generated the
preprocessed output. Such example is when `-MF file.d` was provided in the
original cmd-line. The driver would cleanup the generated files upon a crash,
but then the crash diagnostic process would re-create the file again, and leave
it there.
3. Added a test for `#pragma clang __debug llvm_fatal_error` to test for the
original issue.
4. Added `llvm::sys::Process::Exit()` and replaced `exit()` in places where it
was appropriate. This new function would call the current
`CrashRecoveryContext` if one is running on the same thread; or call `exit()`
otherwise.
Evidently I can cut the patch in smaller pieces, let me know.
This fixes PR44705.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73742
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/Driver/Compilation.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Lex/Pragma.cpp
clang/test/Driver/crash-report.c
clang/tools/driver/cc1_main.cpp
llvm/include/llvm/Support/CrashRecoveryContext.h
llvm/include/llvm/Support/Process.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/ErrorHandling.cpp
llvm/lib/Support/Process.cpp
Index: llvm/lib/Support/Process.cpp
===================================================================
--- llvm/lib/Support/Process.cpp
+++ llvm/lib/Support/Process.cpp
@@ -13,8 +13,9 @@
#include "llvm/Support/Process.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
-#include "llvm/Config/llvm-config.h"
#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
@@ -88,6 +89,13 @@
bool Process::AreCoreFilesPrevented() { return coreFilesPrevented; }
+LLVM_ATTRIBUTE_NORETURN
+void Process::Exit(int RetCode) {
+ if (CrashRecoveryContext *CRC = CrashRecoveryContext::GetCurrent())
+ CRC->HandleExit(RetCode);
+ ::exit(RetCode);
+}
+
// Include the platform-specific parts of this class.
#ifdef LLVM_ON_UNIX
#include "Unix/Process.inc"
Index: llvm/lib/Support/ErrorHandling.cpp
===================================================================
--- llvm/lib/Support/ErrorHandling.cpp
+++ llvm/lib/Support/ErrorHandling.cpp
@@ -19,6 +19,7 @@
#include "llvm/Support/Debug.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/WindowsError.h"
@@ -122,7 +123,7 @@
// files registered with RemoveFileOnSignal.
sys::RunInterruptHandlers();
- exit(1);
+ sys::Process::Exit(1);
}
void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler,
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===================================================================
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -15,7 +15,7 @@
#include <mutex>
#include <setjmp.h>
#ifdef _WIN32
-#include <excpt.h> // for GetExceptionInformation
+#include <windows.h>
#endif
#if LLVM_ON_UNIX
#include <sysexits.h> // EX_IOERR
@@ -41,11 +41,11 @@
::jmp_buf JumpBuffer;
volatile unsigned Failed : 1;
unsigned SwitchedThread : 1;
+ unsigned ValidJumpBuffer : 1;
public:
- CrashRecoveryContextImpl(CrashRecoveryContext *CRC) : CRC(CRC),
- Failed(false),
- SwitchedThread(false) {
+ CrashRecoveryContextImpl(CrashRecoveryContext *CRC)
+ : CRC(CRC), Failed(false), SwitchedThread(false), ValidJumpBuffer(false) {
Next = CurrentContext->get();
CurrentContext->set(this);
}
@@ -80,7 +80,11 @@
CRC->RetCode = RetCode;
// Jump back to the RunSafely we were called under.
- longjmp(JumpBuffer, 1);
+ if (ValidJumpBuffer)
+ longjmp(JumpBuffer, 1);
+
+ // Otherwise let the caller decide of the outcome of the crash. Currently
+ // this occurs when using SEH on Windows with MSVC or clang-cl.
}
};
@@ -186,31 +190,33 @@
static void installExceptionOrSignalHandlers() {}
static void uninstallExceptionOrSignalHandlers() {}
-// We need this function because the call to GetExceptionInformation() can only
-// occur inside the __except evaluation block
-static int ExceptionFilter(bool DumpStackAndCleanup,
- _EXCEPTION_POINTERS *Except) {
- if (DumpStackAndCleanup)
- sys::CleanupOnSignal((uintptr_t)Except);
- return EXCEPTION_EXECUTE_HANDLER;
-}
-
#if defined(__clang__) && defined(_M_IX86)
// Work around PR44697.
__attribute__((optnone))
-static bool InvokeFunctionCall(function_ref<void()> Fn,
- bool DumpStackAndCleanup, int &RetCode) {
-#else
-static bool InvokeFunctionCall(function_ref<void()> Fn,
- bool DumpStackAndCleanup, int &RetCode) {
#endif
- __try {
- Fn();
- } __except (ExceptionFilter(DumpStackAndCleanup, GetExceptionInformation())) {
- RetCode = GetExceptionCode();
- return false;
+// We need this function because the call to GetExceptionInformation() can only
+// occur inside the __except evaluation block
+static int
+ExceptionFilter(_EXCEPTION_POINTERS *Except) {
+ // Lookup the current thread local recovery object.
+ const CrashRecoveryContextImpl *CRCI = CurrentContext->get();
+
+ if (!CRCI) {
+ // Something has gone horribly wrong, so let's just tell everyone
+ // to keep searching
+ CrashRecoveryContext::Disable();
+ return EXCEPTION_CONTINUE_SEARCH;
}
- return true;
+
+ int RetCode = (int)Except->ExceptionRecord->ExceptionCode;
+ if ((RetCode & 0xF0000000) == 0xE0000000)
+ RetCode &= ~0xF0000000; // this crash was generated by sys::Process::Exit
+
+ // Handle the crash
+ const_cast<CrashRecoveryContextImpl *>(CRCI)->HandleCrash(
+ RetCode, reinterpret_cast<uintptr_t>(Except));
+
+ return EXCEPTION_EXECUTE_HANDLER;
}
bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
@@ -218,7 +224,15 @@
Fn();
return true;
}
- return InvokeFunctionCall(Fn, DumpStackAndCleanupOnFailure, RetCode);
+ assert(!Impl && "Crash recovery context already initialized!");
+ Impl = new CrashRecoveryContextImpl(this);
+
+ __try {
+ Fn();
+ } __except (ExceptionFilter(GetExceptionInformation())) {
+ return false;
+ }
+ return true;
}
#else // !_MSC_VER
@@ -395,6 +409,7 @@
CrashRecoveryContextImpl *CRCI = new CrashRecoveryContextImpl(this);
Impl = CRCI;
+ CRCI->ValidJumpBuffer = true;
if (setjmp(CRCI->JumpBuffer) != 0) {
return false;
}
@@ -406,12 +421,18 @@
#endif // !_MSC_VER
-void CrashRecoveryContext::HandleCrash() {
- CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl;
+LLVM_ATTRIBUTE_NORETURN
+void CrashRecoveryContext::HandleExit(int RetCode) {
+#if defined(_WIN32)
+ ::RaiseException(0xE0000000 | RetCode, 0, 0, NULL);
+#else
+ CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *)Impl;
assert(CRCI && "Crash recovery context never initialized!");
// As per convention, -2 indicates a crash or timeout as opposed to failure to
// execute (see llvm/include/llvm/Support/Program.h)
CRCI->HandleCrash(-2, 0);
+#endif
+ llvm_unreachable("Most likely setjmp wasn't called!");
}
// FIXME: Portability.
Index: llvm/include/llvm/Support/Process.h
===================================================================
--- llvm/include/llvm/Support/Process.h
+++ llvm/include/llvm/Support/Process.h
@@ -201,6 +201,12 @@
/// Get the result of a process wide random number generator. The
/// generator will be automatically seeded in non-deterministic fashion.
static unsigned GetRandomNumber();
+
+ /// Terminate the current program if integrated-cc1 is disabled.
+ /// When integrated-cc1 is enabled, this terminates execution of the current
+ /// cc1 tool.
+ LLVM_ATTRIBUTE_NORETURN
+ static void Exit(int RetCode);
};
}
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===================================================================
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -99,7 +99,8 @@
/// Explicitly trigger a crash recovery in the current process, and
/// return failure from RunSafely(). This function does not return.
- void HandleCrash();
+ LLVM_ATTRIBUTE_NORETURN
+ void HandleExit(int RetCode);
/// In case of a crash, this is the crash identifier.
int RetCode = 0;
Index: clang/tools/driver/cc1_main.cpp
===================================================================
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -36,6 +36,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/TargetRegistry.h"
#include "llvm/Support/TargetSelect.h"
@@ -69,7 +70,7 @@
// We cannot recover from llvm errors. When reporting a fatal error, exit
// with status 70 to generate crash diagnostics. For BSD systems this is
// defined as an internal software error. Otherwise, exit with status 1.
- exit(GenCrashDiag ? 70 : 1);
+ llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1);
}
#ifdef CLANG_HAVE_RLIMITS
Index: clang/test/Driver/crash-report.c
===================================================================
--- clang/test/Driver/crash-report.c
+++ clang/test/Driver/crash-report.c
@@ -7,13 +7,33 @@
// RUN: -iprefix /the/prefix -iwithprefix /tmp -iwithprefixbefore /tmp/ \
// RUN: -Xclang -internal-isystem -Xclang /tmp/ \
// RUN: -Xclang -internal-externc-isystem -Xclang /tmp/ \
-// RUN: -Xclang -main-file-name -Xclang foo.c \
+// RUN: -Xclang -main-file-name -Xclang foo.c -DCRASH \
// RUN: -DFOO=BAR -DBAR="BAZ QUX" 2>&1 | FileCheck %s
// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1 \
+// RUN: CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
+// RUN: not %clang -fsyntax-only %s \
+// RUN: -F/tmp/ -I /tmp/ -idirafter /tmp/ -iquote /tmp/ -isystem /tmp/ \
+// RUN: -iprefix /the/prefix -iwithprefix /tmp -iwithprefixbefore /tmp/ \
+// RUN: -Xclang -internal-isystem -Xclang /tmp/ \
+// RUN: -Xclang -internal-externc-isystem -Xclang /tmp/ \
+// RUN: -Xclang -main-file-name -Xclang foo.c \
+// RUN: -DFOO=BAR -DBAR="BAZ QUX" 2>&1 | FileCheck %s
+// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
+// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
+
// REQUIRES: crash-recovery
+#ifdef CRASH
#pragma clang __debug parser_crash
+#else
+#pragma clang __debug llvm_fatal_error
+#endif
+
// CHECK: Preprocessed source(s) and associated run script(s) are located at:
// CHECK-NEXT: note: diagnostic msg: {{.*}}crash-report-{{.*}}.c
FOO
Index: clang/lib/Lex/Pragma.cpp
===================================================================
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -30,6 +30,7 @@
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Lex/Token.h"
#include "clang/Lex/TokenLexer.h"
#include "llvm/ADT/ArrayRef.h"
@@ -37,10 +38,10 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/CrashRecoveryContext.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/ErrorHandling.h"
#include <algorithm>
#include <cassert>
@@ -1035,9 +1036,11 @@
IdentifierInfo *II = Tok.getIdentifierInfo();
if (II->isStr("assert")) {
- llvm_unreachable("This is an assertion!");
+ if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash)
+ llvm_unreachable("This is an assertion!");
} else if (II->isStr("crash")) {
- LLVM_BUILTIN_TRAP;
+ if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash)
+ LLVM_BUILTIN_TRAP;
} else if (II->isStr("parser_crash")) {
Token Crasher;
Crasher.startToken();
@@ -1075,9 +1078,11 @@
<< II->getName();
}
} else if (II->isStr("llvm_fatal_error")) {
- llvm::report_fatal_error("#pragma clang __debug llvm_fatal_error");
+ if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash)
+ llvm::report_fatal_error("#pragma clang __debug llvm_fatal_error");
} else if (II->isStr("llvm_unreachable")) {
- llvm_unreachable("#pragma clang __debug llvm_unreachable");
+ if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash)
+ llvm_unreachable("#pragma clang __debug llvm_unreachable");
} else if (II->isStr("macro")) {
Token MacroName;
PP.LexUnexpandedToken(MacroName);
@@ -1104,11 +1109,8 @@
}
M->dump();
} else if (II->isStr("overflow_stack")) {
- DebugOverflowStack();
- } else if (II->isStr("handle_crash")) {
- llvm::CrashRecoveryContext *CRC =llvm::CrashRecoveryContext::GetCurrent();
- if (CRC)
- CRC->HandleCrash();
+ if (!PP.getPreprocessorOpts().DisablePragmaDebugCrash)
+ DebugOverflowStack();
} else if (II->isStr("captured")) {
HandleCaptured(PP);
} else {
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3469,6 +3469,7 @@
Opts.LexEditorPlaceholders = false;
Opts.SetUpStaticAnalyzer = Args.hasArg(OPT_setup_static_analyzer);
+ Opts.DisablePragmaDebugCrash = Args.hasArg(OPT_disable_pragma_debug_crash);
}
static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions &Opts,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4713,6 +4713,10 @@
: "-");
}
+ // Give the gen diagnostics more chances to succeed.
+ if (D.CCGenDiagnostics)
+ CmdArgs.push_back("-disable-pragma-debug-crash");
+
bool UseSeparateSections = isUseSeparateSections(Triple);
if (Args.hasFlag(options::OPT_ffunction_sections,
Index: clang/lib/Driver/Compilation.cpp
===================================================================
--- clang/lib/Driver/Compilation.cpp
+++ clang/lib/Driver/Compilation.cpp
@@ -258,14 +258,23 @@
// Remove any user specified output. Claim any unclaimed arguments, so as
// to avoid emitting warnings about unused args.
- OptSpecifier OutputOpts[] = { options::OPT_o, options::OPT_MD,
- options::OPT_MMD };
+ OptSpecifier OutputOpts[] = {
+ options::OPT_o, options::OPT_MD, options::OPT_MMD, options::OPT_M,
+ options::OPT_MM, options::OPT_MF, options::OPT_MG, options::OPT_MJ,
+ options::OPT_MQ, options::OPT_MT, options::OPT_MV};
for (unsigned i = 0, e = llvm::array_lengthof(OutputOpts); i != e; ++i) {
if (TranslatedArgs->hasArg(OutputOpts[i]))
TranslatedArgs->eraseArg(OutputOpts[i]);
}
TranslatedArgs->ClaimAllArgs();
+ // Force re-creation of the toolchain Args, otherwise our modifications just
+ // above will have no effect.
+ for (auto Arg : TCArgs)
+ if (Arg.second != TranslatedArgs)
+ delete Arg.second;
+ TCArgs.clear();
+
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
Index: clang/include/clang/Lex/PreprocessorOptions.h
===================================================================
--- clang/include/clang/Lex/PreprocessorOptions.h
+++ clang/include/clang/Lex/PreprocessorOptions.h
@@ -189,6 +189,9 @@
/// Set up preprocessor for RunAnalysis action.
bool SetUpStaticAnalyzer = false;
+ /// Prevents side-effects of #pragma clang __debug. Mainly used for testing.
+ bool DisablePragmaDebugCrash = false;
+
public:
PreprocessorOptions() : PrecompiledPreambleBytes(0, false) {}
Index: clang/include/clang/Driver/CC1Options.td
===================================================================
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -862,6 +862,8 @@
HelpText<"include a detailed record of preprocessing actions">;
def setup_static_analyzer : Flag<["-"], "setup-static-analyzer">,
HelpText<"Set up preprocessor for static analyzer (done automatically when static analyzer is run).">;
+def disable_pragma_debug_crash : Flag<["-"], "disable-pragma-debug-crash">,
+ HelpText<"Disable any #pragma clang __debug that can lead to crashing behavior. This is meant for testing.">;
//===----------------------------------------------------------------------===//
// OpenCL Options
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
