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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D70568#1760185 <https://reviews.llvm.org/D70568#1760185>, @hans wrote:

> Do I understand correctly that the main point is to get a stack trace when CrashRecoveryContext::RunSafely() fails, instead of just returning an error?


Correct. Otherwise, when running with D69825 <https://reviews.llvm.org/D69825>, the callstack was not displayed anymore when a crash occurred in clang-cl. This side-effect is that it also calls the cleanup, thus the change in `clang/test/Modules/signal.m` (the temp files are properly deleted in the case of a crash).

>> It enables CrashRecoveryContext exceptions by default (instead of disabled before) - however the exception handlers are lazily installed on the first creation of a CrashRecoveryContext.
> 
> Why does it now need to be enabled by default?

I used to call `CrashRecoveryContext::Enable()` in `CC1Command`, just before calling the CC1 callback. And to be consistent, call `CrashRecoveryContext::Disable()` after the CC1 callback.
But then, if we're running several compilations in parallel, it gets complicated, we can't just toggle it on/off all the time - it would need a refcount as the comment in `CrashRecoveryContext::Enable()` says.
I thought it'd be easier to just have it always enabled, as the signal handlers won't be installed before usage? Do you prefer that, or the refcounting?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70568