Skip to content

[clang] Prevent sandbox violations in CrossTranslationUnitContext #175097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jansvoboda11 merged 3 commits into from
Jan 12, 2026
Merged

[clang] Prevent sandbox violations in CrossTranslationUnitContext #175097

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d88763
[clang] Use the VFS in `CrossTranslationUnitContext`
jansvoboda11 Jan 8, 2026
522c868
Fix Linux distro detection earlier
jansvoboda11 Jan 9, 2026
1f901d2
Revert CMake change
jansvoboda11 Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion clang/lib/CrossTU/CrossTranslationUnit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/IOSandbox.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/YAMLParser.h"
Expand Down Expand Up @@ -620,6 +621,8 @@ CrossTranslationUnitContext::ASTLoader::loadFromSource(
auto Diags = llvm::makeIntrusiveRefCnt<DiagnosticsEngine>(DiagID, *DiagOpts,
DiagClient);

// This runs the driver which isn't expected to be free of sandbox violations.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
Copy link
Collaborator

@benlangmuir benlangmuir Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change here LGTM, but do you think this deserves a FIXME? I'm not clear whether this is okay or not long term. From a VFS usage perspective calling back to the driver may or may not be okay if you don't pass in a VFS. From a caching perspective it would need special handling.

Copy link
Contributor Author

@jansvoboda11 jansvoboda11 Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. We already have a sandbox disablement due to the driver here:

llvm-project/clang/tools/driver/cc1gen_reproducer_main.cpp

Lines 119 to 120 in 5c3f02c

// The driver is not expected to be free of sandbox violations.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();

and that doesn't have a FIXME. I'm not sure how actionable these FIXMEs would be, since the driver performs lots of FS operations and some don't have an equivalent on the VFS layer (access() comes to mind). Currently there'd be little upside in resolving the FIXMEs (even though it makes sense conceptually).

I don't have a strong opinion, but if we decide to go forward with the FIXMEs, wouldn't it make sense to put them on the driver and have the sandbox disablement somewhere in the clangDriver library?

Copy link
Collaborator

@benlangmuir benlangmuir Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really concerned about the driver itself, that is when it is parsing arguments for and then coordinating the frontend execution, it's the possibility of the driver being called back by the frontend specifically that seems (potentially) problematic if it allows the frontend to indirectly depend on unsandboxed FS access. Probably this is more of a caching issue than a general VFS issue, and it would need special handling to cache correctly anyway, so maybe it's fine to ignore it for now?

return CreateASTUnitFromCommandLine(
CommandLineArgs.begin(), (CommandLineArgs.end()),
CI.getPCHContainerOperations(), DiagOpts, Diags,
Expand Down Expand Up @@ -710,7 +713,7 @@ llvm::Error CrossTranslationUnitContext::ASTLoader::lazyInitInvocationList() {
return llvm::make_error<IndexError>(PreviousParsingResult);

llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> FileContent =
llvm::MemoryBuffer::getFile(InvocationListFilePath);
CI.getVirtualFileSystem().getBufferForFile(InvocationListFilePath);
if (!FileContent) {
PreviousParsingResult = index_error_code::invocation_list_file_not_found;
return llvm::make_error<IndexError>(PreviousParsingResult);
Expand Down