[Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[aganea]

Summary

This patch is an optimization for speed: whenever possible, it will avoid creating a new process for the -cc1 invocation, and instead will call the cc1 tool inside the calling process (clang tool).
On Windows, this has a moderate impact on build times (please see timings in the comments below)

CFG (control flow guard) is disabled on Windows.

This patch also mildly improves build times on a Haswell 6-core Linux PC.

@Meinersbur reported no change in timings on a many-core Linux machine.

---

If you'd like to try this on your configuration, use the script below to ensure a standardized unit of measure:
(this is only for Windows)

abba_test.ps112 KBDownload

Warning: the script pulls and reverts all local changes!
Please edit the script before running, to provide the right filename for applying the patch.

The script can be used such as:

# run the test (lengthy, hours!)
> powershell .\abba_test.ps1

By default, it'll do a 2-stage build, including cmake'ing with a fixed set of options; followed by AB/BA testing where it'll alternatively rebuild using Clang 10 with and without the patch, for at least 5 hours. If you want to fiddle the number of hours it runs, you can do:

> powershell .\abba_test.ps1 50

Where '50' represents the number of hours it'll run. You can also skip building the two stages, if you already did it, by setting the second parameter:

> powershell .\abba_test.ps1 50 $True

You then end up with something like that:

Total iterations:  6
     |         Min       |        Mean       |       Median      |         Max       |
   A |  00:11:55.8588259 |  00:12:11.5531275 |  00:12:09.5867878 |  00:12:30.2468040 |
   B |  00:09:35.1556958 |  00:09:52.5302980 |  00:09:55.2329476 |  00:10:04.4994028 |
Diff | -00:02:20.7031301 | -00:02:19.0228296 | -00:02:14.3538402 | -00:02:25.7474012 |

If you find this useful, I can maybe convert it to a python script and send a review separately.

[aganea]

Fix missing llvm::InitializeAllTargets() in driver.cpp

[aganea]

A few remarks:

clang/lib/Driver/Job.cpp

318

See my other comment about LLVM_THREAD_LOCAL

clang/tools/driver/driver.cpp

366

LLVM_THREAD_LOCAL is here for the llvm-buildozer application I've presented at the LLVM conference. It's there because main() is entered from several threads, thus all clang needs to be thread-safe (I have several other patches to achieve that).
If you want this patch without LLVM_THREAD_LOCAL I'm fine with that.

llvm/lib/Support/Windows/Signals.inc

814 ↗

(On Diff #227779)

When this function is entered through the global exception handler, returning EXCEPTION_EXECUTE_HANDLER lets the CRT do its own exit with the exception code.

[Meinersbur]

This patch reduced the build time from 12 to 7 minutes? Sounds too good to be true. Thanks for the work! I would primarily interested in this to avoid determining the -cc1 command line when debugging.

However, the implementation of calling it's own main function gives the impression to be more like a hack. How much restructuring would be needed to properly create a CompilerInstance inside the driver instead?

clang/lib/Driver/Job.cpp

368–383

This looks fragile as it will break when the user chooses to rename the executable (clang-cuda, --driver-mode=...,...). Why not moving the ClangDriverMain logic into the clangDriver module. Or, at least, pass it through a lambda. It would also make it easier to use clangDriver as a library (if that was ever an intended use-case?!?)

clang/tools/driver/driver.cpp

365

Could you add a comment for the purpose of this function? It's not obvious outside the context of this patch.

[Meinersbur]

clang/lib/Driver/Job.cpp

368–383

I tried this on Linux and it did not work out-of-the box (ClangDriverMain being nullptr) due to the executable's name being clang-10. After fixing, check-clang failed with 5 errors in the Driver test.

The performance benefit was within noise: 6m20s vs. 6m18s on a 2-Socket 28 cores-per-processor (112 SMT threads) Skylake-Xeon system for ninja all check-clang, assertions enabled.

[thakis]

Thanks for sending this out!

Instead of the dynamic lookup of that symbol, what do you think about passing in the function via a normal api? That way, the type checker and linker help us keep things working; dynamic lookup is always a bit subtle and hard to grep for. (llvm-cs wouldn't know about the current use, for example.) See https://reviews.llvm.org/D69848 for a rough sketch.

For robust crash handling, I figured on non-win we'd have a signal handler that on signal self-execs with a "actually, do use a cc1 process" and then use the existing crash machinery, but maybe that's not needed.

[aganea]

Thanks for the feedback @Meinersbur!

This patch is mainly geared towards Windows users. I'm not expecting anything on Linux, you already have all the bells & whistles there :-) Although I definitely see improvements on my Linux box. Would the distro make a difference? Mine is:

$ uname -a
Linux (name edited) 5.0.0-29-generic #31~18.04.1-Ubuntu SMP Thu Sep 12 18:29:21 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

In D69825#1733254, @Meinersbur wrote:

This patch reduced the build time from 12 to 7 minutes? Sounds too good to be true.

Starting up and cooling down a large process like clang on Windows is very expensive. A fair amount of cpu time is spent in the clang driver, essentially in the kernel, in the range of 30 - 100 ms per process:
(timings below are before this patch)

We don't have fork() on Windows meaning that the kernel needs to re-start a the cc1 process from scratch, allocate pages, remap the EXE, allocate heap, allocate TLS, start the CRT, etc. Also, InitLLVM::InitLLVM is expensive because it calls into dbghelp.dll and loads symbols - just that can sometimes take up to 10 ms per process (depending on the system load).
In addition, recent Windows builds have all sorts of kernel regressions related to process creation and virtual pages allocation. Bruce Dawson has several blog entries about all this.
We're simply mitigating these issues. Upgrading the 36-core server to Windows 10 build 1903 will probably decrease the gap (12 min -> 7 min). However I would still expect users of Clang on pre-1903 builds for a few years from now.

I will fix the other issues you've mentioned.

[aganea]

In D69825#1733958, @thakis wrote:

Thanks for sending this out!

Instead of the dynamic lookup of that symbol, what do you think about passing in the function via a normal api? That way, the type checker and linker help us keep things working; dynamic lookup is always a bit subtle and hard to grep for. (llvm-cs wouldn't know about the current use, for example.) See https://reviews.llvm.org/D69848 for a rough sketch.

Absolutely, it make more sense, I can't remember why I used the dynamic lookup :-( I have this patch lying around since last year.

For robust crash handling, I figured on non-win we'd have a signal handler that on signal self-execs with a "actually, do use a cc1 process" and then use the existing crash machinery, but maybe that's not needed.

Right now on Windows I'm doing about that, not in the exception handler, but a bit later on in clang/lib/Driver/Driver.cpp, L1338. The worse that can happen would be a memory corruption, which would crash Driver::generateCompilationDiagnostics() and prevent displaying the preprocessed file and the cmd-line. But you would still see the callstack (which is rendered now in the SEH, see llvm/lib/Support/CrashRecoveryContext.cpp, L194 - I still need to hook that up for the posix implementation). Ideally we could render some of these things in advance (or pre-allocate memory at least) to minimize the risk of a crash when displaying crash diagnostics.

[aganea]

Addressed comments & finished the Linux part. All tests pass.

@Meinersbur : Just to show the difference between Windows & Linux, here's some timings for running the tests over the same git checkout, compiled with clang-9.
Everything was already built, I used ninja check-all and only the tests ran.
LLVM_ENABLE_PROJECTS = "clang;lld;clang-tools-extra;compiler-rt"

6-core Intel(R) Xeon(R) CPU E5-1650 0 @ 3.20GHz	Ubuntu 18.04	770.07s (12 min 50 sec)	59182 tests ran
6-core Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz	Windows 10 build 1903	1779.83s (29 min 39 sec)	56589 tests ran

This patch does not make much difference for running the tests on Windows (it's saving only 30 sec). The friction in the OS is so high, improving just clang.exe does not change much.

[hans]

Thanks for sharing this!

I still need to take a proper look, but my first thought is this:

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?

[Meinersbur]

clang/lib/Driver/Job.cpp

376

Why is this check necessary? Why not assuming that if Driver::Main is set, it will be the right one?

[zturner]

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?

llvm/lib/Support/CrashRecoveryContext.cpp

207 ↗

(On Diff #228729)

You can fix this by writing:

static bool wrap_function_call(function_ref<void()> Fn, bool EnableExceptionHandler, unsigned &RetCode)
{
   __try {
     Fn();
     return true;
  } __except (EnableExceptionHandler
                  ? LLVMUnhandledExceptionFilter(GetExceptionInformation())
                  : 1) {
    RetCode = GetExceptionCode();
    return false;
  }
}

bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
    ...
    bool Result = wrap_function_call(EnableExceptionHandler, Fn, RetCode);
    ...
}

[aganea]

In 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, @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 as before.

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)

clang/lib/Driver/Job.cpp

376

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.

[aganea]

Call into ExecuteCC1Tool() directly, as suggested by @hans
Add more comments.
Remove pThis in CrashRecoveryContext.cpp:RunSafely() as suggested by @zturner

[hans]

Thanks for the update! I think calling ExecuteCC1Tool instead of main is an improvement (there's no need to think about "re-entering" the process, it's really just a function call). I still don't see why it needs to go through a function pointer. I was hoping the code could be something like:

In Command::Execute:
if the tool we're going to execute is cc1 (not the gen-reproducer though),
then instead of forking, call ExecuteCC1Tool with the args directly (inside a context so that we can recover from a crash)

Do you think that's achievable?

clang/include/clang/Driver/Driver.h

209

Why do we need a variable for this? Couldn't the code just invoke the function directly?

[aganea]

@hans : Simply because ExecuteCC1Tool() lives in the clang tool (clang/tools/driver/driver.cpp) and we're calling it from the clangDriver.lib (clang/lib/Driver/Job.cpp). The clangDriver.lib is linked into many other tools (clang-refactor, clang-rename, clang-diff, clang-check, libclang, ...) If I reference the cc1 function directly, we end of with linker errors in all those tools.
We could maybe move the tool code into the clangDriver.lib, but I'm not sure it's practical as the tool pulls on many other libraries. I thought the callback was the path of least resistance. Let me know if we could do it otherwise.

[hans]

In D69825#1747539, @aganea wrote:

@hans : Simply because ExecuteCC1Tool() lives in the clang tool (clang/tools/driver/driver.cpp) and we're calling it from the clangDriver.lib (clang/lib/Driver/Job.cpp). The clangDriver.lib is linked into many other tools (clang-refactor, clang-rename, clang-diff, clang-check, libclang, ...) If I reference the cc1 function directly, we end of with linker errors in all those tools.
We could maybe move the tool code into the clangDriver.lib, but I'm not sure it's practical as the tool pulls on many other libraries. I thought the callback was the path of least resistance. Let me know if we could do it otherwise.

Oh, I see. I didn't think about that. That's annoying, but maybe using the variable makes sense then.

clang/include/clang/Driver/Driver.h

207

It's not really a callback though. How about just "Pointer to the ExecuteCC1Tool function, if available."
It would also be good if the comment explained why the pointer is needed.
And why does it need to be thread-local? Can different threads end up with different values for this?

clang/include/clang/Driver/Job.h

136

Is there another way of avoiding in-process-cc1 after a crash? For example, could the Job know that it's being built for generating the crash report? It seems unfortunate to introduce this widely-scoped variable here.

clang/lib/Driver/Job.cpp

367–368

I don't think this form of initialization is common in LLVM code.

375

Now that we're not calling main() anymore, but cc1 directly -- is this checking still necessary?

clang/tools/driver/driver.cpp

333

It's not really re-entering. Maybe "If we call the cc1 tool directly in the driver process" is clearer?

llvm/include/llvm/Support/CrashRecoveryContext.h

105 ↗

(On Diff #229333)

I don't think this form of initialization is common in LLVM. Same below.

llvm/lib/Support/CrashRecoveryContext.cpp

23 ↗

(On Diff #229333)

Can we move this declaration, and the one for SignalHandler, into some header file? Declaring it manually like this doesn't seem so nice.

[aganea]

Addressed @hans' comments.

I've also added a script in the summary, if you'd like to reproduce the results on your end, and to ensure we all run the same thing.

[aganea]

clang/lib/Driver/Job.cpp

375

Yes it is - see my other comment just above.

The driver builds phases that do not always call the cc1 process. Simply stating clang a.cpp would invoke clang -cc1, then the linker, say link.exe. In this later case (invoking link.exe), 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 building cuda files, where a different executable is invoked along the way."

[hans]

clang/lib/Driver/Job.cpp

367–368

I think it would be clearer with a boolean "CallCC1" or something instead of a function pointer.

375

Okay, but couldn't we find a cleaner way of figuring out that the Command is a cc1 invocation? The thing that creates the Command would know.. maybe it could set a flag or something? Or maybe instead of creating a Command it would create something else?

[aganea]

clang/lib/Driver/Job.cpp

375

Yeah you're right, I've refactored and added a CC1Command instead, makes things much cleaner, thank you!

[aganea]

I've split the CrashRecoveryContext changes into another patch to minimize the risks, please see D70568. That patch will need to go first before this one.

[russell.gallop]

Thanks for this change. I applied this patch (prior to splitting out https://reviews.llvm.org/D70568) and it built and worked okay (I only see one clang-cl in process explorer).

I don't see anything like same performance improvement however. I did my own assessment of build times. I built 2 x stage 1 toolchains using clang 9.0.0 with and without this patch (applied to 1b9ef3bbb595206b0097b7adec2c1b69eae6fab4). Then I used each of those to build LLVM (no clang or lld) again.

CMake settings:

# Build stage 1 (same with and without patch)
cmake -G Ninja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_LINKER="C:/Program Files/LLVM/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang;lld -DLLVM_TARGETS_TO_BUILD=X86 -DCLANG_ENABLE_STATIC_ANALYZER=OFF -DCLANG_ENABLE_ARCMT=OFF ../llvm

# Build stage 2 without patch
cmake -G Ninja -DCMAKE_C_COMPILER="<llvm-project.git>/build_stage1_no_patch/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="<llvm-project.git>/build_stage1_no_patch/bin/clang-cl.exe" -DCMAKE_LINKER="<llvm-project.git>/build_stage1_no_patch/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 ../llvm

# Build stage 2 with patch
cmake -G Ninja -DCMAKE_C_COMPILER="<llvm-project.git>/build_stage1_patch/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="<llvm-project.git>/build_stage1_patch/bin/clang-cl.exe" -DCMAKE_LINKER="<llvm-project.git>/build_stage1_patch/bin/lld-link.exe" -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 ../llvm

These both built the patched code.

I got a ~1.5% performance improvement, reducing "ninja all" from 571.5s to 562.5s seconds on a 6 core machine (average over three 3 builds on a quiet machine), times below. For reference I am using winver 1803 and have CFG enabled.

Assuming that time_saved ≈ (process_invocation_overhead * compiler_jobs) / parallel_build_jobs
or (rearranging) process_invocation_overhead ≈ (time_saved * parallel_build_jobs) / compiler_jobs
(I know this is not perfectly true as build jobs will tail off towards the end of a build, but I think is okay for a ballpark estimate.)

I saved 9 seconds, over 1720 compiler jobs (from ninja -v -n all | grep -c clang-cl), with 14 parallel build jobs which gives the process invocation overhead of about 73ms which is in the range you mentioned above (30-100ms). Therefore I think that this is in the right ballpark.

I can't see how this would get 20% (from the table) or 30% improvement (from the first graph). In abba_test.ps1 you include running tests (due to check-all) so it is possible that you are saving time on a very large number of clang driver invocations on very small files. This is helpful for LLVM developers but I don't think it's representative of other builds. Alternatively, is the process overhead higher on your machine(s) for some reason (e.g. security software).

To be clear, I'm not against this patch going in, and I can confirm that it is a performance improvement so good to have. I just can't see where the claimed 20-30% saving on build time comes from.

Time data for 3 builds (seconds)
No patch (571.349, 575.353, 567.638)
With Patch (560.870, 563.110, 563.368)

[aganea]

Thanks for the feedback Russell!

Can you possibly try again with /MT? (ie. -DLLVM_USE_CRT_RELEASE=MT)

In the abba_test.ps1 script, ninja check-all is only used for preparing clang.exe with the patch. The A/B loop does not use check-all.

I've modified the abba_test.ps1 script to only build llvm/, with the same options as you do. There was also a bug in the timespan calculation, the test would last a lot more than you specified.

I also found out that on some of our multi-socket servers, Ninja defaulted to only the # threads from the first socket. If you had a 2x 18-core system, Ninja would default to 38 threads (18 x 2 + 2) instead of 74 threads (2 x 18 x 2 + 2). This behavior seems to be random, depending on the system. This has been fixed by Ninja PR 1674, I've tested all our systems with this patch and they now all default to the proper # of threads.

I'm doing a new round of tests, I'll get back with updated figures.

[hans]

Thanks! This is starting to look really good now.

clang/lib/Driver/Job.cpp

318

Maybe better to just return a bool for success/failure since there is not really a process exit code yet.

(But I'm not even sure this is worth a separate method; see comment further down.)

424

For CC1Command, PrepareExecution doesn't do very much (since it doesn't use response files). Because of that, I'm not sure breaking out PrepareExecution into a separate method is worth it. I'd just copy the part that applies to CC1Command here. Then we wouldn't need to check the return value from PrepareExecution (it can't fail if response files aren't used), and we also don't need to intercept SetResponsefile -- we'd just ignore it even if it's set.

453

llvm_unreachable("msg") is usually preferred instead of assert(0 && "msg")

clang/tools/driver/driver.cpp

338

This feels a little risky to me. Do we still need to rely on argv[1], now that we have a nice CC1Command abstraction?

[russell.gallop]

In D69825#1758949, @aganea wrote:

Thanks for the feedback Russell!

Can you possibly try again with /MT? (ie. -DLLVM_USE_CRT_RELEASE=MT)

I tried adding this to my stage 1 builds and it didn't make any significant difference (times in seconds):

No patch (573.879, 568.519, 567.709)
With Patch (578.253, 558.515, 562.368)

In case it makes a difference, this is from a VS2017 command prompt with Visual Studio 15.9.17.

In the abba_test.ps1 script, ninja check-all is only used for preparing clang.exe with the patch. The A/B loop does not use check-all.

Ah, yes. Sorry, my mistake.

I also found out that on some of our multi-socket servers, Ninja defaulted to only the # threads from the first socket. If you had a 2x 18-core system, Ninja would default to 38 threads (18 x 2 + 2) instead of 74 threads (2 x 18 x 2 + 2). This behavior seems to be random, depending on the system. This has been fixed by Ninja PR 1674, I've tested all our systems with this patch and they now all default to the proper # of threads.

I'm not testing on a multi-socket system so don't believe I need that patch.

I'm doing a new round of tests, I'll get back with updated figures.

Thanks.

I ran the new version of abba_test.ps1 (just building LLVM) with small modifications to comment out the "Validate Ninja" step and change "check-all" to "all" to save time as I'm concentrating on build build speed. This means that I'm applying it to the same git revision as you. For clang_bypass_cc1.patch I used the patch I downloaded before (before spinning off D70568). Running for one hour on a six core machine (winver 1803, 14 ninja jobs, CFG enabled, recently rebooted) I get:

Total iterations:  2

     |         Min       |        Mean       |       Median      |         Max       |
   A |  00:15:13.0773389 |  00:15:13.7219710 |  00:15:13.7219710 |  00:15:14.3666030 |
   B |  00:15:08.6134425 |  00:15:09.0363284 |  00:15:09.0363284 |  00:15:09.4592142 |
Diff | -00:00:04.4638964 | -00:00:04.6856426 | -00:00:04.6856426 | -00:00:04.9073888 |

So in this case it saved 0.5% of time. Using the previous maths, with 2378 clang-cl jobs, this implies process creation time of 29ms. This was fairly soon after a reboot. Maybe I'm just lucky and none of the process creation problems are affecting me on this system (at this moment).

[russell.gallop]

In D69825#1760279, @russell.gallop wrote:

So in this case it saved 0.5% of time. Using the previous maths, with 2378 clang-cl jobs, this implies process creation time of 29ms. This was fairly soon after a reboot. Maybe I'm just lucky and none of the process creation problems are affecting me on this system (at this moment).

It looks like the git apply didn't work, but the script continued so this was a duff experiment, please ignore. I'll try to fix this and run it again.

[aganea]

In D69825#1760373, @russell.gallop wrote:

It looks like the git apply didn't work, but the script continued so this was a duff experiment, please ignore. I'll try to fix this and run it again.

Thanks Russell! Can you try running the script overnight? It'll give a better result and we'll see the error bar.

In D69825#1760279, @russell.gallop wrote:

Maybe I'm just lucky and none of the process creation problems are affecting me on this system (at this moment).

I'm running tests on 6 differents Windows systems (both desktops and servers), with different OS revisions, and they all behave differently. With this patch, some see better improvements than others. I'll post the results once I have a better picture.

[russell.gallop]

In D69825#1760400, @aganea wrote:

In D69825#1760373, @russell.gallop wrote:

It looks like the git apply didn't work, but the script continued so this was a duff experiment, please ignore. I'll try to fix this and run it again.

Note for other reviewers, the abba_test.ps1 script uses a git format patch, not the format you can download from Phabricator.

Thanks Russell! Can you try running the script overnight? It'll give a better result and we'll see the error bar.

Okay. With the patch sorted, I ran for 10 hours overnight.

Total iterations:  21

     |         Min       |        Mean       |       Median      |         Max       |
   A |  00:14:11.0340636 |  00:14:33.2494597 |  00:14:25.3644475 |  00:15:25.4082274 |
   B |  00:13:57.9445995 |  00:14:20.3073502 |  00:14:09.0937819 |  00:16:06.2885457 |
Diff | -00:00:13.0894641 | -00:00:12.9421096 | -00:00:16.2706656 |  00:00:40.8803183 |

From looking at the individual times this was stable for a couple of hours then there were some particularly long runs (on A and B), possibly some scheduled maintenance tasks, so I'd recommend using the median value to ignore these outliers. Based on the median this saves about 1.9% for me.

[aganea]

You're right indeed Russell, my bad, this gain is not as important as I initially claimed -- however there's a gain.

Several factors came into play in my previous testings:

1. The latest ninja 1.9.0 doesn't use all cpu sockets on some Windows systems (see this), and as it uses +2 cores in addition to the detected # of cores, this possibly induces some unwanted side-effects (such as the speed-up seen in the summary).
2. I was using Clang 9.0 RC4, which is compiled with -DLLVM_ENABLE_ASSERTIONS=ON. When @hans sent the email for 9.0.1 I realized this flag is turned off only for the final release :-(
3. I was using all sorts of options (/MT /GS- /arch:AVX) which actually improved build times, when compared with the baseline Clang release (see graphs below)

We indeed have a security Windows service running on our machines which we cannot disable.

I have re-run the tests on 6 different configurations (see graph for specs)
When using this patch along with -GNinja -DLLVM_ENABLE_PROJECTS=clang;lld;llvm -DLLVM_USE_CRT_RELEASE=MT -DCMAKE_CXX_FLAGS="/arch:AVX /GS-" -DCMAKE_C_FLAGS="/arch:AVX /GS-", it reduces build times by about 30 sec on many-cores machines, and about 15-20 sec on 6-cores. The 28-core machine did not see any improvements, I cannot explain why.
The variability is also reduced when this patch is applied. This seems to be consistent across all machines.

WARNING: the X coordinate (time) does not start at zero

In my sense, the patch remains valid overall:

• It is easier to debug with just one process.
• Later on, I'd like to put forward a patch which (optionally) replaces the CRT allocator with rpmalloc, and not having this current patch would have less impact.
• I have an alternate proposal for D52193 and D69582 which uses a thread pool instead of a process pool, and this patch is needed for that purpose (however that requires reviving & solving the thread-safe cl::opt RFC).

[russell.gallop]

Thanks for the updated timings. I have no objection to this going in.

I haven't gone through the code changes in detail so I think you should probably get approval from someone who has (such as @hans).

[hans]

I'm also supported, even though the gains are smaller than first thought. It depends on D70568 though.

[arphaman]

@aganea Please disable the new behavior for the Darwin platform. We rely on the fact that Clang -cc1 processes crash to report crashes using system's crash reporting infrastructure.

[aganea]

In D69825#1768111, @arphaman wrote:

@aganea Please disable the new behavior for the Darwin platform. We rely on the fact that Clang -cc1 processes crash to report crashes using system's crash reporting infrastructure.

Will do of course. Just for my own curiosity, how do you actually disable the LLVM signal handler when you ship clang to customers? Through the env var LLVM_DISABLE_CRASH_REPORT? Or maybe this is disabled by default in your downstream fork? Does SignalHandler() get called?

Should I disable it through a #ifndef __APPLE__? Or something else?

[thakis]

In D69825#1768111, @arphaman wrote:

@aganea Please disable the new behavior for the Darwin platform. We rely on the fact that Clang -cc1 processes crash to report crashes using system's crash reporting infrastructure.

We'll want to use this in chromium on Mac. There should probably be a cmake def to pick the default driver mode for this (and an explicit flag to pick it at runtime). We can then set that cmake def.

[arphaman]

I guess Duncan's comments on https://reviews.llvm.org/D70568 were for this patch. I think it would be good to have:

• A cmake option that controls the default behavior. It should still fork for Darwin by default, the detection should be done through CMake when it's building for a Darwin platform.
• A driver/cc1 flag that allows the user to override the behavior.

This will allow users to avoid cc1 forking either by building their own Clang, or by passing in a flag with their build options. WDYT?

[dexonsmith]

In D69825#1771300, @arphaman wrote:

I guess Duncan's comments on https://reviews.llvm.org/D70568 were for this patch. I think it would be good to have:

That's right, I was confused about which review I was on.

• A cmake option that controls the default behavior. It should still fork for Darwin by default, the detection should be done through CMake when it's building for a Darwin platform.
• A driver/cc1 flag that allows the user to override the behavior.

• I don't think a -cc1 flag is reasonable, because it affects the caller, not -cc1 itself. I would be confused by this I think.
• Driver flag seems a bit too user-facing, but probably fine.
• Another (maybe odd) option would be an environment variable, such as CLANG_FORK_CC1=1 or CLANG_FORK_CC1=0, which overrides the default.

This will allow users to avoid cc1 forking either by building their own Clang, or by passing in a flag with their build options. WDYT?

Right. Some way to control at runtime, especially so we can test both ways of launching in check-clang but also probably useful for other scenarios, and a cmake flag to set the default so vendors can choose.

[aganea]

I did this in driver.cpp:

// Whether the cc1 tool should be called inside the current process, or forked
// to a new new process.
bool UseCC1ProcessFork = CLANG_FORK_CC1;

StringRef ForkCC1Str = ::getenv("CLANG_FORK_CC1");
if (!ForkCC1Str.empty()) {
  UseCC1ProcessFork = ForkCC1Str.compare("1") == 0 ||
                      ForkCC1Str.compare_lower("on") == 0 ||
                      ForkCC1Str.compare_lower("true") == 0;
}
if (!UseCC1ProcessFork) {
  // Here we provide a shortcut for calling the -cc1 cmd-line within the
  // same process, instead of starting a new process. This saves a some
  // execution time of Windows, as process creation can be expensive on
  // that platform.
  TheDriver.CC1Main = &ExecuteCC1Tool;
}

With a new option in cmake to control the default behavior:

set(CLANG_FORK_CC1 OFF BOOL
    "Whether clang should use a new process for the CC1 invocation")

if (APPLE)
  set(CLANG_FORK_CC1 1)
endif()

And the corresponding flag in config.h:

/* If we want to fork a new process clang.exe for the CC1 invocation */
#cmakedefine01 CLANG_FORK_CC1

This option needs to be controlled at large (otherwise some TUs might compile with forking "on" and others with forking "off", depending on what happens in the build scripts), so I thought the env variable would make more sense. Do you think a new cmd-line option is worth it, or is the env var enough?

[arphaman]

The environment variable should be enough IMHO

[rnk]

I'd be fine with the cmake var and the env var and no driver flag.

I think clang developers on Darwin probably want the single-process clang -c behavior (that's how I interpreted what Duncan said), even though Apple as a distributor of clang wants the multi-process behavior. I would skip the platform check and in upstream clang and suggest that Apple customize the CMake variable when producing the XCode tools package. But, that's just my opinion, please disagree if you don't like that idea.

Lastly, we could make the environment variable more cross-platform by calling it CLANG_SPAWN_CC1 to be inclusive of OSs where fork doesn't exist. Besides, doesn't clang use posix_spawn on Mac anyway?

[aganea]

Addressed all comments.
Added CLANG_SPAWN_CC1 cmake flag & environment variable to control whether we want a cc1 process or not. It is disabled by default. I left out the if (APPLE) part in cmake, as suggested by @rnk.

[aganea]

clang/tools/driver/driver.cpp

338

Risky in what way? What would you suggest? The contents of argv are exactly the same in both cases: when called from the CC1Command, or explicitly from the command-line.

[hans]

I'm basically happy with this, just some minor comments.

clang/lib/Driver/Job.cpp

376

I think this gets printed by "clang -###" so maybe just print something short here. Perhaps just an "(in-process)" prefix? Then we don't need to change so many test expectations also.

394

Do we need to disable afterwards?

clang/tools/driver/driver.cpp

268

Maybe just do the "!!" thing like for the environment variables above? It's not pretty, but at least that would be consistent, and it avoids the problem of deciding what values mean "on" and what mean "off".

338

You're right, I suppose the code was already doing this.

[aganea]

clang/lib/Driver/Job.cpp

394

I moved this line to clang/tools/driver/driver.cpp like discussed previously.

clang/tools/driver/driver.cpp

268

Not if we want to negate the CLANG_SPAWN_CC1 compile-time flag initialized just above. !!::getenv will only say the option is there, it won't say what we should do. My rationale was, if the env variable is there, its value overrides the compile-time flag. Unless we want something along the lines of CLANG_ENABLE_SPAWN_CC1 / CLANG_DISABLE_SPAWN_CC1?

[hans]

clang/tools/driver/driver.cpp

268

Ah, I didn't think about that.

This feels a bit fragile though, for example if the user sets it to "yes" instead of "on", it won't be obvious that it has the opposite effect. What do you think about only supporting 1 and 0, and printing a helpful error for other values?

Also, the comparisons could be done with equals(_lower) instead of the compare methods.

[aganea]

clang/tools/driver/driver.cpp

268

As requested.

[hans]

Looks good to me (with a minor comment). Thanks!

clang/tools/driver/driver.cpp

271

I think just checking SpawnCC1Str == "0" or == "1" would be easier.

[aganea]

Attention Apple maintainers: after this patch, do not forget to add -DCLANG_SPAWN_CC1=ON to your release scripts, to retain your previous crash reporting behavior (see discussion above). @dexonsmith @arphaman

[thakis]

Thanks for seeing this through!

env var or flag

Our build setup can easily add commandline flags, but it can't easily add env vars. So I think this should have a driver mode flag.

For people who asked for an env var: Why do you need an env var for this? Almost all compiler settings are accessible by flag but not by env var. Why is this one special?

[mgorny]

This broke NetBSD buildbot:

+ : 'RUN: at line 1'
+ env -u CLANG_SPAWN_CC1 /home/motus/netbsd8/netbsd8/build-stage2/bin/clang -c /data/motus/netbsd8/netbsd8/llvm-project/clang/test/Driver/cc1-spawnprocess.c -o /dev/null
env: unknown option -- u
Usage: env [-i] [name=value ...] [command]

Please fix.

clang/test/Driver/cc1-spawnprocess.c

1 ↗

(On Diff #237677)

env -u is not portable.

[joerg]

clang/test/Driver/cc1-spawnprocess.c

1 ↗

(On Diff #237677)

I think just going for env CLANG_SPAWN_CC1= works for the purpose of this test.

[aganea]

Fixed NetBSD as suggested in rG88b8cb7215d4333ab990c99f21c7f92262ef02ef

[xazax.hun]

Hi!

This patch breaks scan-build-py which parses the output of "-###" to get -cc1 command. There might be other tools with the same problems. Could we either remove (in-process) from CC1Command::Print or add a line break?

Having the last line as a valid invocation is valuable and there might be tools relying on that.

[NoQ]

In D69825#1827826, @xazax.hun wrote:

This patch breaks scan-build-py which parses the output of "-###" to get -cc1 command.

Uh-oh, thanks for catching this. The same applies to the regular scan-build. Unfortunately, neither is properly tested. We really need to get this fixed.

[aganea]

@thakis landed a patch to control this behavior on the cmd-line, adding -fno-integrated-cc1 would solve your issue (ie. revert to old behavior). @NoQ @xazax.hun is that an acceptable solution?

[xazax.hun]

There might be other tools parsing the output of -###. I think adding a line break in the output is better/easier than updating all those tools (amd making them potentially slower, not profiting from this patch).

[wenlei]

@aganea, @hans This patch broke response file expansion for preprocessor via -Wp - all our internal builds failed with this patch because we invoke clang with response file passed to preprocessor, e.g. clang++ ... -Wp,@pp.rsp @cc.rsp. Looks like we can't just call ExecuteCC1Tool directly, because response file expansion happens in main, so with this patch, -Wp,@pp.rsp won't be expanded properly.

repo:

CLANG_SPAWN_CC1=0  clang++ -c  -Wp,@a.rsp  @a.rsp test.cpp

error: error reading '@rr.rsp'
1 error generated.

and it works with CLANG_SPAWN_CC1=1

[aganea]

@wenlei Taking a look now.
@hans I remember now why I was re-entering main() in the first iteration of this patch. Response files were one reason. And handling /MP was another, which triggers the re-entrance twice (once for CL driver, and once for cc1).

[bjope]

When I build the clang on trunk, using clang 8.0 with asan enabled, and then run the clang lit tests I see lots of failures in Clang :: InterfaceStubs/driver-test.c and Clang :: InterfaceStubs/driver-test2.c (and maybe the faults I get in Clang :: Driver/cl-showfilenames.c comes from this patch as well).

The failures go away if I revert this patch (commit b4a99a061f517e60985667e39519f601). To make the revert I also needed to revert commit 8e5018e990b701391e6c33ba85b012343df67272 which is based on this patch.

Examples of failures:

==130643==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 192 byte(s) in 3 object(s) allocated from:
    #0 0xd0ea38 in operator new(unsigned long) /workspace/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:106
    #1 0x8153b36 in make_unique<clang::GenerateInterfaceIfsExpV1Action> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:3132:28
    #2 0x8153b36 in CreateFrontendBaseAction /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:68
    #3 0x8153b36 in clang::CreateFrontendAction(clang::CompilerInstance&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:137
    #4 0x8156a22 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:287:39
    #5 0xd23349 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/cc1_main.cpp:239:15
    #6 0x7a0082e in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:34
    #7 0x7a0082e in void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const::$_1>(long) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:108
    #8 0x6634feb in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:125:12
    #9 0x6634feb in llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../lib/Support/CrashRecoveryContext.cpp:396
    #10 0x79fefaf in clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:12
    #11 0x795265d in clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:182:15
    #12 0x7953922 in clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:233:19
    #13 0x79a4d0e in clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Driver.cpp:1473:5
    #14 0xd19eac in main /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/driver.cpp:493:21
    #15 0x7f882be4e544 in __libc_start_main (/lib64/libc.so.6+0x22544)

Direct leak of 184 byte(s) in 1 object(s) allocated from:
    #0 0xd0ea38 in operator new(unsigned long) /workspace/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:106
    #1 0x81537b4 in make_unique<clang::EmitAssemblyAction> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:3132:28
    #2 0x81537b4 in CreateFrontendBaseAction /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:52
    #3 0x81537b4 in clang::CreateFrontendAction(clang::CompilerInstance&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:137
    #4 0x8156a22 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:287:39
    #5 0xd23349 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/cc1_main.cpp:239:15
    #6 0x7a0082e in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:34
    #7 0x7a0082e in void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const::$_1>(long) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:108
    #8 0x6634feb in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:125:12
    #9 0x6634feb in llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../lib/Support/CrashRecoveryContext.cpp:396
    #10 0x79fefaf in clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:12
    #11 0x795265d in clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:182:15
    #12 0x7953922 in clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:233:19
    #13 0x79a4d0e in clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Driver.cpp:1473:5
    #14 0xd19eac in main /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/driver.cpp:493:21
    #15 0x7f882be4e544 in __libc_start_main (/lib64/libc.so.6+0x22544)

Indirect leak of 589860 byte(s) in 3 object(s) allocated from:
    #0 0xcd7ca7 in calloc /workspace/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:155
    #1 0x6694273 in safe_calloc /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/Support/MemAlloc.h:40:18
    #2 0x6694273 in llvm::StringMapImpl::init(unsigned int) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../lib/Support/StringMap.cpp:61
    #3 0xe4f90ee in StringMap /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/StringMap.h:251:7
    #4 0xe4f90ee in clang::IdentifierTable::IdentifierTable(clang::IdentifierInfoLookup*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Basic/IdentifierTable.cpp:60
    #5 0xe49ab27 in clang::Preprocessor::Preprocessor(std::__1::shared_ptr<clang::PreprocessorOptions>, clang::DiagnosticsEngine&, clang::LangOptions&, clang::SourceManager&, clang::HeaderSearch&, clang::ModuleLoader&, clang::IdentifierInfoLookup*, bool, clang::TranslationUnitKind) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Lex/Preprocessor.cpp:92:7
    #6 0x7db6092 in __compressed_pair_elem<std::__1::shared_ptr<clang::PreprocessorOptions> &&, clang::DiagnosticsEngine &, clang::LangOptions &, clang::SourceManager &, clang::HeaderSearch &, clang::CompilerInstance &, nullptr_t &&, bool &&, clang::TranslationUnitKind &, 0, 1, 2, 3, 4, 5, 6, 7, 8> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:2156:9
    #7 0x7db6092 in __compressed_pair<std::__1::allocator<clang::Preprocessor> &, std::__1::shared_ptr<clang::PreprocessorOptions> &&, clang::DiagnosticsEngine &, clang::LangOptions &, clang::SourceManager &, clang::HeaderSearch &, clang::CompilerInstance &, nullptr_t &&, bool &&, clang::TranslationUnitKind &> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:2259
    #8 0x7db6092 in __shared_ptr_emplace<std::__1::shared_ptr<clang::PreprocessorOptions>, clang::DiagnosticsEngine &, clang::LangOptions &, clang::SourceManager &, clang::HeaderSearch &, clang::CompilerInstance &, nullptr_t, bool, clang::TranslationUnitKind &> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:3672
    #9 0x7db6092 in make_shared<std::__1::shared_ptr<clang::PreprocessorOptions>, clang::DiagnosticsEngine &, clang::LangOptions &, clang::SourceManager &, clang::HeaderSearch &, clang::CompilerInstance &, nullptr_t, bool, clang::TranslationUnitKind &> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:4331
    #10 0x7db6092 in make_shared<clang::Preprocessor, std::__1::shared_ptr<clang::PreprocessorOptions>, clang::DiagnosticsEngine &, clang::LangOptions &, clang::SourceManager &, clang::HeaderSearch &, clang::CompilerInstance &, nullptr_t, bool, clang::TranslationUnitKind &> /proj/flexasic/app/llvm/8.0/bin/../include/c++/v1/memory:4710
    #11 0x7db6092 in clang::CompilerInstance::createPreprocessor(clang::TranslationUnitKind) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Frontend/CompilerInstance.cpp:389
    #12 0x7ef58bf in clang::FrontendAction::BeginSourceFile(clang::CompilerInstance&, clang::FrontendInputFile const&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Frontend/FrontendAction.cpp:744:8
    #13 0x7dc809f in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Frontend/CompilerInstance.cpp:964:13
    #14 0x8156a4d in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:290:25
    #15 0xd23349 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/cc1_main.cpp:239:15
    #16 0x7a0082e in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:34
    #17 0x7a0082e in void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const::$_1>(long) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:108
    #18 0x6634feb in operator() /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../include/llvm/ADT/STLExtras.h:125:12
    #19 0x6634feb in llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../lib/Support/CrashRecoveryContext.cpp:396
    #20 0x79fefaf in clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, bool*) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Job.cpp:402:12
    #21 0x795265d in clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:182:15
    #22 0x7953922 in clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) const /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Compilation.cpp:233:19
    #23 0x79a4d0e in clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::__1::pair<int, clang::driver::Command const*> >&) /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/lib/Driver/Driver.cpp:1473:5
    #24 0xd19eac in main /workspace//llvm-master-sanitize-asan/llvm/build-all-asan/../../clang/tools/driver/driver.cpp:493:21
    #25 0x7f882be4e544 in __libc_start_main (/lib64/libc.so.6+0x22544)

[smeenai]

CC @plotfi for the InterfaceStubs failures

[plotfi]

In D69825#1831611, @smeenai wrote:

CC @plotfi for the InterfaceStubs failures

Thanks

In D69825#1831611, @smeenai wrote:

CC @plotfi for the InterfaceStubs failures

@smeenai thanks for the CC. I have been looking into this asan failure and it isn't actually caused by the interface stubs AST consumer at all. What is triggering it is the pipeline setup. Here is what I gather:

• Two standard cc1 CompileJobs followed by two Interface Stubs cc1 CompileJobs (or any other Job type for that matter): asan failure
• if I clear the jobs list in Driver.cpp prior to appending the Interface Stubs jobs: No asan failure.
• One standard cc1 CompileJob followed by one Interface Stubs/Alternate cc1 Compile job: No asan failure.

TL;DR
The InterfaceStubs ASTConsumer CompileJob is not causing the ASAN failure here, it's the setup of having a standard compilation pipeline followed by a secondary side-care pipeline thats triggering it in -fintegrated-cc1 mode. I am still looking into this and any help would be much appreciated. In the meantime I could -fno-integrated-cc1 the driver.c InterfaceStubs tests to quite the bots if that is what people want (and file a bug on bugzilla).

[hctim]

In D69825#1831691, @plotfi wrote:

In the meantime I could -fno-integrated-cc1 the driver.c InterfaceStubs tests to quite the bots if that is what people want (and file a bug on bugzilla).

Yes, please. A temporary workaround here that can be resolved later would be much appreciated to unblock our bots. Normally, I'd prefer a rollback-and-resubmit-with-fixes, but given that there have been multiple dependent changes submitted that would all have to be rolled back - whatever's the quickest way to get us back online without resorting to rolling back everything would be great :)

[hctim]

For tracking purposes.

The ASan leaks have been worked around in two separate patches in order to make the bots green again:

• c38e42527b21acee8d01a016d5bfa2fb83202e29
• e174da447c180b586719cb28f7bd556e30625762

@plotfi is graciously debugging :)

[hans]

In D69825#1837243, @hctim wrote:

For tracking purposes.

The ASan leaks have been worked around in two separate patches in order to make the bots green again:

• c38e42527b21acee8d01a016d5bfa2fb83202e29
• e174da447c180b586719cb28f7bd556e30625762

Thanks! I've cherry-picked these to the 10.x branch in 2dd6b91f35edb967f329f0437b53ea14395aa770 and bfaba51f07d1b79ff6e71da106c2b7e153874b1d

[thakis]

In D69825#1837243, @hctim wrote:

For tracking purposes.

The ASan leaks have been worked around in two separate patches in order to make the bots green again:

• c38e42527b21acee8d01a016d5bfa2fb83202e29
• e174da447c180b586719cb28f7bd556e30625762

@plotfi is graciously debugging :)

How is the debugging going? If it takes a while to fix, maybe it's better to make -emit-interface-stub imply -fno-integrated-cc1 until it's fixed for real, instead of adding that in the tests? As-is, the tests are happy but the feature is likely broken (?)

[erichkeane]

I've filed this bug against this patch: https://bugs.llvm.org/show_bug.cgi?id=44823

We've discovered in cases where multiple TUs are invoked in a single command line that the memory between calls isn't cleaned up like it was previously. This results in really high memory use from a compilation.

Experimentation has shown that removing the addition of the -disable-free flag from the CC1Command command lines fixes the issue. However, I believe we should only remove that in cases where it is NOT the last CC1 command (since the disable-free has some pretty solid performance improvements).

[plotfi]

@thakis I got sidetracked by some other things and had not made more progress, if progress is not made yes I think -emit-interface-stubs should imply -fno-integrated-cc1 if that seems like the right thing to do to you. @erichkeane Thanks for filing the bugzilla bug. I was having some problems with my bugzilla login myself and wasn't able to do so. I'll track progress if this bug and decide shortly if my feature should imply no-integraed-cc1. Thanks Guys.

In D69825#1859504, @thakis wrote:

In D69825#1837243, @hctim wrote:

For tracking purposes.

The ASan leaks have been worked around in two separate patches in order to make the bots green again:

• c38e42527b21acee8d01a016d5bfa2fb83202e29
• e174da447c180b586719cb28f7bd556e30625762

@plotfi is graciously debugging :)

How is the debugging going? If it takes a while to fix, maybe it's better to make -emit-interface-stub imply -fno-integrated-cc1 until it's fixed for real, instead of adding that in the tests? As-is, the tests are happy but the feature is likely broken (?)

[hans]

@erichkeane Thanks for filing the bugzilla bug. I was having some problems with my bugzilla login myself and wasn't able to do so. I'll track progress if this bug and decide shortly if my feature should imply no-integraed-cc1. Thanks Guys.

I think that bug was for the problem of high memory usage when doing multiple compilations in one clang invocation.

I've filed https://bugs.llvm.org/show_bug.cgi?id=44865 for the interface stubs vs asan issue to keep track of it easier. Please cc yourselves and track progress there.

[aganea]

Hello @plotfi and @hctim -- rG20f1abe306d0 should solve the initial issues you were seeing, please let me know if it doesn't.

[daltenty]

On AIX and PPC LE Linux after this change we are seeing invalid accesses when the backend asserts/fatal_errors. Looks like the driver and CC1 now share some global TimerGroup state that points to Timers which got created by CC1 but didn't get cleanup after the assert. Reported as: https://bugs.llvm.org/show_bug.cgi?id=45164

Example failure:

./bin/clang ./llvm-project/clang/test/CodeGenObjC/illegal-UTF8.m -S -target powerpc-ibm-aix
...
fatal error: error in backend: Unhandled linkage when mapping linkage to StorageClass.
Stack dump:
...
*******************
AddressSanitizer:DEADLYSIGNAL
=================================================================
==37262==ERROR: AddressSanitizer: SEGV on unknown address 0x000010715768 (pc 0x3fffb7cdb2c4 bp 0x3e1fb764cdb0 sp 0x3fffffffc460 T0)
==37262==The signal is caused by a UNKNOWN memory access.
    #0 0x3fffb7cdb2c0 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::string const&) (/lib64/libstdc++.so.6+0x11b2c0)
    #1 0x16ca75c0 in PrintRecord /home/daltenty/llvm/dev/llvm-project/llvm/include/llvm/Support/Timer.h:180:21
    #2 0x16ca75c0 in construct<llvm::TimerGroup::PrintRecord, llvm::TimeRecord &, std::basic_string<char> &, std::basic_string<char> &> /opt/rh/devtoolset-7/root/usr/lib/gcc/ppc64le-redhat-linux/7/../../../../include/c++/7/ext/new_allocator.h:136:23
    #3 0x16ca75c0 in construct<llvm::TimerGroup::PrintRecord, llvm::TimeRecord &, std::basic_string<char> &, std::basic_string<char> &> /opt/rh/devtoolset-7/root/usr/lib/gcc/ppc64le-redhat-linux/7/../../../../include/c++/7/bits/alloc_traits.h:475:8
    #4 0x16ca75c0 in void std::vector<llvm::TimerGroup::PrintRecord, std::allocator<llvm::TimerGroup::PrintRecord> >::_M_realloc_insert<llvm::TimeRecord&, std::string&, std::string&>(__gnu_cxx::__normal_iterator<llvm::TimerGroup::PrintRecord*, std::vector<llvm::TimerGroup::PrintRecord, std::allocator<llvm::TimerGroup::PrintRecord> > >, llvm::TimeRecord&, std::string&, std::string&) /opt/rh/devtoolset-7/root/usr/lib/gcc/ppc64le-redhat-linux/7/../../../../include/c++/7/bits/vector.tcc:415:4
    #5 0x16ca2220 in emplace_back<llvm::TimeRecord &, std::basic_string<char> &, std::basic_string<char> &> /opt/rh/devtoolset-7/root/usr/lib/gcc/ppc64le-redhat-linux/7/../../../../include/c++/7/bits/vector.tcc:105:4
    #6 0x16ca2220 in llvm::TimerGroup::prepareToPrintList(bool) /home/daltenty/llvm/dev/llvm-project/llvm/lib/Support/Timer.cpp:359:19
    #7 0x16ca2498 in llvm::TimerGroup::print(llvm::raw_ostream&, bool) /home/daltenty/llvm/dev/llvm-project/llvm/lib/Support/Timer.cpp:373:5
    #8 0x16ca2a4c in llvm::TimerGroup::printAll(llvm::raw_ostream&) /home/daltenty/llvm/dev/llvm-project/llvm/lib/Support/Timer.cpp:391:9
    #9 0x107f1940 in main /home/daltenty/llvm/dev/llvm-project/clang/tools/driver/driver.cpp:535:3
    #10 0x3fffb79b497c in generic_start_main.isra.0 (/lib64/libc.so.6+0x2497c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libstdc++.so.6+0x11b2c0) in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::string const&)
==37262==ABORTING

[hubert.reinterpretcast]

In D69825#1916865, @daltenty wrote:

On AIX and PPC LE Linux after this change we are seeing invalid accesses when the backend asserts/fatal_errors. Looks like the driver and CC1 now share some global TimerGroup state that points to Timers which got created by CC1 but didn't get cleanup after the assert. Reported as: https://bugs.llvm.org/show_bug.cgi?id=45164

From the bug report, it looks like we are discovering more problems associated with this patch that affect the 10.0 release. I've marked the bug as blocking release-10.0.0 for triage purposes for now.

[aganea]

Looking at the issue now, it should be a straightforward fix. Will send a patch ASAP.

[dim]

FWIW, this change causes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630, see in particular https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630#c18.

For some reason, not spawning a new process for the cc1 stage can lead to non-reproducible output. The exact cause is not known yet.

[hans]

In D69825#2063979, @dim wrote:

FWIW, this change causes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630, see in particular https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630#c18.

For some reason, not spawning a new process for the cc1 stage can lead to non-reproducible output. The exact cause is not known yet.

Can you share the preprocessed source and compiler command-line for printf.c, either on the bug or here?

[dim]

In D69825#2070926, @hans wrote:

In D69825#2063979, @dim wrote:

FWIW, this change causes https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630, see in particular https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630#c18.

For some reason, not spawning a new process for the cc1 stage can lead to non-reproducible output. The exact cause is not known yet.

Can you share the preprocessed source and compiler command-line for printf.c, either on the bug or here?

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246630#c26, and I've also uploaded the repro files to https://www.andric.com/freebsd/clang/bug246630-repro.tar.xz.

Running rGa3220dffcb1 under valgrind (on Linux) shows:

==120363== Conditional jump or move depends on uninitialised value(s)
==120363==    at 0x1634474: llvm::ConstantExpr::getGetElementPtr(llvm::Type*, llvm::Constant*, llvm::ArrayRef<llvm::Value*>, bool, llvm::Optional<unsigned int>, llvm::Type*) (Constants.cpp:2191)
==120363==    by 0x112D6D9: getGetElementPtr (Constants.h:1163)
==120363==    by 0x112D6D9: (anonymous namespace)::SymbolicallyEvaluateGEP(llvm::GEPOperator const*, llvm::ArrayRef<llvm::Constant*>, llvm::DataLayout const&, llvm::TargetLibraryInfo const*) (ConstantFolding.cpp:1005)
==120363==    by 0x112DF70: (anonymous namespace)::ConstantFoldInstOperandsImpl(llvm::Value const*, unsigned int, llvm::ArrayRef<llvm::Constant*>, llvm::DataLayout const&, llvm::TargetLibraryInfo const*) (ConstantFolding.cpp:1039)
==120363==    by 0x112C165: (anonymous namespace)::ConstantFoldConstantImpl(llvm::Constant const*, llvm::DataLayout const&, llvm::TargetLibraryInfo const*, llvm::SmallDenseMap<llvm::Constant*, llvm::Constant*, 4u, llvm::DenseMapInfo<llvm::Constant*>, llvm::detail::DenseMapPair<llvm::Constant*, llvm::Constant*> >&) [clone .part.0] (ConstantFolding.cpp:1114)
==120363==    by 0x112C5CF: llvm::ConstantFoldConstant(llvm::Constant const*, llvm::DataLayout const&, llvm::TargetLibraryInfo const*) (ConstantFolding.cpp:1194)
==120363==    by 0x188F410: prepareICWorklistFromFunction (InstructionCombining.cpp:3584)
==120363==    by 0x188F410: combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, llvm::BlockFrequencyInfo*, llvm::ProfileSummaryInfo*, unsigned int, llvm::LoopInfo*) (InstructionCombining.cpp:3703)
==120363==    by 0x189205F: runOnFunction (InstructionCombining.cpp:3789)
==120363==    by 0x189205F: llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) (InstructionCombining.cpp:3768)
==120363==    by 0x16F4352: llvm::FPPassManager::runOnFunction(llvm::Function&) (LegacyPassManager.cpp:1482)
==120363==    by 0x16F4DE8: llvm::FPPassManager::runOnModule(llvm::Module&) (LegacyPassManager.cpp:1518)
==120363==    by 0x16F51A2: runOnModule (LegacyPassManager.cpp:1583)
==120363==    by 0x16F51A2: llvm::legacy::PassManagerImpl::run(llvm::Module&) (LegacyPassManager.cpp:1695)
==120363==    by 0x1FF4CFE: EmitAssembly (BackendUtil.cpp:954)
==120363==    by 0x1FF4CFE: clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) (BackendUtil.cpp:1677)
==120363==    by 0x2C471A8: clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (CodeGenAction.cpp:335)
==120363==  Uninitialised value was created by a stack allocation
==120363==    at 0x112C653: (anonymous namespace)::SymbolicallyEvaluateGEP(llvm::GEPOperator const*, llvm::ArrayRef<llvm::Constant*>, llvm::DataLayout const&, llvm::TargetLibraryInfo const*) (ConstantFolding.cpp:832)

Trying to reduce this now.

[dim]

In D69825#2071687, @dim wrote:

Trying to reduce this now.

Unfortunately this turned out to be a red herring, as the test case got reduced by creduce to just:

a() { b(""); }

The valgrind warnings are also different when you build clang with clang, instead of gcc as I did first. In that case, you get dozens of these instead:

==525651== Conditional jump or move depends on uninitialised value(s)
==525651==    at 0xBB1D71: SimplifyAndInst(llvm::Value*, llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0xBBF6D0: ThreadBinOpOverPHI(llvm::Instruction::BinaryOps, llvm::Value*, llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0xBB1F87: SimplifyAndInst(llvm::Value*, llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x13898E8: llvm::InstCombiner::visitAnd(llvm::BinaryOperator&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x1365A6D: llvm::InstCombiner::run() (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x1367AEC: combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, llvm::BlockFrequencyInfo*, llvm::ProfileSummaryInfo*, bool, unsigned int, llvm::LoopInfo*) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x1369369: llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x121015B: llvm::FPPassManager::runOnFunction(llvm::Function&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x3BA5188: (anonymous namespace)::CGPassManager::runOnModule(llvm::Module&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x1210B3F: llvm::legacy::PassManagerImpl::run(llvm::Module&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x19A503A: clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream> >) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)
==525651==    by 0x2500BD4: clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (in /home/dim/obj/llvm-llvmorg-10.0.0-53-gf79cd71e145-linux5-x86_64-ninja-rel-1/bin/clang-10)

I'm unsure how to continue.

[dim]

For completeness sake, here are the valgrind outputs of clang rGf7f1abdb889 (llvmorg-11-init-16778) built by gcc 9.3.0-10ubuntu2 (in RelWithDebInfo mode):

llvmorg-11-init-16778-gf7f1abdb889-gcc-1.log5 KBDownload

And the exact same version of clang, built by clang 10.0.0-4ubuntu1:

llvmorg-11-init-16778-gf7f1abdb889-clang-1.log148 KBDownload

.

[dim]

Okay, after a long while of attempting to make a reproduction scenario, I finally managed one that consistently worked. As BSD make puts in different redirections for stderr when running in -j mode, it turned out that I could simply run the compilation twice on a terminal, first normally, then with stderr redirected to /dev/null, e.g. my test script is:

#!/bin/sh

exec </dev/tty >/dev/tty 2>&1

ulimit -c 0
trap "rm -f testcase[12].s testcase[12].s-* testcase-*.s.tmp" EXIT

${CLANG:-clang} -fintegrated-cc1 -Werror -O2 -std=gnu99 -fstack-protector-strong -S testcase.c -o testcase1.s || exit 1
${CLANG:-clang} -fintegrated-cc1 -Werror -O2 -std=gnu99 -fstack-protector-strong -S testcase.c -o testcase2.s 2>/dev/null || exit 1

cmp -s testcase1.s testcase2.s
test $? = 1 || exit 1

exit 0

(Here testcase.c is a copy of the .c file in https://www.andric.com/freebsd/clang/bug246630-repro.tar.xz)

Using this script I could finally bisect, and arrived at the following commit which fixes the issue, i.e. it makes the outputs of both clang runs the same again, and it is rG0cecafd647cc ("[BasicAA] Make BasicAA a cfg pass"). @asbirlea seems to indicate in the commit message that it influences the way the BasicAA pass is freed/invalidaed or not during runs?

So is this commit fixing it as an unintended side effect, or would it be according to expectation? If the latter, we should merge it into 10.0.1, and I will create a PR for it.

clang/CMakeLists.txt

 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
 
 set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
     "enable x86 relax relocations by default")
 
 set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
   "Enable the experimental new pass manager by default.")
 
+set(CLANG_SPAWN_CC1 OFF CACHE BOOL
+    "Whether clang should use a new process for the CC1 invocation")
+
 # TODO: verify the values against LangStandards.def?
 set(CLANG_DEFAULT_STD_C "" CACHE STRING
   "Default standard to use for C/ObjC code (IDENT from LangStandards.def, empty for platform default)")
 set(CLANG_DEFAULT_STD_CXX "" CACHE STRING
   "Default standard to use for C++/ObjC++ code (IDENT from LangStandards.def, empty for platform default)")
 
 set(CLANG_DEFAULT_LINKER "" CACHE STRING
   "Default linker to use (linker name or absolute path, empty for platform default)")

clang/include/clang/Config/config.h.cmake

 /* Enable the experimental new pass manager by default */
 #cmakedefine01 ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
 
 /* Enable each functionality of modules */
 #cmakedefine01 CLANG_ENABLE_ARCMT
 #cmakedefine01 CLANG_ENABLE_OBJC_REWRITER
 #cmakedefine01 CLANG_ENABLE_STATIC_ANALYZER
 
+/* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
+#cmakedefine01 CLANG_SPAWN_CC1
+
 #endif

clang/include/clang/Driver/Driver.h

public:
   /// Set CC_LOG_DIAGNOSTICS mode, which causes the frontend to log diagnostics
   /// to CCLogDiagnosticsFilename or to stderr, in a stable machine readable
   /// format.
   unsigned CCLogDiagnostics : 1;
 
   /// Whether the driver is generating diagnostics for debugging purposes.
   unsigned CCGenDiagnostics : 1;
 
+  /// Pointer to the ExecuteCC1Tool function, if available.
+  /// When the clangDriver lib is used through clang.exe, this provides a
+  /// shortcut for executing the -cc1 command-line directly, in the same
+  /// process.
+  typedef int (*CC1ToolFunc)(ArrayRef<const char *> argv);
+  CC1ToolFunc CC1Main = nullptr;
+
 private:
   /// Raw target triple.
   std::string TargetTriple;
 
   /// Name to use when invoking gcc/g++.
   std::string CCCGenericGCCName;
 
   /// Name of configuration file if used.

clang/include/clang/Driver/Job.h

public:
   void setInputFileList(llvm::opt::ArgStringList List) {
     InputFileList = std::move(List);
   }
 
   /// Sets the environment to be used by the new process.
   /// \param NewEnvironment An array of environment variables.
   /// \remark If the environment remains unset, then the environment
   ///         from the parent process will be used.
-  void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment);
+  virtual void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment);
 
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }
 
   /// Print a command argument, and optionally quote it.
   static void printArg(llvm::raw_ostream &OS, StringRef Arg, bool Quote);
 
   /// Set whether to print the input filenames when executing.
   void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
+
+protected:
+  /// Optionally print the filenames to be compiled
+  void PrintFileNames() const;
+};
+
+/// Use the CC1 tool callback when available, to avoid creating a new process
+class CC1Command : public Command {
+public:
+  using Command::Command;
+
+  void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
+             CrashReportInfo *CrashInfo = nullptr) const override;
+
+  int Execute(ArrayRef<Optional<StringRef>> Redirects, std::string *ErrMsg,
+              bool *ExecutionFailed) const override;
+
+  void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment) override;
 };
 
 /// Like Command, but with a fallback which is executed in case
 /// the primary command crashes.
 class FallbackCommand : public Command {
 public:
   FallbackCommand(const Action &Source_, const Tool &Creator_,
                   const char *Executable_,

clang/lib/Driver/Job.cpp

 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <string>
 #include <system_error>
 #include <utility>

 }
 
 void Command::setEnvironment(llvm::ArrayRef<const char *> NewEnvironment) {
   Environment.reserve(NewEnvironment.size() + 1);
   Environment.assign(NewEnvironment.begin(), NewEnvironment.end());
   Environment.push_back(nullptr);
 }
 
-int Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
-                     std::string *ErrMsg, bool *ExecutionFailed) const {
+void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
     for (const char *Arg : InputFilenames)
       llvm::outs() << llvm::sys::path::filename(Arg) << "\n";
     llvm::outs().flush();
   }
-
-  SmallVector<const char*, 128> Argv;
-
-  Optional<ArrayRef<StringRef>> Env;
-  std::vector<StringRef> ArgvVectorStorage;
-  if (!Environment.empty()) {
-    assert(Environment.back() == nullptr &&
-           "Environment vector should be null-terminated by now");
-    ArgvVectorStorage = llvm::toStringRefArray(Environment.data());
-    Env = makeArrayRef(ArgvVectorStorage);
-  }
 
+int Command::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
+                     std::string *ErrMsg, bool *ExecutionFailed) const {
+  PrintFileNames();
+
+  SmallVector<const char *, 128> Argv;
   if (ResponseFile == nullptr) {
     Argv.push_back(Executable);
     Argv.append(Arguments.begin(), Arguments.end());
     Argv.push_back(nullptr);
-
-    auto Args = llvm::toStringRefArray(Argv.data());
-    return llvm::sys::ExecuteAndWait(
-        Executable, Args, Env, Redirects, /*secondsToWait*/ 0,
-        /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
-  }
-
-  // We need to put arguments in a response file (command is too large)
-  // Open stream to store the response file contents
+  } else {
+    // If the command is too large, we need to put arguments in a response file.
     std::string RespContents;
     llvm::raw_string_ostream SS(RespContents);
 
     // Write file contents and build the Argv vector
     writeResponseFile(SS);
     buildArgvForResponseFile(Argv);
     Argv.push_back(nullptr);
     SS.flush();
 
     // Save the response file in the appropriate encoding
     if (std::error_code EC = writeFileWithEncoding(
             ResponseFile, RespContents, Creator.getResponseFileEncoding())) {
       if (ErrMsg)
         *ErrMsg = EC.message();
       if (ExecutionFailed)
         *ExecutionFailed = true;
+      // Return -1 by convention (see llvm/include/llvm/Support/Program.h) to
+      // indicate the requested executable cannot be started.
       return -1;
     }
+  }
+
+  Optional<ArrayRef<StringRef>> Env;
+  std::vector<StringRef> ArgvVectorStorage;
+  if (!Environment.empty()) {
+    assert(Environment.back() == nullptr &&
+           "Environment vector should be null-terminated by now");
+    ArgvVectorStorage = llvm::toStringRefArray(Environment.data());
+    Env = makeArrayRef(ArgvVectorStorage);
+  }
 
   auto Args = llvm::toStringRefArray(Argv.data());
   return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects,
                                    /*secondsToWait*/ 0,
                                    /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
 }
 
+void CC1Command::Print(raw_ostream &OS, const char *Terminator, bool Quote,
+                       CrashReportInfo *CrashInfo) const {
+  OS << " (in-process)";
+  Command::Print(OS, Terminator, Quote, CrashInfo);
+}
+
+int CC1Command::Execute(ArrayRef<llvm::Optional<StringRef>> /*Redirects*/,
+                        std::string *ErrMsg, bool *ExecutionFailed) const {
+  PrintFileNames();
+
+  SmallVector<const char *, 128> Argv;
+  Argv.push_back(getExecutable());
+  Argv.append(getArguments().begin(), getArguments().end());
+  Argv.push_back(nullptr);
+
+  // This flag simply indicates that the program couldn't start, which isn't
+  // applicable here.
+  if (ExecutionFailed)
+    *ExecutionFailed = false;
+
+  llvm::CrashRecoveryContext CRC;
+  CRC.DumpStackAndCleanupOnFailure = true;
+
+  const void *PrettyState = llvm::SavePrettyStackState();
+  const Driver &D = getCreator().getToolChain().getDriver();
+
+  int R = 0;
+  // Enter ExecuteCC1Tool() instead of starting up a new process
+  if (!CRC.RunSafely([&]() { R = D.CC1Main(Argv); })) {
+    llvm::RestorePrettyStackState(PrettyState);
+    return CRC.RetCode;
+  }
+  return R;
+}
+
+void CC1Command::setEnvironment(llvm::ArrayRef<const char *> NewEnvironment) {
+  // We don't support set a new environment when calling into ExecuteCC1Tool()
+  llvm_unreachable(
+      "The CC1Command doesn't support changing the environment vars!");
+}
+
 FallbackCommand::FallbackCommand(const Action &Source_, const Tool &Creator_,
                                  const char *Executable_,
                                  const llvm::opt::ArgStringList &Arguments_,
                                  ArrayRef<InputInfo> Inputs,
                                  std::unique_ptr<Command> Fallback_)
     : Command(Source_, Creator_, Executable_, Arguments_, Inputs),
       Fallback(std::move(Fallback_)) {}
 
 void FallbackCommand::Print(raw_ostream &OS, const char *Terminator,
                             bool Quote, CrashReportInfo *CrashInfo) const {
   Command::Print(OS, "", Quote, CrashInfo);
   OS << " ||";
   Fallback->Print(OS, Terminator, Quote, CrashInfo);
 }
 
 static bool ShouldFallback(int ExitCode) {
   // FIXME: We really just want to fall back for internal errors, such
   // as when some symbol cannot be mangled, when we should be able to
if (ErrMsg)
     ErrMsg->clear();
   if (ExecutionFailed)
     *ExecutionFailed = false;
 
   const Driver &D = getCreator().getToolChain().getDriver();
   D.Diag(diag::warn_drv_invoking_fallback) << Fallback->getExecutable();
 
   int SecondaryStatus = Fallback->Execute(Redirects, ErrMsg, ExecutionFailed);
   return SecondaryStatus;
 }
 
 ForceSuccessCommand::ForceSuccessCommand(
     const Action &Source_, const Tool &Creator_, const char *Executable_,
     const llvm::opt::ArgStringList &Arguments_, ArrayRef<InputInfo> Inputs)
     : Command(Source_, Creator_, Executable_, Arguments_, Inputs) {}
 
 void ForceSuccessCommand::Print(raw_ostream &OS, const char *Terminator,

clang/lib/Driver/ToolChains/Clang.cpp

if (Args.hasArg(options::OPT__SLASH_fallback) &&
     C.addCommand(std::make_unique<FallbackCommand>(
         JA, *this, Exec, CmdArgs, Inputs, std::move(CLCommand)));
   } else if (Args.hasArg(options::OPT__SLASH_fallback) &&
              isa<PrecompileJobAction>(JA)) {
     // In /fallback builds, run the main compilation even if the pch generation
     // fails, so that the main compilation's fallback to cl.exe runs.
     C.addCommand(std::make_unique<ForceSuccessCommand>(JA, *this, Exec,
                                                         CmdArgs, Inputs));
+  } else if (D.CC1Main && !D.CCGenDiagnostics) {
+    // Invoke the CC1 directly in this process
+    C.addCommand(
+        std::make_unique<CC1Command>(JA, *this, Exec, CmdArgs, Inputs));
   } else {
     C.addCommand(std::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
   }
 
   // Make the compile command echo its inputs for /showFilenames.
   if (Output.getType() == types::TY_Object &&
       Args.hasFlag(options::OPT__SLASH_showFilenames,
                    options::OPT__SLASH_showFilenames_, false)) {

clang/test/CMakeLists.txt

 # Test runner infrastructure for Clang. This configures the Clang test trees
 # for use by Lit, and delegates to LLVM's lit test handlers.
 
 if (CMAKE_CFG_INTDIR STREQUAL ".")
   set(LLVM_BUILD_MODE ".")
 else ()
   set(LLVM_BUILD_MODE "%(build_mode)s")
 endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR})
 
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
+  CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)
 
 configure_lit_site_cfg(

clang/test/Driver/clang_f_opts.c

 
 // RUN: %clang -### -S -fno-unsigned-char %s 2>&1 | FileCheck -check-prefix=CHAR-SIGN4 %s
 // CHAR-SIGN4-NOT: -fno-signed-char
 
 // RUN: %clang -target x86_64-unknown-none-none -### -fshort-wchar -fno-short-wchar %s 2>&1 | FileCheck -check-prefix=CHECK-WCHAR1 -check-prefix=DELIMITERS %s
 // RUN: %clang -target x86_64-unknown-none-none -### -fno-short-wchar -fshort-wchar %s 2>&1 | FileCheck -check-prefix=CHECK-WCHAR2 -check-prefix=DELIMITERS %s
 // Make sure we don't match the -NOT lines with the linker invocation.
 // Delimiters match the start of the cc1 and the start of the linker lines
-// DELIMITERS: {{^ *"}}
+// DELIMITERS: {{^ (\(in-process\)|")}}
 // CHECK-WCHAR1: -fwchar-type=int
 // CHECK-WCHAR1-NOT: -fwchar-type=short
 // CHECK-WCHAR2: -fwchar-type=short
 // CHECK-WCHAR2-NOT: -fwchar-type=int
 // DELIMITERS: {{^ *"}}
 
 // RUN: %clang -### -fno-experimental-new-pass-manager -fexperimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=CHECK-PM --check-prefix=CHECK-NEW-PM %s
 // RUN: %clang -### -fexperimental-new-pass-manager -fno-experimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=CHECK-PM --check-prefix=CHECK-NO-NEW-PM %s

 // RUN: %clang -### -S -ftrivial-auto-var-init=zero %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-ZERO-BAD %s
 // CHECK-TRIVIAL-UNINIT-NOT: hasn't been enabled
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-BAD: hasn't been enabled
 
 // RUN: %clang -### -S -fno-temp-file %s 2>&1 | FileCheck -check-prefix=CHECK-NO-TEMP-FILE %s
 // CHECK-NO-TEMP-FILE: "-fno-temp-file"
 No newline at end of file

clang/test/Driver/fsanitize-blacklist.c

 // General blacklist usage.
 
 // PR12920
 // REQUIRES: clang-driver
 
 // Make sure we don't match the -NOT lines with the linker invocation.
 // Delimiters match the start of the cc1 and the start of the linker lines
 // for fragile tests.
-// DELIMITERS: {{^ *"}}
+// DELIMITERS: {{^ (\(in-process\)|")}}
 
 // RUN: echo "fun:foo" > %t.good
 // RUN: echo "fun:bar" > %t.second
 // RUN: echo "badline" > %t.bad
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=hwaddress -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
 // CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second

clang/test/Driver/unknown-arg.c

 // CL-ERROR: error: unknown argument ignored in clang-cl: '-munknown-to-clang-option'
 // CL-ERROR: error: unknown argument ignored in clang-cl: '-print-stats'
 // CL-ERROR: error: unknown argument ignored in clang-cl: '-funknown-to-clang-option'
 // CL-ERROR-DID-YOU-MEAN: error: unknown argument ignored in clang-cl '-helo'; did you mean '-help'?
 // SILENT-NOT: error:
 // SILENT-NOT: warning:
 // CC1AS-DID-YOU-MEAN: error: unknown argument '-hell'; did you mean '-help'?
 // CC1AS-DID-YOU-MEAN: error: unknown argument '--version'; did you mean '-version'?
-// UNKNOWN-INTEGRATED: error: unknown integrated tool 'asphalt'. Valid tools include '-cc1' and '-cc1as'.
+// UNKNOWN-INTEGRATED: error: unknown integrated tool '-cc1asphalt'. Valid tools include '-cc1' and '-cc1as'.
 
 // RUN: %clang -S %s -o %t.s  -Wunknown-to-clang-option 2>&1 | FileCheck --check-prefix=IGNORED %s
 
 // IGNORED: warning: unknown warning option '-Wunknown-to-clang-option'

clang/test/Driver/warning-options_pedantic.cpp

 // Make sure we don't match the -NOT lines with the linker invocation.
 // Delimiters match the start of the cc1 and the start of the linker lines
-// DELIMITERS: {{^ *"}}
+// DELIMITERS: {{^ (\(in-process\)|")}}
 
 // RUN: %clang -### -pedantic -no-pedantic %s 2>&1 | FileCheck -check-prefix=NO_PEDANTIC -check-prefix=DELIMITERS %s
 // RUN: %clang -### -pedantic -Wno-pedantic %s 2>&1 | FileCheck -check-prefix=PEDANTIC -check-prefix=DELIMITERS %s
 // NO_PEDANTIC-NOT: -pedantic
 // RUN: %clang -### -pedantic -pedantic -no-pedantic -pedantic %s 2>&1 | FileCheck -check-prefix=PEDANTIC -check-prefix=DELIMITERS %s
 // RUN: %clang -### -pedantic -pedantic -no-pedantic -Wpedantic %s 2>&1 | FileCheck -check-prefix=NO_PEDANTIC -check-prefix=DELIMITERS %s
 // PEDANTIC: -pedantic
 // REQUIRES: clang-driver
 
 // DELIMITERS: {{^ *"}}

clang/tools/driver/driver.cpp

 //===-- driver.cpp - Clang GCC-Compatible Driver --------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This is the entry point to the clang driver; it is a thin wrapper
 // for functionality in the Driver clang library.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Driver.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/Stack.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Frontend/ChainedDiagnosticConsumer.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/SerializedDiagnosticPrinter.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/Utils.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
static void getCLEnvVarOptions(std::string &EnvValue, llvm::StringSaver &Saver,
                                SmallVectorImpl<const char *> &Opts) {
   llvm::cl::TokenizeWindowsCommandLine(EnvValue, Saver, Opts);
   // The first instance of '#' should be replaced with '=' in each option.
   for (const char *Opt : Opts)
     if (char *NumberSignPtr = const_cast<char *>(::strchr(Opt, '#')))
       *NumberSignPtr = '=';
 }
 
+static int ExecuteCC1Tool(ArrayRef<const char *> argv);
+
 static void SetBackdoorDriverOutputsFromEnvVars(Driver &TheDriver) {
   // Handle CC_PRINT_OPTIONS and CC_PRINT_OPTIONS_FILE.
   TheDriver.CCPrintOptions = !!::getenv("CC_PRINT_OPTIONS");
   if (TheDriver.CCPrintOptions)
     TheDriver.CCPrintOptionsFilename = ::getenv("CC_PRINT_OPTIONS_FILE");
 
   // Handle CC_PRINT_HEADERS and CC_PRINT_HEADERS_FILE.
   TheDriver.CCPrintHeaders = !!::getenv("CC_PRINT_HEADERS");
   if (TheDriver.CCPrintHeaders)
     TheDriver.CCPrintHeadersFilename = ::getenv("CC_PRINT_HEADERS_FILE");
 
   // Handle CC_LOG_DIAGNOSTICS and CC_LOG_DIAGNOSTICS_FILE.
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
     TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Whether the cc1 tool should be called inside the current process, or if we
+  // should spawn a new clang process (old behavior).
+  // Not having an additional process saves some execution time of Windows,
+  // and makes debugging easier.
+  bool UseNewCC1Process = CLANG_SPAWN_CC1;
+
+  StringRef SpawnCC1Str = ::getenv("CLANG_SPAWN_CC1");
+  if (!SpawnCC1Str.empty()) {
+    unsigned long long Enable;
+    if (getAsUnsignedInteger(SpawnCC1Str, 10, Enable) || Enable > 1) {
+      llvm::errs() << "error: the value of the environment variable "
+                      "CLANG_SPAWN_CC1 must be either 0 or 1.\n";
+      ::exit(1);
+    }
+    UseNewCC1Process = Enable;
+  }
+  if (!UseNewCC1Process) {
+    TheDriver.CC1Main = &ExecuteCC1Tool;
+    // Ensure the CC1Command actually catches cc1 crashes
+    llvm::CrashRecoveryContext::Enable();
+  }
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
                                    const std::string &Path) {
   // If the clang binary happens to be named cl.exe for compatibility reasons,
   // use clang-cl.exe as the prefix to avoid confusion between clang and MSVC.
   StringRef ExeBasename(llvm::sys::path::stem(Path));
   if (ExeBasename.equals_lower("cl"))
static void SetInstallDir(SmallVectorImpl<const char *> &argv,
   if (CanonicalPrefixes)
     llvm::sys::fs::make_absolute(InstalledPath);
 
   StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath));
   if (llvm::sys::fs::exists(InstalledPathParent))
     TheDriver.setInstalledDir(InstalledPathParent);
 }
 
-static int ExecuteCC1Tool(ArrayRef<const char *> argv, StringRef Tool) {
+static int ExecuteCC1Tool(ArrayRef<const char *> argv) {
+  // If we call the cc1 tool from the clangDriver library (through
+  // Driver::CC1Main), we need to cleanup the options usage count. The options
+  // are currently global, and they might have been used previously by the
+  // driver.
+  llvm::cl::ResetAllOptionOccurrences();
+  StringRef Tool = argv[1];
   void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath;
-  if (Tool == "")
+  if (Tool == "-cc1")
     return cc1_main(argv.slice(2), argv[0], GetExecutablePathVP);
-  if (Tool == "as")
+  if (Tool == "-cc1as")
     return cc1as_main(argv.slice(2), argv[0], GetExecutablePathVP);
-  if (Tool == "gen-reproducer")
+  if (Tool == "-cc1gen-reproducer")
     return cc1gen_reproducer_main(argv.slice(2), argv[0], GetExecutablePathVP);
 
   // Reject unknown tools.
   llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
                << "Valid tools include '-cc1' and '-cc1as'.\n";
   return 1;
 }
 
 int main(int argc_, const char **argv_) {
   noteBottomOfStack();
   llvm::InitLLVM X(argc_, argv_);
   SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
     return 1;
 
   llvm::InitializeAllTargets();
   auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
   // Parse response files using the GNU syntax, unless we're in CL mode. There
   // are two ways to put clang in CL compatibility mode: argv[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
   // command line parsing can't happen until after response file parsing, so we
   // have to manually search for a --driver-mode=cl argument the hard way.
   // Finally, our -cc1 tools don't care which tokenization mode we use because
   // response files written by clang will tokenize the same way in either mode.
   bool ClangCLMode = false;
int main(int argc_, const char **argv_) {
   auto FirstArg = std::find_if(argv.begin() + 1, argv.end(),
                                [](const char *A) { return A != nullptr; });
   if (FirstArg != argv.end() && StringRef(*FirstArg).startswith("-cc1")) {
     // If -cc1 came from a response file, remove the EOL sentinels.
     if (MarkEOLs) {
       auto newEnd = std::remove(argv.begin(), argv.end(), nullptr);
       argv.resize(newEnd - argv.begin());
     }
-    return ExecuteCC1Tool(argv, argv[1] + 4);
+    return ExecuteCC1Tool(argv);
   }
 
   bool CanonicalPrefixes = true;
   for (int i = 1, size = argv.size(); i < size; ++i) {
     // Skip end-of-line response file markers
     if (argv[i] == nullptr)
       continue;
     if (StringRef(argv[i]) == "-no-canonical-prefixes") {
#endif
 
   // If any timers were active but haven't been destroyed yet, print their
   // results now.  This happens in -disable-free mode.
   llvm::TimerGroup::printAll(llvm::errs());
   llvm::TimerGroup::clearAll();
 
 #ifdef _WIN32
   // Exit status should not be negative on Win32, unless abnormal termination.
-  // Once abnormal termiation was caught, negative status should not be
+  // Once abnormal termination was caught, negative status should not be
   // propagated.
   if (Res < 0)
     Res = 1;
 #endif
 
   // If we have multiple failing commands, we return the result of the first
   // failing command.
   return Res;
 }