dexonsmith added a comment.
This is really cool; we've wanted this for a long time.
One concern I have is that I think this will interfere (effectively disable)
automatic OS handling of these crashes, which means they won't be collated and
reported anymore. Can we make this configurable somehow? (E.g., leave behind
an `-ffork-cc1` `-fno-fork-cc1` and a CMake flag to pick? Or just the CMake
flag?)
(I also have a couple of minor nits inline.)
================
Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:114
+CrashRecoveryContext::CrashRecoveryContext() : Impl(nullptr), head(nullptr) {
+ Install();
----------------
Can you move the `nullptr` default assignments back to the header, so it's
still obvious how they are set up?
================
Comment at: llvm/lib/Support/Unix/Signals.inc:348
+signed sys::InvokeExceptionHandler(int &RetCode, void *) {
+ SignalHandler(RetCode);
----------------
I'm not sure I've seen `signed` in code before. Might it be simpler to just
say `int` for the return type? Or is that somehow incorrect/confusing?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70568/new/
https://reviews.llvm.org/D70568
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
