Skip to content

fix(build): reject system LLVM outside compiler sysroot #426

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

Open
Perdixky wants to merge 2 commits into
base: main
Choose a base branch
from
+69 −5
Open

fix(build): reject system LLVM outside compiler sysroot #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b993f6
fix(build): reject system LLVM outside compiler sysroot
Perdixky Apr 17, 2026
e2dc21d
Merge branch 'clice-io:main' into main
Perdixky Apr 18, 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
6 changes: 4 additions & 2 deletions cmake/llvm.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ function(setup_llvm LLVM_VERSION)
# set llvm include and lib path
add_library(llvm-libs INTERFACE IMPORTED)

# add to include directories
target_include_directories(llvm-libs INTERFACE "${LLVM_INSTALL_PATH}/include")
target_include_directories(llvm-libs INTERFACE
"${LLVM_INSTALL_PATH}/include"
"${CMAKE_CURRENT_BINARY_DIR}/include"
)

if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT WIN32)
target_link_directories(llvm-libs INTERFACE "${LLVM_INSTALL_PATH}/lib")
Expand Down
68 changes: 65 additions & 3 deletions scripts/setup-llvm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,55 @@ def parse_version_tuple(text: str) -> tuple[int, ...]:
return tuple(digits)


# When building with pixi or NixOS, the compiler is sandboxed inside its own
# sysroot. System LLVM at /usr is outside that sysroot, so linking against it
# pulls in libraries built for the host's libc/libstdc++ which may be
# ABI-incompatible with the ones inside the sandbox. Detect this and refuse to
# use any LLVM install that falls outside the compiler's sysroot.
def compiler_sysroot() -> Path | None:
"""Return the ``--sysroot`` from clang++'s per-target config, if any."""
compiler = shutil.which("clang++")
if not compiler:
return None

try:
triple = subprocess.check_output([compiler, "-dumpmachine"], text=True).strip()
except (subprocess.CalledProcessError, OSError):
return None

if not triple:
return None

cfg = Path(compiler).with_name(f"{triple}.cfg")
if not cfg.is_file():
return None

try:
for line in cfg.read_text(encoding="utf-8").splitlines():
if not line.startswith("--sysroot="):
continue
sysroot = Path(line.split("=", 1)[1]).resolve()
Comment on lines +206 to +209

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

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

compiler_sysroot() only recognizes lines that start exactly with --sysroot=. Clang config files commonly contain blank lines/comments and may include leading whitespace or use the --sysroot <path> form; in those cases this would fail to detect the active sysroot and could reintroduce the ABI-mismatch scenario this PR is trying to prevent. It would be more robust to strip whitespace, ignore comments, and accept both --sysroot= and --sysroot forms when parsing the cfg.

Suggested change
for line in cfg.read_text(encoding="utf-8").splitlines():
if not line.startswith("--sysroot="):
continue
sysroot = Path(line.split("=", 1)[1]).resolve()
pending_sysroot = False
for line in cfg.read_text(encoding="utf-8").splitlines():
line = line.strip()
if not line or line.startswith("#"):
continue
sysroot_value = None
if pending_sysroot:
sysroot_value = line
pending_sysroot = False
elif line.startswith("--sysroot="):
sysroot_value = line.split("=", 1)[1].strip()
elif line == "--sysroot":
pending_sysroot = True
continue
if not sysroot_value:
continue
sysroot = Path(sysroot_value).resolve()

Copilot uses AI. Check for mistakes.
if sysroot.exists():
return sysroot
except OSError:
return None

return None
Comment on lines +182 to +215

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Does pixi (prefix.dev) generate a <triple>.cfg next to clang++ containing --sysroot, and how does nixpkgs stdenv configure clang's sysroot (wrapper vs config file)?

💡 Result:

No, pixi (prefix.dev) does not generate a <triple>.cfg file next to clang++. Pixi, built on the conda/rattler ecosystem, installs clang packages from conda-forge/prefix.dev channels like clang_osx-64 or clang_linux-64. These packages provide the clang++ binary directly without generating Clang driver config files (.cfg) containing --sysroot next to it. Clang config files are a native Clang/LLVM feature for default flags (e.g., x86_64-unknown-linux-gnu.cfg), loaded automatically by the driver, but no evidence shows pixi/rattler-build or conda clang packages creating them dynamically. Sysroot handling in conda/pixi clang occurs via explicit --sysroot flags (e.g., in cross-compilation via CONDA_BUILD_SYSROOT or compiler activation), environment variables like SDKROOT (macOS), or compiler activation scripts—not .cfg files. nixpkgs stdenv configures clang's sysroot primarily through a wrapper script (cc-wrapper.sh), not config files. stdenv wraps clang++ (often clang-unwrapped) in a bash wrapper that sets environment variables (NIX_CFLAGS_COMPILE, defaultTarget), adds flags like -nostdlibinc (Linux, to block /usr/include), and passes search paths (-L, -B) via nix-support/cc-{cflags,ldflags}. For cross-compilation, it adjusts paths (e.g., /${targetPlatform.config}) and sets machineFlags/defaultTarget. No .cfg files are generated; sysroot is handled via wrapper flags/env vars, preserving --sysroot user overrides.

Citations:

The detection mechanism doesn't work for either pixi or NixOS — the PR won't prevent the stated failure mode in either environment.

The implementation assumes both pixi and NixOS place a <triple>.cfg file next to clang++ with --sysroot directives. However:

  • Pixi: Does not generate .cfg files. It installs clang packages from conda-forge and handles sysroot via explicit --sysroot flags, environment variables, or compiler activation scripts.
  • NixOS: Does not use .cfg files. It wraps clang++ with a shell script (cc-wrapper.sh) that injects sysroot via compiler wrapper flags and environment variables.

In both cases, compiler_sysroot() returns None, llvm_install_matches_compiler() returns True, and system LLVM at /usr is accepted — the exact failure mode the PR claims to prevent.

🧰 Tools
🪛 Ruff (0.15.10)

[error] 194-194: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup-llvm.py` around lines 182 - 215, The current compiler_sysroot()
only looks for a <triple>.cfg file and misses sysroot information injected by
wrappers or flags (e.g., pixi/Conda or NixOS cc-wrapper); update detection to
(1) run the compiler with a diagnostic dry-run to capture injected flags (invoke
clang++ with ["-###","-c","-x","c++","-"] or similar) and parse that output for
--sysroot= or --sysroot, (2) if the compiler path is a script/wrapper (e.g.,
cc-wrapper.sh), read the wrapper text and scan for --sysroot occurrences or
redirected env-vars, and (3) check common environment variables (e.g., SYSROOT,
CONDA_BUILD_SYSROOT, CT_SYSROOT) for a sysroot override; return the first valid
existing Path and ensure llvm_install_matches_compiler() uses this new
compiler_sysroot() result to correctly reject system LLVM outside the compiler
sysroot.

Comment on lines +187 to +215

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Detected compiler may not be the one CMake uses.

shutil.which("clang++") discovers whatever clang++ appears first in PATH, not CMAKE_CXX_COMPILER. If the user configured CMake with a different compiler (e.g., CXX=/opt/llvm20/bin/clang++, a versioned binary like clang++-18, or gcc), the sysroot probed here will not reflect the actual build compiler. Consider passing the compiler path from cmake/llvm.cmake via a new argument (e.g. --cxx-compiler "${CMAKE_CXX_COMPILER}") and preferring that over which.

🧰 Tools
🪛 Ruff (0.15.10)

[error] 194-194: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup-llvm.py` around lines 187 - 215, The compiler_sysroot()
function currently uses shutil.which("clang++") which can differ from the
compiler CMake uses; update the script to accept a new CLI argument (e.g.
--cxx-compiler) and prefer that path when provided, falling back to shutil.which
only if the argument is missing; update any callers (e.g. cmake/llvm.cmake) to
pass ${CMAKE_CXX_COMPILER} into the script; make sure compiler_sysroot() (and
the code that invokes it) uses the provided compiler path to derive the triple
and .cfg file instead of always calling shutil.which("clang++").



def llvm_install_matches_compiler(prefix: Path) -> bool:
"""True when *prefix* is inside the compiler's sysroot (or no sysroot)."""
sysroot = compiler_sysroot()
if sysroot is None:
return True

try:
prefix.resolve().relative_to(sysroot)
return True
except ValueError:
return False
Comment on lines +224 to +228

Copilot AI Apr 17, 2026

Copy link

Choose a reason for hiding this comment

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

llvm_install_matches_compiler() currently requires the LLVM install prefix to be inside the compiler sysroot. On toolchains where the sysroot is a nested directory (common for sandboxed toolchains), the LLVM prefix may legitimately live alongside the sysroot rather than under it, and this check would incorrectly reject a compatible toolchain LLVM and force a bundled download. Consider loosening the match criteria (e.g., treat sysroot as an indicator that host paths like /usr should be rejected, or compare against the toolchain prefix that contains the sysroot rather than the sysroot directory itself).

Copilot uses AI. Check for mistakes.


def system_llvm_ok(required_version: str, build_type: str) -> Path | None:
if build_type.lower().startswith("debug"):
return None
Expand All @@ -194,7 +243,12 @@ def system_llvm_ok(required_version: str, build_type: str) -> Path | None:
found = parse_version_tuple(version)
if not found or found < required:
return None
return Path(prefix)

prefix_path = Path(prefix)
if not llvm_install_matches_compiler(prefix_path):
return None

return prefix_path


def github_api(url: str, token: str | None) -> dict:
Expand Down Expand Up @@ -287,13 +341,21 @@ def main() -> None:
if args.install_path:
candidate = Path(args.install_path)
if candidate.exists():
log(f"Using provided LLVM install at {candidate}")
if llvm_install_matches_compiler(candidate):
log(f"Using provided LLVM install at {candidate}")
install_path = candidate
else:
log(
f"Provided LLVM install at {candidate} is outside the active compiler sysroot; "
"will install a bundled LLVM instead"
)
needs_install = True
else:
log(
f"Provided LLVM install path does not exist; will install to {candidate}"
)
needs_install = True
install_path = candidate
install_path = candidate
Comment on lines 353 to +358

@coderabbitai coderabbitai Bot Apr 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Sysroot check is skipped when --install-path does not exist yet.

When the user passes --install-path=/foo and /foo does not yet exist, the script proceeds to extract the bundled LLVM into that path without checking whether it falls inside the compiler sysroot. If it does not, the freshly-installed LLVM still sits outside the sandbox and you've reintroduced the very ABI mismatch this PR guards against — only now it's silently written to a user-specified location.

Consider applying llvm_install_matches_compiler to the candidate path regardless of whether it exists (and fall back to install_root when it doesn't match).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup-llvm.py` around lines 353 - 358, When handling a user-provided
path that doesn't yet exist (the else branch setting needs_install and
install_path to candidate), still run llvm_install_matches_compiler(candidate)
to ensure the candidate would be inside the compiler sysroot; if
llvm_install_matches_compiler(candidate) returns False, set install_path back to
install_root (and log that the provided path was rejected for sysroot mismatch)
so we fall back to the safe install location instead of silently installing
outside the sysroot. Update the logic around install_path/needs_install (and use
candidate/install_root/llvm_install_matches_compiler) accordingly.

else:
detected = system_llvm_ok(args.version, build_type)
if detected:
Expand Down