[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added a comment.

In D69825#1742276 <https://reviews.llvm.org/D69825#1742276>, @hans wrote:

> Instead of invoking main() (or a similar function like ClangDriverMain) as an alternative to spinning up the cc1 process, would it be possible to call ExecuteCC1Tool() directly? I guess it would be a little less general, but maybe it would be more straight-forward?


Possibly, yes, I'll have to check. There might be a few edge cases where we re-enter `ClangDriverMain()` twice if I recall correctly - I'll check.

In D69825#1742621 <https://reviews.llvm.org/D69825#1742621>, @zturner wrote:

> Does this change crash recovery semantics in any meaningful way?  Will we still be able to get stack traces on all platforms when the compiler crashes?


Yes, the changes in `CrashRecoveryContext` allow the same side-effects as the global exception handler. You would see the callstack and minidump in the same way.

There's still a minimal risk of memory/heap/CRT corruption, even with `CrashRecoveryContext` (after a crash). But that would possibly affect `LLVMUnhandledExceptionFilter` as well, and right now (//without this patch//), that handler runs in the context of the **crashed process **(not the calling one) so it wouldn't be worse than what it is now.

I could write a follow-up patch to prepare //on startup// the cmd-line invoked by `Driver::generateCompilationDiagnostics()`, instead of preparing after a crash. We could also pre-allocate a few virtual pages on advance, and use that in a BumpAllocator, instead of allocating after the crash. We could also merge the feature of `llvm-symbolizer` into `clang.exe`, so that we could call-back into `clang.exe` to render the callstack on stdout, even if `llvm-symbolizer` is not present.

The only drawback I see now is that we don't get the coredump on Linux, because the program doesn't end through the kernel signal handler. However, given that @Meinersbur says he sees no improvement on his side, we could disable this optimization on non-win? (or hide it behind a disabled flag on non-win)



================
Comment at: clang/lib/Driver/Job.cpp:347
+    StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+    if (CommandExe.equals_lower(DriverExe))
+      ClangDriverMain = Driver::Main;
----------------
Meinersbur wrote:
> Why is this check necessary? Why not assuming that if `Driver::Main` is set, it will be the right one?
The driver builds phases that do not always call the cc1 process. Simply stating `clang a.cpp` would invoke `clang -cc1`, then the linker. In the later case, even if we have `Driver::Main` it doesn't mean we should use it. There are a number of other edge cases of the same kind, such as `/fallback` or build cuda files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825