[Driver] When forcing a crash print the bug report message

[john.brawn]

Summary

Commit a945037e8fd0c30e250a62211469eea6765a36ae moved the printing of the "PLEASE submit a bug report" message to the crash handler, but that means we don't print it when forcing a crash using FORCE_CLANG_DIAGNOSTICS_CRASH. Fix this by adding a function to get the bug report message and printing it when forcing a crash.

[jhenderson]

This seems sensible to me, but I have zero experience with the CrashRecoveryContext class. It might be best to get someone who has more knowledge of it to look at it.

[john.brawn]

Added a couple of reviewers that have recently worked on CrashRecoveryContext.

[aganea]

clang/tools/driver/driver.cpp

518

The only concern I have is that a unrelated call stack will be printed.
Could you possibly add (and use here) a function along the lines of emitBugReportMsg() { errs() << BugReportMsg; }?

[john.brawn]

clang/tools/driver/driver.cpp

518

I had a go at doing that, but then realised that currently the bug report message only exists, and is only printed, when llvm is built with LLVM_ENABLE_BACKTRACES=ON which I don't think is what we want. I've instead adjusted things so that the bug report message is printed in CrashRecoveryContext instead of in the backtrace handler, which also means we can get the message without the backtrace here.

[john.brawn]

Moved BugReportMsg printing into CrashRecoveryContext, and stopped printing of backtrace when forcing a crash.

[aganea]

But that won't work when compiling & crashing with -fno-integrated-cc1, would it? (or if building with cmake ... -DCLANG_SPAWN_CC1=1). In that case, normal crashes (not -gen-reproducer) won't go through the CrashRecoveryContext.
Try running:

$ CCC_OVERRIDE_OPTIONS=+-fno-integrated-cc1
$ py your_build_folder/bin/llvm-lit.py -vv -a clang/test/Driver/crash-report.c

See if the commands issued by the test display the "PLEASE submit a bug report" message.

[jhenderson]

In D81672#2101513, @aganea wrote:

But that won't work when compiling & crashing with -fno-integrated-cc1, would it? (or if building with cmake ... -DCLANG_SPAWN_CC1=1). In that case, normal crashes (not -gen-reproducer) won't go through the CrashRecoveryContext.
Try running:

$ CCC_OVERRIDE_OPTIONS=+-fno-integrated-cc1
$ py your_build_folder/bin/llvm-lit.py -vv -a clang/test/Driver/crash-report.c

See if the commands issued by the test display the "PLEASE submit a bug report" message.

If I'm following it correctly (I could easily not be), the latest version of the patch will also no longer print the bug report submission message for crashes in other parts of LLVM, not just clang, which was rather the motivation of @gbreynoo's recent changes.

[john.brawn]

Print the bug report message when forcing a crash, but also have the message available even with LLVM_ENABLE_BACKTRACES=OFF.

[MaskRay]

llvm/lib/Support/PrettyStackTrace.cpp

36 ↗

(On Diff #272471)

This variable is mutable. Please use const char BugReportMsg[]

[jhenderson]

LGTM, but wait for others too, please.

[john.brawn]

llvm/lib/Support/PrettyStackTrace.cpp

36 ↗

(On Diff #272471)

It needs to be mutable because it is set by llvm::setBugReportMsg.

[MaskRay]

clang/tools/driver/driver.cpp

515

Why llvm::dbgs() << llvm::getBugReportMsg(); when -gen-reproducer is specified? The user requests to generate a reproduce file, but this does not suggest that clang has a bug. Dumping an URL is not very appropriate.

[john.brawn]

clang/tools/driver/driver.cpp

515

Emitting the bug report message is primarily for when FORCE_CLANG_DIAGNOSTICS_CRASH is set (because we can get here either due to that or -gen-reproducer). I could adjust this to only emit the message when that is set if you'd like? Though either way we get the "PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:" message.

[MaskRay]

clang/tools/driver/driver.cpp

515

When FORCE_CLANG_DIAGNOSTICS_CRASH is set or -gen-reproducer is specified, I feel that a URL or "PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT" is not very appropriate because this is not a real crash......

[john.brawn]

clang/tools/driver/driver.cpp

515

FORCE_CLANG_DIAGNOSTICS_CRASH exists exactly to test the diagnostics (at least according to 681e4b8d962ed848d2bd443642e53909af53a6cd which added it) so we should be emitting it them. I'll adjust it to not do so when using -gen-reproducer.

[john.brawn]

Don't print the diagnostic with -gen-reproducer.

[MaskRay]

Thanks!

[jhenderson]

Latest version LGTM too.

clang/test/Driver/crash-report-crashfile.m

 // RUN:   -I %S/Inputs/module -isysroot %/t/i/ \
 // RUN:   -fmodules -fmodules-cache-path=%t/m/ -DFOO=BAR 2>&1 | \
 // RUN:   FileCheck -check-prefix=CRASH_FLAG %s
 
 @import simple;
 const int x = MODULE_MACRO;
 
 // CRASH_ENV: failing because environment variable 'FORCE_CLANG_DIAGNOSTICS_CRASH' is set
+// CRASH_ENV: PLEASE submit a bug report to {{.*}} and include the crash backtrace, preprocessed source, and associated run script.
 // CRASH_ENV: Preprocessed source(s) and associated run script(s) are located at:
 // CRASH_ENV-NEXT: note: diagnostic msg: {{.*}}.m
 // CRASH_ENV-NEXT: note: diagnostic msg: {{.*}}.cache
 // CRASH_ENV-NEXT: note: diagnostic msg: {{.*}}.sh
 // CRASH_ENV-NEXT: note: diagnostic msg: Crash backtrace is located in
 // CRASH_ENV-NEXT: note: diagnostic msg: {{.*}}Library/Logs/DiagnosticReports{{.*}}
 
 // CRASH_FLAG: failing because '-gen-reproducer' is used
+// CRASH_FLAG: PLEASE submit a bug report to {{.*}} and include the crash backtrace, preprocessed source, and associated run script.
 // CRASH_FLAG: Preprocessed source(s) and associated run script(s) are located at:
 // CRASH_FLAG-NEXT: note: diagnostic msg: {{.*}}.m
 // CRASH_FLAG-NEXT: note: diagnostic msg: {{.*}}.cache
 // CRASH_FLAG-NEXT: note: diagnostic msg: {{.*}}.sh
 // CRASH_FLAG-NEXT: note: diagnostic msg: Crash backtrace is located in
 // CRASH_FLAG-NEXT: note: diagnostic msg: {{.*}}Library/Logs/DiagnosticReports{{.*}}

clang/test/Driver/crash-report-modules.m

 
 // FIXME: This test creates excessively deep directory hierarchies that cause
 // problems on Windows.
 // UNSUPPORTED: system-windows
 
 @import simple;
 const int x = MODULE_MACRO;
 
+// CHECK: PLEASE submit a bug report to {{.*}} and include the crash backtrace, preprocessed source, and associated run script.
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
 // CHECK-NEXT: note: diagnostic msg: {{.*}}.m
 // CHECK-NEXT: note: diagnostic msg: {{.*}}.cache
 
 // CHECKSRC: @import simple;
 // CHECKSRC: const int x = 10;
 
 // CHECKSH: # Crash reproducer

clang/test/Driver/crash-report-null.test

 // RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH=1 not %clang -fsyntax-only -x c /dev/null -lstdc++ 2>&1 | FileCheck %s
 
 // FIXME: Investigating. "fatal error: file 'nul' modified since it was first processed"
 // XFAIL: windows-gnu
 
+// CHECK: PLEASE submit a bug report to {{.*}} and include the crash backtrace, preprocessed source, and associated run script.
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
 // CHECK-NEXT: note: diagnostic msg: {{.*}}null-{{.*}}.c

clang/tools/driver/driver.cpp

if (TheDriver.GenReproducer) {
       Diags.Report(diag::err_drv_force_crash)
         << !::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH");
 
       // Pretend that every command failed.
       FailingCommands.clear();
       for (const auto &J : C->getJobs())
         if (const Command *C = dyn_cast<Command>(&J))
           FailingCommands.push_back(std::make_pair(-1, C));
+
+      // Crash using abort.
+      llvm::CrashRecoveryContext CRC;
+      CRC.DumpStackAndCleanupOnFailure = true;
+      CRC.RunSafely([&]() { abort(); });
     }
 
     for (const auto &P : FailingCommands) {
       int CommandRes = P.first;
       const Command *FailingCommand = P.second;
       if (!Res)
         Res = CommandRes;