[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In D70568#1763824 <https://reviews.llvm.org/D70568#1763824>, @aganea wrote:

> Thanks for analyzing this!
>
> In D70568#1763769 <https://reviews.llvm.org/D70568#1763769>, @hans wrote:
>
> > - About making CrashRecoveryContext::Enable() the default, that seems somewhat orthogonal to this change. You mention the use case of running several compilations in parallel, but I don't think that's a scenario that happens today? If doing this is necessary, I think it would be good to break it out into a separate patch.
>
>
> Even for non-parallel compilations, we still need to enable the `CrashRecoveryContext` inside the new `CC1Command`, just before calling `RunSafely()`.
>  If you look at a previous diff <https://reviews.llvm.org/D69825?id=227779> of D69825 <https://reviews.llvm.org/D69825>, in `clang/lib/Driver/Job.cpp, L389` there is:
>
>   static bool CRCEnabled{};
>   if (!CRCEnabled) {
>     llvm::CrashRecoveryContext::Enable();
>     CRCEnabled = true;
>   }
>   
>
> Which I removed, because I find it a bit awkward to "enable" something that should be decided internally by CrashRecoveryContext (this Enable API sounds more like internal behaviour leaking outside of CrashRecoveryContext).
>  The purpose of Enable/Disable is either for debugging (when enabling `LIBCLANG_DISABLE_CRASH_RECOVERY`) or possibly bubbling up the crash, as described in D23662 <https://reviews.llvm.org/D23662>.
>  Also consider the scenario where we sequentially compile several TUs: `clang-cl a.cpp b.cpp c.cpp`. Only the first call to `CC1Command::Execute()` should call `llvm::CrashRecoveryContext::Enable()`.
>  It is orthogonal, I can send a separate patch. Should we instead call `llvm::CrashRecoveryContext::Enable()` in D69825 <https://reviews.llvm.org/D69825>, when `clang.exe` starts?


Yes, maybe enabling it on startup is the best way to do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70568