[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-12 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

In D73742#1871037 , @smeenai wrote:

> In D73742#1871012 , @aganea wrote:
>
> > In D73742#1870961 , @smeenai wrote:
> >
> > > I'm assuming this needs to be picked to 10.0?
> >
> >
> > Yes! Is it up to the authors to integrate their patches to the release 
> > branch? I'm seeing @hans is merging a lot of the patches.
>
>
> I believe @hans takes care of the merging; I just wanted to make sure this 
> hadn't been missed :) I'm gonna assume this is on his radar.


Yup :) Pushed to 10.x as fd04cb43e1d83c6f18c932de94c1e341272ed160 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-11 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai added a comment.

In D73742#1871012 , @aganea wrote:

> In D73742#1870961 , @smeenai wrote:
>
> > I'm assuming this needs to be picked to 10.0?
>
>
> Yes! Is it up to the authors to integrate their patches to the release 
> branch? I'm seeing @hans is merging a lot of the patches.


I believe @hans takes care of the merging; I just wanted to make sure this 
hadn't been missed :) I'm gonna assume this is on his radar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea marked an inline comment as done.
aganea added a comment.

In D73742#1870961 , @smeenai wrote:

> I'm assuming this needs to be picked to 10.0?


Yes! Is it up to the authors to integrate their patches to the release branch? 
I'm seeing @hans is merging a lot of the patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-11 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai added a comment.

I'm assuming this needs to be picked to 10.0?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea marked 2 inline comments as done.
aganea added a comment.

Side-note: Perhaps we would need to do the same as this patch, for calls to 
`::abort()`?




Comment at: llvm/include/llvm/Support/Process.h:205
+
+  /// When integrated-cc1 is disabled, terminate the current program.
+  /// When integrated-cc1 is enabled, terminates execution of the current

hans wrote:
> Could the comment be phrased in terms of CrashRecoveryContext instead of 
> integrated-cc1, which is a Clang thing?
Done in the commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfaace365088a: [Clang][Driver] After default 
-fintegrated-cc1, make llvm::report_fatal_error()… (authored by aganea).

Changed prior to commit:
  https://reviews.llvm.org/D73742?vs=243380=243863#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742

Files:
  clang/test/Driver/crash-report.c
  clang/tools/driver/cc1_main.cpp
  clang/tools/driver/cc1as_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
@@ -14,9 +14,6 @@
 #include "llvm/Support/ThreadLocal.h"
 #include 
 #include 
-#ifdef _WIN32
-#include  // for GetExceptionInformation
-#endif
 #if LLVM_ON_UNIX
 #include  // EX_IOERR
 #endif
@@ -178,6 +175,9 @@
 }
 
 #if defined(_MSC_VER)
+
+#include  // for GetExceptionInformation
+
 // If _MSC_VER is defined, we must have SEH. Use it if it's available. It's way
 // better than VEH. Vectored exception handling catches all exceptions happening
 // on the thread with installed exception handlers, so it can interfere with
@@ -203,6 +203,8 @@
   }
 
   int RetCode = (int)Except->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF000) == 0xE000)
+RetCode &= ~0xF000; // this crash was generated by sys::Process::Exit
 
   // Handle the crash
   const_cast(CRCI)->HandleCrash(
@@ -280,10 +282,13 @@
   // TODO: We can capture the stack backtrace here and store it on the
   // implementation if we so choose.
 
+  int RetCode = (int)ExceptionInfo->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF000) == 0xE000)
+RetCode &= ~0xF000; // this crash was generated by sys::Process::Exit
+
   // Handle the crash
   const_cast(CRCI)->HandleCrash(
-  (int)ExceptionInfo->ExceptionRecord->ExceptionCode,
-  reinterpret_cast(ExceptionInfo));
+  RetCode, reinterpret_cast(ExceptionInfo));
 
   // Note that we don't actually get here because HandleCrash calls
   // longjmp, which means the HandleCrash function never returns.
@@ -416,6 +421,21 @@
 
 #endif // !_MSC_VER
 
+LLVM_ATTRIBUTE_NORETURN
+void CrashRecoveryContext::HandleExit(int RetCode) {
+#if defined(_WIN32)
+  // SEH and VEH
+  ::RaiseException(0xE000 | RetCode, 0, 0, NULL);
+#else
+  // On Unix we don't need to raise an exception, we go directly to
+  // HandleCrash(), then longjmp will unwind the stack for us.
+  CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *)Impl;
+  assert(CRCI && "Crash recovery context never initialized!");
+  CRCI->HandleCrash(RetCode, 0 /*no sig num*/);
+#endif
+  llvm_unreachable("Most likely setjmp wasn't called!");
+}
+
 // FIXME: Portability.
 static void setThreadBackgroundPriority() {
 #ifdef __APPLE__
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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-10 Thread Hans Wennborg via Phabricator via cfe-commits 
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

Looks good to me, just a nit about one of the comments.

Thanks for making this easier to review by splitting it up!




Comment at: llvm/include/llvm/Support/Process.h:205
+
+  /// When integrated-cc1 is disabled, terminate the current program.
+  /// When integrated-cc1 is enabled, terminates execution of the current

Could the comment be phrased in terms of CrashRecoveryContext instead of 
integrated-cc1, which is a Clang thing?


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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-08 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:73
   // defined as an internal software error.  Otherwise, exit with status 1.
-  exit(GenCrashDiag ? 70 : 1);
+  llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1);
 }

arichardson wrote:
> I don't think it matters (yet?) but we should probably also use 
> llvm::sys::Process::Exit() in cc1as_main.cpp?
Changed cc1as_main.cpp as well.


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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-08 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea updated this revision to Diff 243380.
aganea added a comment.

Minor update -- added support & tested the VEH codepath as well.


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

https://reviews.llvm.org/D73742

Files:
  clang/test/Driver/crash-report.c
  clang/tools/driver/cc1_main.cpp
  clang/tools/driver/cc1as_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
@@ -14,9 +14,6 @@
 #include "llvm/Support/ThreadLocal.h"
 #include 
 #include 
-#ifdef _WIN32
-#include  // for GetExceptionInformation
-#endif
 #if LLVM_ON_UNIX
 #include  // EX_IOERR
 #endif
@@ -178,6 +175,9 @@
 }
 
 #if defined(_MSC_VER)
+
+#include  // for GetExceptionInformation
+
 // If _MSC_VER is defined, we must have SEH. Use it if it's available. It's way
 // better than VEH. Vectored exception handling catches all exceptions happening
 // on the thread with installed exception handlers, so it can interfere with
@@ -203,6 +203,8 @@
   }
 
   int RetCode = (int)Except->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF000) == 0xE000)
+RetCode &= ~0xF000; // this crash was generated by sys::Process::Exit
 
   // Handle the crash
   const_cast(CRCI)->HandleCrash(
@@ -280,10 +282,13 @@
   // TODO: We can capture the stack backtrace here and store it on the
   // implementation if we so choose.
 
+  int RetCode = (int)ExceptionInfo->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF000) == 0xE000)
+RetCode &= ~0xF000; // this crash was generated by sys::Process::Exit
+
   // Handle the crash
   const_cast(CRCI)->HandleCrash(
-  (int)ExceptionInfo->ExceptionRecord->ExceptionCode,
-  reinterpret_cast(ExceptionInfo));
+  RetCode, reinterpret_cast(ExceptionInfo));
 
   // Note that we don't actually get here because HandleCrash calls
   // longjmp, which means the HandleCrash function never returns.
@@ -416,6 +421,21 @@
 
 #endif // !_MSC_VER
 
+LLVM_ATTRIBUTE_NORETURN
+void CrashRecoveryContext::HandleExit(int RetCode) {
+#if defined(_WIN32)
+  // SEH and VEH
+  ::RaiseException(0xE000 | RetCode, 0, 0, NULL);
+#else
+  // On Unix we don't need to raise an exception, we go directly to
+  // HandleCrash(), then longjmp will unwind the stack for us.
+  CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *)Impl;
+  assert(CRCI && "Crash recovery context never initialized!");
+  CRCI->HandleCrash(RetCode, 0 /*no sig num*/);
+#endif
+  llvm_unreachable("Most likely setjmp wasn't called!");
+}
+
 // FIXME: Portability.
 static void setThreadBackgroundPriority() {
 #ifdef __APPLE__
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();
+
+  /// When integrated-cc1 is disabled, terminate the

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea updated this revision to Diff 243241.
aganea marked 2 inline comments as done.
aganea edited the summary of this revision.
aganea added a comment.

Simplified patch after landing previous incremental changes mentioned above 
.

@arichardson If you have the chance, could you please apply this, see if that 
fixes your use-case?


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

https://reviews.llvm.org/D73742

Files:
  clang/test/Driver/crash-report.c
  clang/tools/driver/cc1_main.cpp
  clang/tools/driver/cc1as_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
@@ -203,6 +203,8 @@
   }
 
   int RetCode = (int)Except->ExceptionRecord->ExceptionCode;
+  if ((RetCode & 0xF000) == 0xE000)
+RetCode &= ~0xF000; // this crash was generated by sys::Process::Exit
 
   // Handle the crash
   const_cast(CRCI)->HandleCrash(
@@ -416,6 +418,21 @@
 
 #endif // !_MSC_VER
 
+LLVM_ATTRIBUTE_NORETURN
+void CrashRecoveryContext::HandleExit(int RetCode) {
+#if defined(_WIN32)
+  // SEH and VEH
+  ::RaiseException(0xE000 | RetCode, 0, 0, NULL);
+#else
+  // On Unix we don't need to raise an exception, we go directly to
+  // HandleCrash(), then longjmp will unwind the stack for us.
+  CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *)Impl;
+  assert(CRCI && "Crash recovery context never initialized!");
+  CRCI->HandleCrash(RetCode, 0 /*no sig num*/);
+#endif
+  llvm_unreachable("Most likely setjmp wasn't called!");
+}
+
 // FIXME: Portability.
 static void setThreadBackgroundPriority() {
 #ifdef __APPLE__
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();
+
+  /// When integrated-cc1 is disabled, terminate the current program.
+  /// When integrated-cc1 is enabled, 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
@@ -97,6 +97,11 @@
 return RunSafelyOnThread([&]() { Fn(UserData); }, RequestedStackSize);
   }
 
+  /// Explicitly trigger a crash recovery in the current process, and
+  /// return failure from RunSafely(). This function does not return.
+  LLVM_ATTRIBUTE_NORETURN
+  void HandleExit(int RetCode);
+
   /// In case of a crash, this is the crash identifier.
   int RetCode = 0;
 
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-07 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:73
   // defined as an internal software error.  Otherwise, exit with status 1.
-  exit(GenCrashDiag ? 70 : 1);
+  llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1);
 }

I don't think it matters (yet?) but we should probably also use 
llvm::sys::Process::Exit() in cc1as_main.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-05 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

In D73742#1854810 , @hans wrote:

> I think so, but I need to look closer at the CrashRecoveryContext changes, 
> and it would help to do that separately.


I split this into smaller pieces. Please take a look at D74063 
, D74070 , 
D74076  and D74078 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-03 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

I think so, but I need to look closer at the CrashRecoveryContext changes, and 
it would help to do that separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-03 Thread Alexandre Ganea via Phabricator via cfe-commits 
aganea added a comment.

In D73742#1854773 , @hans wrote:

> > Evidently I can cut the patch in smaller pieces, let me know.
>
> Yes, I think this would be good. There's a lot going on in this patch, and it 
> would be good to separate the simple stuff from the tricky parts.


@hans It was only so you can have the big picture. Does it make sense overall?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-03 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

> Evidently I can cut the patch in smaller pieces, let me know.

Yes, I think this would be good. There's a lot going on in this patch, and it 
would be good to separate the simple stuff from the tricky parts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-01-30 Thread Alexandre Ganea via Phabricator via cfe-commits 
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 
 #include 
 #ifdef _WIN32
-#include  // for GetExceptionInformation
+#include 
 #endif
 #if LLVM_ON_UNIX
 #include  // 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