[clang] [llvm] [clang] Allow enabling sandbox for direct `-cc1` invocations (PR #174653)

[Jan Svoboda via cfe-commits]

================
@@ -264,8 +265,10 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
   }
 
   // Handle -cc1 integrated tools.
-  if (Args.size() >= 2 && StringRef(Args[1]).starts_with("-cc1"))
+  if (Args.size() >= 2 && StringRef(Args[1]).starts_with("-cc1")) {
+    auto EnableSandbox = llvm::sys::sandbox::scopedEnable();
----------------
jansvoboda11 wrote:

You're raising a valid point. I looked into this more closely and there seem to be 5 ways `cc1_main()` can be reached.

1. `clang -cc1 -emit-obj` - frontend invocation,
2. `clang -fintegrated-cc1 -c` - driver invocation with integrated -cc1 and single job,
3. `clang -fintegrated-cc1` - driver invocation with integrated -cc1 and multiple jobs,
4. `clang -fno-integrated-cc1 -c` - driver invocation without integrated -cc1 and single job,
5. `clang -fino-ntegrated-cc1` - driver invocation without integrated -cc1 and multiple jobs.

---

This PR clearly handles case (1):
* `clang_main()`
* `ExecuteCC1Tool()`
* `cc1_main()`

---

The original PR [#165350](https://github.com/llvm/llvm-project/pull/165350) handled case (2):
   * `clang_main()`
   * `Driver::ExecuteCompilation()`
   * ...
   * `CC1Command::Execute()`

The original PR enabled the sandbox in `CC1Command::Execute()` which is reached like so:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/tools/driver/driver.cpp#L371-L372

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/ToolChains/Clang.cpp#L7985-L7989

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/Job.cpp#L432

---

Interestingly, in case (3), with multiple jobs generated by the driver, this code:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/Driver.cpp#L5348-L5352

makes `CC1Command` forward to the regular (out-of-process) `Command`:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/Job.cpp#L410-L414

This means the new `-cc1` process will go down the path this PR handles and the sandbox will be enabled.

---

The same thing then happens in cases (4) and (5), without integrated `-cc1`, where this branch is not entered:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/tools/driver/driver.cpp#L371-L372

and then Clang creates a plain `Command` instead of `CC1Command`:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/ToolChains/Clang.cpp#L7990-L7994

and again, the new `-cc1` process will go down the path handled in this PR.

---

So I believe this PR makes sandboxing of `-cc1` commands work as intended, but maybe in a somewhat convoluted way. I think a simpler way to reliably enable sandbox for `cc1_main()` might be to do so right at the start of `cc1_main()` itself, regardless of how it was reached. However, I think this call to `scopedEnable()`:

https://github.com/llvm/llvm-project/blob/52f85b0159d7f9cd3f21d644e92684afc53f0ed1/clang/lib/Driver/Job.cpp#L430-L432

is still necessary, so that in case `cc1_main()` crashes and doesn't get a chance to restore the original sandbox setting itself in case (2), the sandbox is not unintentionally left enabled for the rest of the driver process. (But maybe we should consider moving the "restore previous sandbox setting" logic into `llvm::CrashRecoveryContext` that gets used for this case?)

Let me know what you think @benlangmuir.

https://github.com/llvm/llvm-project/pull/174653