================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
