[RFC] File system sandboxing in Clang/LLVM

[jansvoboda11]

TL;DR

Proposing to set up a file system sandbox by default for developer builds that enforces use of vfs::FileSystem instead of direct llvm::sys::fs and llvm::MemoryBuffer::getFile*() calls, preventing new code from bypassing file system virtualization.

Background

In 2014, Clang gained the ability to virtualize file system reads through the vfs::FileSystem interface, mainly in support of the new -ivfsoverlay option. In 2018, the API was lifted from Clang into LLVM for use by other projects.

Until recently, many parts of Clang and LLVM were still using the llvm::sys::fs and llvm::MemoryBuffer::getFile*() APIs directly instead of going through the interface. This inconsistency means that different parts of the compiler see the file system differently.

Over the past couple of weeks, I’ve been working towards Clang and LLVM having a consistent view of the file system by adopting the vfs::FileSystem interface. Now, I would like to ensure that no more new direct uses of llvm::sys::fs and llvm::MemoryBuffer::getFile*() make it into the compiler.

Proposal

Set up a sandbox for file system reads within Clang itself. The idea is that for developer builds, the compiler aborts whenever a direct use of the discouraged APIs occurs, forcing LLVM/Clang developers to use the vfs::FileSystem interface instead. This behavior can be turned off with the CMake option -DLLVM_ENABLE_IO_SANDBOX=NO if needed.

Motivation

My team at Apple is working on sound compilation caching in Clang. Our implementation relies on a fast scanning step that builds up a CAS database containing all the input files necessary for compilation. During the compilation itself, Clang loads the database and the contained file system snapshot is exposed via the vfs::FileSystem API.

Not using vfs::FileSystem consistently in the compiler breaks CAS-based caching. For example, in a distributed setting, circumventing the CAS database means going to the actual builder file system that wasn’t set up with the expected files, resulting in compilation failures or unexpected compiler behavior.

Implementation

My PR #165350 contains an initial implementation that calls reportFatalInternalError() whenever the discouraged APIs are used directly. This behavior is enabled by default for asserts builds and can be controlled with new CMake option -DLLVM_ENABLE_IO_SANDBOX.

The sandbox is currently only applied to clang -cc1 and clang -cc1as invocations; Clang driver itself and other binaries remain unaffected for now.

By turning on the sandbox for local development and in pre-merge CI, we can prevent Clang/LLVM developers from introducing more non-virtualized file system reads into the codebase. Over time, we can work on eliminating the remaining violations that are not reachable by running the check-clang target.

Developer Impact

When the sandbox is enabled (default for assert builds):

• Tests will fail if new code uses llvm::sys::fs or llvm::MemoryBuffer::getFile*() directly.
• Developers must use the vfs::FileSystem interface or explicitly use the bypass mechanism.
• The sandbox can be disabled with -DLLVM_ENABLE_IO_SANDBOX=NO if needed.
• Only affects clang -cc1 and clang -cc1as invocations.

This should not affect release builds or production use.

Bypass Mechanism

For specific areas where we are virtualizing compiler inputs in a different way (such as the module cache), and for incremental adoption, there is a bypass mechanism that:

• Must be applied locally at the call site.
• Should be easy to grep and audit over time.
• Requires justification during code review.

This allows intentional direct file system access where necessary while keeping it explicit and auditable.

Example of sandbox bypass:

// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
auto BufOrErr = llvm::MemoryBuffer::getFile(Path);

Current Status

Right now, the upstream repo with the sandbox enabled passes the pre-commit checks on all platforms. I was also able to bootstrap an Apple Clang toolchain and build a handful of internal projects with the sandbox enabled. This demonstrates that the sandbox does not break existing tested code paths in multiple build and test configurations.

Migration Path

1. Phase 1 (this RFC): Enable sandbox in assert builds to prevent new violations.
2. Phase 2: Identify and fix remaining violations in untested code paths.
3. Phase 3: Add clang-tidy check for static verification that non-virtualized file system reads in Clang and LLVM only occur with the explicit bypass mechanism applied locally.
4. Future: Extend sandbox to other tools beyond clang -cc1 and clang -cc1as.
5. Future: Apply similar approach to file system writes (see below).

File System Writes

Clang has the same problem with compiler outputs / file system writes. Some parts of the Clang use the new vfs::OutputBackend interface, but some still create raw_fd_ostream and similar directly. With our caching implementation this means that replaying a cached compilation may not materialize all outputs.

This should be unified as well, but I consider that a separate follow-up project.

Alternatives Considered

I have considered building a file system sandbox outside of the compiler, but that would most likely result in platform-specific solutions that cannot be broadly applied during development upstream. I believe that building this infrastructure in the most cross-platform way and in the upstream repository provides the greatest benefit to the community.

Questions?

I welcome any feedback and questions on this approach. The key decision point is whether the community is comfortable with enabling the sandbox for assert builds by default, with the understanding that some compilations may fail due to sandbox violations that haven’t been fixed yet in untested code paths. The sandbox can always be disabled with -DLLVM_ENABLE_IO_SANDBOX=NO.

this RFC was accepted in this message.

[AaronBallman]

Thanks for the RFC! I like the overall goal. Some questions I have are:

• What’s the compile time performance impact of requiring the virtual file system? (Some folks use Debug mode as a daily driver, will they see a noticeable slowdown when running lit tests, for example)
• Will this be enabled on at least one post-commit buildbot so people get an email notification if they broke something?
• Have you considered making this a CMake option that’s disabled by default but is explicitly enabled for a precommit CI workload so developers get an alert when posting a PR? Basically, do that until Phase 3 is complete and we no longer have unfixed violations or tools to help catch them statically, then make it the default for everyone at that point?

I’m mostly worried about the amount of pain will be caused by the remaining violations in untested code paths. The less we make people muck around with CMake variables, the better IMO.

[jansvoboda11]

@AaronBallman

• What’s the compile time performance impact of requiring the virtual file system? (Some folks use Debug mode as a daily driver, will they see a noticeable slowdown when running lit tests, for example)

I don’t know to be honest. I suspect for compilations only using the vfs::getRealFileSystem(), the overhead won’t be measurable, since we’re talking about one virtual function call before performing an IO syscall. For compilations making heavy use of -ivfsoverlay or other VFSs I’m less confident. I can create a benchmark to figure that out, if you think that’s an important aspect of the RFC.

@AaronBallman

• Will this be enabled on at least one post-commit buildbot so people get an email notification if they broke something?

I was assuming that by enabling the IO sandbox whenever assertions are enabled most post-commit bots will pick this up automatically.

@AaronBallman

• Have you considered making this a CMake option that’s disabled by default but is explicitly enabled for a precommit CI workload so developers get an alert when posting a PR? Basically, do that until Phase 3 is complete and we no longer have unfixed violations or tools to help catch them statically, then make it the default for everyone at that point?

Not until now. My thinking was that the sooner developers get to see a test failing, the more power to investigate they have and the less confusing it ends up being. I imagine it might be hard to spot that the pre-commit bots are building with a different CMake preset. I’m open to discussion, though.

@AaronBallman

I’m mostly worried about the amount of pain will be caused by the remaining violations in untested code paths. The less we make people muck around with CMake variables, the better IMO.

That’s a good point. I’d be interested to hear from others whether they’d prefer to get failures in pre-commit testing and then have to figure out the CMake variables they need to reproduce the sandbox violation, or see the sandbox violations by default whenever assertions are enabled.

Thanks for the feedback!

[Endill]

@AaronBallman

Have you considered making this a CMake option that’s disabled by default but is explicitly enabled for a precommit CI workload so developers get an alert when posting a PR? Basically, do that until Phase 3 is complete and we no longer have unfixed violations or tools to help catch them statically, then make it the default for everyone at that point?

I’m not sure this would help the goal of not making people think of CMake variables, because if I see a relevant CI failure, I’m very inclined to reproduce the failure locally, so that I can debug it with a debugger and overall have a faster development cycle with less friction. Which means that for every CI failure caused by this I’d need to tinker with CMake variables to enable it.

Whereas if it’s enabled together with assertions, I get it automatically with a debug binary, and don’t need to think about CMake variables at all.

@jansvoboda11

I can create a benchmark to figure that out, if you think that’s an important aspect of the RFC.

I think it is important, because if you find it’s expensive, you should default not to assertions being enabled, but expensive checks.

[boomanaiden154-1]

Soft -1 on a CMake option that is only enabled for premerge. I don’t think that gives us any meaningful additional test coverage, especially if check-clang and building the runtimes is already clean (which I assume it would be). It definitely doesn’t help to catch the long tail issues that I think we do want to catch.

I think we just enable this by default (I’m assuming the real file system case is the default unless someone explicitly sets it otherwise, so enabling should be negligible performance wise) and see what happens.

[AaronBallman]

@jansvoboda11

I can create a benchmark to figure that out, if you think that’s an important aspect of the RFC.

I don’t know if it’s critical, but it would be good to have an understanding of the impacts.

@jansvoboda11

Not until now. My thinking was that the sooner developers get to see a test failing, the more power to investigate they have and the less confusing it ends up being. I imagine it might be hard to spot that the pre-commit bots are building with a different CMake preset. I’m open to discussion, though.

That’s a fair point about needing to notice the CMake variable anyway. I don’t insist on this approach, was just speculating about other ideas.

[jansvoboda11]

Thanks for weighing in all. I’ll measure both the performance impact of going through vfs::FileSystem (which is what I think Aaron was concerned about) and of the sandbox-enforcing code (which is what I think Vlad is talking about).

[jansvoboda11]

I created a microbenchmark exercising different file system APIs. The benchmark calls stat() on 1000 empty files in the same directory through sys::fs::status(), vfs::RealFileSystem::status() and vfs::RedirectingFileSystem::status(). It pre-warms the operating system’s file system caches and runs each configuration 50 times. I measured the number of retired instructions, which is fairly stable (standard deviation is typically around 0.22%) and the changes in medians compared to using sys::fs without any sandboxing infrastructure are shown below:

Sandboxing status	sys::fs	vfs::RealFileSystem	vfs::RedirectingFileSystem
Sandboxing not implemented	0.00%	0.99%	20.12%
Sandboxing disabled in CMake	-0.01%	0.97%	20.12%
Sandboxing enabled in CMake, disabled at runtime	0.06%	1.12%	20.28%
Sandboxing enabled in CMake, enabled at runtime	N/A	1.12%	20.24%

Measuring wall-clock is quite a bit noisier (standard deviation around 7%), but I think the results are fairly similar:

Sandboxing status	sys::fs	vfs::RealFileSystem	vfs::RedirectingFileSystem
Sandboxing not implemented	0.00%	0.54%	17.54%
Sandboxing disabled in CMake	0.96%	0.94%	18.20%
Sandboxing enabled in CMake, disabled at runtime	-0.09%	0.48%	16.79%
Sandboxing enabled in CMake, enabled at runtime	N/A	2.18%	16.79%

Note that this is IO-only micro-benchmark. The impact on Clang during normal compilation without injecting custom VFSs is going to be much smaller, I’d call it negligible. Also note that vfs::RedirectingFileSystem was configured to actually perform a redirect, so it introduces custom behavior and I put it here just for the fun of it.

Unless people have more specific concerns around performance, I think the most productive way to move forward would be to focus on the enablement story, i.e. whether sandboxing can be implied by asserts being enabled, or only in pre-commit/post-commit checks.

[AaronBallman]

Thank you for gathering that data for us!

Do I understand correctly that we can expect a 1% slowdown for most compilations (because we’d be using RealFileSystem now) and a ~20% slowdown for the sandboxed compilations using RedirectingFileSystem, at least in terms of IO?

[blangmuir]

@AaronBallman

Do I understand correctly that we can expect a 1% slowdown for most compilations (because we’d be using RealFileSystem now) and a ~20% slowdown for the sandboxed compilations using RedirectingFileSystem, at least in terms of IO?

I think this is 1% on just the microbenchmark, not on compilation:

Note that this is IO-only micro-benchmark. The impact on Clang during normal compilation without injecting custom VFSs is going to be much smaller, I’d call it negligible. Also note that vfs::RedirectingFileSystem was configured to actually perform a redirect, so it introduces custom behavior and I put it here just for the fun of it.

[jansvoboda11]

@AaronBallman

Do I understand correctly that we can expect a 1% slowdown for most compilations (because we’d be using RealFileSystem now) and a ~20% slowdown for the sandboxed compilations using RedirectingFileSystem, at least in terms of IO?

We already use RealFileSystem for the vast majority of the IO in the compiler, this RFC doesn’t change much about that reality, it’s just trying to enforce it. But yes, RealFileSystem is likely 1% slower to stat() files than sys::fs in the worst-case micro-benchmark.

However, what is important for this RFC are the differences within the RealFileSystem column. They suggest that the overhead of file system sandboxing is within the noise.

I hope that clears it up.

[AaronBallman]

Thanks! Yeah, I’m not worried about the overhead from RealFileSystem because that is likely to fall out in the noise. But 20% change for the sandboxed code seems like it’s going to be a measurable impact on wall clock time. So I’m wondering if we want the option to be tied to asserts builds or something more explicit as an opt-in. I think people expect asserts builds to be slower than non-asserts builds, so some amount of slowdown is probably reasonable. But it’s hard for me to tell how 20% on IO will impact things in terms of wall clock time, too.

[jansvoboda11]

I think you’re reading the tables wrong, Aaron. My sandboxing patch doesn’t introduce 20% overhead. Let me rephrase the data.

---

Here’s how my sandboxing patch affects the performance for sys::fs::status() in different configurations compared to current state of the repo. (This is mostly irrelevant for Clang, since it already uses vfs::FileSystem APIs for almost all of its IO.)

Configuration	Delta
Baseline (before sandboxing my patch)	0.00%
Sandboxing OFF in CMake	0.00%
Sandboxing ON in CMake, OFF at runtime	0.07%
Sandboxing ON in CMake, ON at runtime	N/A (sandbox violation)

---

Here’s how my sandboxing patch affects the performance of llvm::vfs::RealFileSystem::status(). This is the bit relevant for Clang.

Configuration	Delta
Baseline (before sandboxing my patch)	0.00%
Sandboxing OFF in CMake	-0.01%
Sandboxing ON in CMake, OFF at runtime	0.14%
Sandboxing ON in CMake, ON at runtime	0.15%

Notice that the delta between the current state of the repo and the “Sandboxing ON in CMake” rows is smaller than the standard deviation of 0.22%. Not shown in this table is that in absolute terms, there’s a 1% overhead of using the virtual llvm::vfs::RealFileSystem over llvm::sys::fs APIs. But also note that this RFC doesn’t change that, it merely enforces the existing practice.

---

Here’s how my sandboxing patch affects the performance of llvm::vfs::RedirectingFileSystem::status() compared to the current state of the repo. This is the bit relevant for Clang whenever the user specifies -ivfsoverlay on the Clang command line.

Configuration	Delta
Baseline (before my sandboxing patch)	0.00%
Sandboxing OFF in CMake	-0.01%
Sandboxing ON in CMake, OFF at runtime	0.12%
Sandboxing ON in CMake, ON at runtime	0.12%

Notice again that the delta between the current state and the “Sandboxing ON in CMake” rows is smaller than the standard deviation of 0.22%. Not shown in this table is that in absolute terms, there’s a 20% overhead of using llvm::vfs::RedirectingFileSystem over llvm::sys::fs APIs. But also note that this is already the case today, without my RFC, and that this is the worst-case micro benchmark with warm OS file system caches.

---

Can we agree that 0.15% overhead for asserts Clang (with no overhead for release builds) is an acceptable trade-off for having a consistent view of the FS in the compiler and for enabling sound CAS-based compilation caching?

[smeenai]

@AaronBallman

I think people expect asserts builds to be slower than non-asserts builds, so some amount of slowdown is probably reasonable

I don’t have strong opinions here, just wanted to say that we’ve consistently measured Clang with asserts to be about 20% slower than Clang without asserts. That’s not to say we should have a free-for-all on making it slower since it’s already slow, but the 0.15% overhead seems pretty negligible overall.

[AaronBallman]

Ah, thank you for the further explanation, I think I’m on the same page now!

@jansvoboda11

Can we agree that 0.15% overhead for asserts Clang (with no overhead for release builds) is an acceptable trade-off for having a consistent view of the FS in the compiler and for enabling sound CAS-based compilation caching?

Yeah, I think that’s a reasonable amount of overhead for asserts builds.

@smeenai

I don’t have strong opinions here, just wanted to say that we’ve consistently measured Clang with asserts to be about 20% slower than Clang without asserts. That’s not to say we should have a free-for-all on making it slower since it’s already slow, but the 0.15% overhead seems pretty negligible overall.

Agreed, I was originally thinking this would be more like a 5-10% additional slowdown over the slowdown from asserts, but am glad that’s not the case. .15% additional overhead for an asserts build is reasonable IMO.

[jansvoboda11]

This RFC has been up for two weeks now. It seems to me that people are okay with enabling the file system sandbox for clang -cc1 and clang -cc1as invocations by default in assert builds. I believe the concerns around performance impact have been settled as well. Is it safe to say that we’ve reached a consensus?

[AaronBallman]

We did not discuss this RFC at the last Clang area team meeting, so these are my personal musings, not anything official.

There appears to be no pushback on the idea but there’s also only weak signs of support. Basically, there’s a handful of folks who “liked” the summary of the RFC but no comments about why folks feel this is worth the costs. So I definitely don’t see consensus against the RFC, but it’s still a bit unclear whether there’s significant support for it either.

Per our usual criteria: I don’t think (3), (4), or (8) apply at all. I think (6) and (7) are covered. My biggest questions are around (1), (2), and (5).

For (1), I’m not certain how much that applies to this RFC to begin with, because this isn’t really a user-facing feature. For (2), the question really boils down to whether the fast scanning step can be done in another tool and my gut feeling is maybe, but probably not realistically. For (5), it’s mostly “how disruptive will this be to other maintainers” and my gut feeling is, probably not disruptive because most folks in Clang don’t need to touch the file system directly, they use the source manager or file manager objects and those handle this kind of detail.

Do you have different thoughts?

Does anyone else want to leave comments making a case for why they’d like this RFC accepted (particularly interesting if you’re outside of Apple and need this functionality)?

[jansvoboda11]

@AaronBallman

Do you have different thoughts?

No, I think you summarized how the criteria apply to this RFC pretty well.

[naveen-seth]

I’m also in favor of this RFC.

For newcomers to the project, it would help remove confusion about whether to use the VFS or sys::fs API when getting started. (I remember being uncertain about this when I began).

If the CAS eventually supports caching of explicit C++ named modules, I’d be interested in integrating it into the modules-driver (see this RFC). Compilations using the modules-driver already perform a scan, and some form of modules caching will likely be added in the future. Extending that caching to leverage the CAS seems like a straightforward way to further reduce compilation times for modules-driver users (which may partially address point (1) of the usual criteria).

Regarding performance impact, it also seems negligible to me.

[blangmuir]

@AaronBallman

Does anyone else want to leave comments making a case for why they’d like this RFC accepted (particularly interesting if you’re outside of Apple and need this functionality)?

I work at Apple, but speaking as the person who introduced the VFS to clang, I would also like to see this RFC accepted. It solves a real problem we’ve had since the beginning with maintaining correct usage of the VFS in the compiler. I also think Jan has amply demonstrated the performance impact of the change is minimal for the asserts build where this is to be turned on and zero when not enabled.