[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-19 Thread James Henderson via Phabricator via cfe-commits 
jhenderson added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-18 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-18 Thread John Brawn via Phabricator via cfe-commits 
john.brawn updated this revision to Diff 271757.
john.brawn edited the summary of this revision.
john.brawn added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672

Files:
  clang/test/Driver/crash-report-crashfile.m
  clang/test/Driver/crash-report-modules.m
  clang/test/Driver/crash-report-null.test
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/PrettyStackTrace.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/PrettyStackTrace.cpp

Index: llvm/lib/Support/PrettyStackTrace.cpp
===
--- llvm/lib/Support/PrettyStackTrace.cpp
+++ llvm/lib/Support/PrettyStackTrace.cpp
@@ -142,15 +142,9 @@
 static CrashHandlerStringStorage crashHandlerStringStorage;
 #endif
 
-static const char *BugReportMsg =
-"PLEASE submit a bug report to " BUG_REPORT_URL
-" and include the crash backtrace.\n";
-
 /// This callback is run if a fatal signal is delivered to the process, it
 /// prints the pretty stack trace.
 static void CrashHandler(void *) {
-  errs() << BugReportMsg ;
-
 #ifndef __APPLE__
   // On non-apple systems, just emit the crash stack trace to stderr.
   PrintCurStackTrace(errs());
@@ -202,12 +196,6 @@
 
 #endif // ENABLE_BACKTRACES
 
-void llvm::setBugReportMsg(const char *Msg) {
-#if ENABLE_BACKTRACES
-  BugReportMsg = Msg;
-#endif
-}
-
 PrettyStackTraceEntry::PrettyStackTraceEntry() {
 #if ENABLE_BACKTRACES
   // Handle SIGINFO first, because we haven't finished constructing yet.
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -7,11 +7,13 @@
 //===--===//
 
 #include "llvm/Support/CrashRecoveryContext.h"
+#include "llvm/Config/config.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadLocal.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #if LLVM_ON_UNIX
@@ -27,6 +29,10 @@
 static ManagedStatic<
 sys::ThreadLocal > CurrentContext;
 
+static const char *BugReportMsg =
+"PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace.\n";
+
 struct CrashRecoveryContextImpl {
   // When threads are disabled, this links up all active
   // CrashRecoveryContextImpls.  When threads are enabled there's one thread
@@ -71,6 +77,8 @@
 assert(!Failed && "Crash recovery context already failed!");
 Failed = true;
 
+errs() << BugReportMsg;
+
 if (CRC->DumpStackAndCleanupOnFailure)
   sys::CleanupOnSignal(Context);
 
@@ -86,6 +94,10 @@
 };
 }
 
+void llvm::setBugReportMsg(const char *Msg) {
+  BugReportMsg = Msg;
+}
+
 static ManagedStatic gCrashRecoveryContextMutex;
 static bool gCrashRecoveryEnabled = false;
 
Index: llvm/include/llvm/Support/PrettyStackTrace.h
===
--- llvm/include/llvm/Support/PrettyStackTrace.h
+++ llvm/include/llvm/Support/PrettyStackTrace.h
@@ -37,10 +37,6 @@
   /// \see PrettyStackTraceEntry
   void EnablePrettyStackTraceOnSigInfoForThisThread(bool ShouldEnable = true);
 
-  /// Replaces the generic bug report message that is output upon
-  /// a crash.
-  void setBugReportMsg(const char *Msg);
-
   /// PrettyStackTraceEntry - This class is used to represent a frame of the
   /// "pretty" stack trace that is dumped when a program crashes. You can define
   /// subclasses of this and declare them on the program stack: when they are
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -14,6 +14,10 @@
 namespace llvm {
 class CrashRecoveryContextCleanup;
 
+/// Replaces the generic bug report message that is output upon
+/// a crash.
+void setBugReportMsg(const char *Msg);
+
 /// Crash recovery helper object.
 ///
 /// This class implements support for running operations in a safe context so
Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -511,6 +511,10 @@
   for (const auto  : C->getJobs())
 if (const Command *C = dyn_cast())
   FailingCommands.push_back(std::make_pair(-1, C));
+
+  // Crash using abort.
+  llvm::CrashRecoveryContext CRC;
+  CRC.RunSafely([&]() { abort(); });
 }

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-18 Thread John Brawn via Phabricator via cfe-commits 
john.brawn marked an inline comment as done.
john.brawn added inline comments.



Comment at: clang/tools/driver/driver.cpp:518
+  CRC.DumpStackAndCleanupOnFailure = true;
+  CRC.RunSafely([&]() { abort(); });
 }

aganea wrote:
> 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; }`?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-16 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added inline comments.



Comment at: clang/tools/driver/driver.cpp:518
+  CRC.DumpStackAndCleanupOnFailure = true;
+  CRC.RunSafely([&]() { abort(); });
 }

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; }`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-16 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added reviewers: aganea, rnk.
john.brawn added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-15 Thread James Henderson via Phabricator via cfe-commits 
jhenderson added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81672/new/

https://reviews.llvm.org/D81672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

2020-06-11 Thread John Brawn via Phabricator via cfe-commits 
john.brawn created this revision.
john.brawn added reviewers: gbreynoo, jhenderson, probinson.
Herald added a project: clang.

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 -gen-reproducer. Fix 
this by calling abort inside of a CrashRecoveryContext so we go through the 
crash handler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81672

Files:
  clang/test/Driver/crash-report-crashfile.m
  clang/test/Driver/crash-report-modules.m
  clang/test/Driver/crash-report-null.test
  clang/tools/driver/driver.cpp


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -511,6 +511,11 @@
   for (const auto  : C->getJobs())
 if (const Command *C = dyn_cast())
   FailingCommands.push_back(std::make_pair(-1, C));
+
+  // Crash using abort.
+  llvm::CrashRecoveryContext CRC;
+  CRC.DumpStackAndCleanupOnFailure = true;
+  CRC.RunSafely([&]() { abort(); });
 }
 
 for (const auto  : FailingCommands) {
Index: clang/test/Driver/crash-report-null.test
===
--- clang/test/Driver/crash-report-null.test
+++ clang/test/Driver/crash-report-null.test
@@ -3,5 +3,6 @@
 // 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
Index: clang/test/Driver/crash-report-modules.m
===
--- clang/test/Driver/crash-report-modules.m
+++ clang/test/Driver/crash-report-modules.m
@@ -19,6 +19,7 @@
 @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
Index: clang/test/Driver/crash-report-crashfile.m
===
--- clang/test/Driver/crash-report-crashfile.m
+++ clang/test/Driver/crash-report-crashfile.m
@@ -18,6 +18,7 @@
 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
@@ -26,6 +27,7 @@
 // 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


Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -511,6 +511,11 @@
   for (const auto  : C->getJobs())
 if (const Command *C = dyn_cast())
   FailingCommands.push_back(std::make_pair(-1, C));
+
+  // Crash using abort.
+  llvm::CrashRecoveryContext CRC;
+  CRC.DumpStackAndCleanupOnFailure = true;
+  CRC.RunSafely([&]() { abort(); });
 }
 
 for (const auto  : FailingCommands) {
Index: clang/test/Driver/crash-report-null.test
===
--- clang/test/Driver/crash-report-null.test
+++ clang/test/Driver/crash-report-null.test
@@ -3,5 +3,6 @@
 // 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
Index: clang/test/Driver/crash-report-modules.m
===
--- clang/test/Driver/crash-report-modules.m
+++