Make --codegen=linker and --sysroot path-mapping-aware for a cc_toolchain-provided linker - #4253
Draft
yakkala-pooja wants to merge 1 commit into
Draft
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
yakkala-pooja
force-pushed
the
fix-4250-path-map-linker-args
branch
3 times, most recently
from
September 8, 2026 04:16
da3905a to
152f6ab
Compare
…hain-provided linker cc_common.get_tool_for_action() returns the C++ toolchain's linker as a plain string, and cc_common.get_memory_inefficient_command_line() returns its link args (including a --sysroot= value) as plain strings too. Passing either straight into Args means Bazel's path mapping (--experimental_output_paths=strip) has nothing it can rewrite, since only a File threaded through Args is eligible -- so a hermetic C++ toolchain whose linker or sysroot is a generated Bazel artifact ends up with a stale, un-rewritten configuration segment in the rustc command line, and linking fails when the Rustc action runs against the stripped bazel-out/cfg layout. get_linker_and_args now recovers the backing File for the linker (searching cc_toolchain.all_files for the string cc_common.get_tool_for_action() returned, including the case where the tool lives inside a directory/tree artifact) and threads it through rustc_flags.add/add_all instead of the raw string, mirroring the map_each/format_each pattern this file already uses for the Rust toolchain's own sysroot. The Rust-toolchain-provided linker (toolchain.linker) already had a File on hand for free, so that branch is fixed the same way. link_args gets the same treatment for a --sysroot= entry, via a small wrapper that leaves every other entry, and any entry that fails to resolve, untouched. The resolution is skipped up front for any path that doesn't start with bazel-out/: a system-absolute path (the common, non-hermetic case), an external-repository path, and a plain source-tree path are all already configuration-independent, so path mapping has nothing to rewrite for them and the scan would be pure overhead. construct_arguments now also tracks any File it resolved this way (the linker, and any --sysroot= link-arg) on the returned args struct as extra_action_inputs, and rustc_compile_action merges that into the Rustc/RustcMetadata actions' inputs. Passing a File through Args puts it on the command line but does not by itself register it as an action input; collect_inputs' own cc_toolchain.linker_files()-based mechanism did not reliably do this for a File resolved this way (observed against a real hermetic toolchain in CI -- see below), so it is tracked explicitly instead of relying on that path. Added test/unit/path_mapped_linker: a minimal cc_toolchain whose linker and sysroot are genrule outputs (so their real location is a config-dependent bazel-out/... path, unlike a normal system toolchain), registered via --extra_toolchains with linker_preference forced to "cc". Four things only surfaced against real CI, not local testing, and are all fixed: - analysis_test_transition refuses to set --experimental_* options, so the test cannot force --experimental_output_paths=strip on itself. The assertions check only that --codegen=linker= and the --sysroot= link-arg name the right file, not the mapped bazel-out/cfg/... prefix specifically; the repo's own "Path Mapping Linux/RBE/MacOS" CI jobs already run the whole suite, this test included, with the flag set, which is where that stronger check actually happens. Verified manually both ways: run normally the test passes trivially; run with --experimental_output_paths=strip on the command line, temporarily reverting just the resolution step in rustc.bzl makes it fail, showing the real, un-rewritten configuration segment in place of bazel-out/cfg/ for exactly the flag that was reverted -- confirmed independently for the linker and the sysroot. - The fake toolchain, registered via --extra_toolchains with no compatibility constraints, was a candidate for every C++ toolchain resolution in the build. Under bazel coverage specifically (which forces a fresh, uncached build of exec-configuration tools), it was being selected to "link" util/process_wrapper -- a Rust binary with no relation to this test -- and since the fake linker was a no-op script, that build failed with "output ... was not created" on both Linux/RBE and macOS CI. Fixed by giving fake_cc_toolchain a target_compatible_with constraint satisfied only by a dedicated fake_platform (extending the real host platform via `parents`), and transitioning --platforms to it alongside --extra_toolchains: the fake toolchain now only ever matches this test's own target configuration. - analysistest never executes the target-under-test's own actions (it only inspects the analysis-time action graph), but bazel coverage does -- so once the fake linker was actually the one selected for this test's own rust_binary (correctly, now that the previous leak is fixed), its being a pure no-op became a real problem: Bazel requires a declared output to actually exist, and the no-op script produced none. The fake linker (test/unit/path_mapped_linker/fake_gcc_template.sh, copied into place by a genrule so its exec path stays under bazel-out/) now parses `-o <path>` and `/OUT:<path>` and touches whichever one it's given -- enough to satisfy the output check without attempting a real link, since this test only inspects the command line rustc was given, never the resulting binary. - Fixing the toolchain leak above then surfaced the exact bug this PR fixes, one level up: with the fake toolchain now correctly scoped to just this test's own rust_binary, its Rustc action failed with "linker `bazel-out/.../fake_gcc` not found" under bazel coverage on the repo's minimum-supported Bazel version -- the file was on the command line but not materialized in the sandbox, i.e. exactly the extra_action_inputs gap construct_arguments/rustc_compile_action now close. Reproduced the toolchain-leak scenario locally with `bazel coverage` over this test plus util/process_wrapper together; confirmed process_wrapper falls through to the real toolchain (fails only on this machine's unrelated, pre-existing missing MSVC install, not the fake linker) once scoped. The fake linker script's argument parsing was verified directly in isolation (both conventions); a full end-to-end Rust build could not be exercised on this machine at all, MSVC being absent throughout. Fixes bazelbuild#4250 Assisted-by: Claude (Anthropic)
yakkala-pooja
force-pushed
the
fix-4250-path-map-linker-args
branch
from
September 8, 2026 04:29
152f6ab to
8825577
Compare
yakkala-pooja
marked this pull request as draft
September 8, 2026 04:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4250
cc_common.get_tool_for_action() returns the C++ toolchain's linker as a
plain string, and get_memory_inefficient_command_line() returns its link
args (including --sysroot=) as plain strings too. Bazel's path mapping
(--experimental_output_paths=strip) can only rewrite a path that reaches
Args as a File, so a hermetic C++ toolchain whose linker or sysroot is a
generated Bazel artifact ends up with a stale, un-rewritten configuration
segment on the rustc command line, and linking fails when the Rustc action
runs against the stripped bazel-out/cfg layout.
Change
(searching cc_toolchain.all_files, including the case where the tool
lives inside a directory/tree artifact) and threads it through Args
instead of the raw string, mirroring the map_each/format_each pattern
already used elsewhere in this file for the Rust toolchain's own sysroot.
on hand for free, so that branch is fixed the same way.
wrapper that leaves every other entry (and any entry that fails to
resolve) untouched.
bazel-out/, since only a generated-artifact path is ever
configuration-dependent — this keeps the added scan a no-op for the
common non-hermetic-toolchain case.
Testing
Added test/unit/path_mapped_linker: a minimal cc_toolchain whose linker
and sysroot are genrule outputs (so their real location is genuinely
config-dependent, unlike a normal system toolchain), registered via
--extra_toolchains with linker_preference forced to "cc".
Two things only surfaced against real CI, not local testing:
analysis_test_transition refuses to set --experimental_* options, so
the test can't force --experimental_output_paths=strip on itself. The
assertions check that --codegen=linker= and the --sysroot= link-arg
name the right file, not the mapped prefix specifically — this repo's
own "Path Mapping Linux/RBE/MacOS" CI jobs already run the whole suite
with that flag set, which is where the stronger check happens.
Verified manually both ways: normal run passes trivially; with
--experimental_output_paths=strip on the command line, temporarily
reverting just the resolution step makes it fail, showing the real
unmapped configuration segment instead of bazel-out/cfg/, confirmed
independently for the linker and the sysroot.
The fake toolchain, registered via --extra_toolchains with no
compatibility constraints, was a candidate for every C++ toolchain
resolution in the build. Under bazel coverage specifically, it was
getting selected to "link" util/process_wrapper — an unrelated Rust
tool — and since the fake linker is a no-op script, that build failed
with "output ... was not created" on both Linux/RBE and macOS CI.
Fixed by giving fake_cc_toolchain a target_compatible_with constraint
satisfied only by a dedicated fake_platform, and transitioning
--platforms to it alongside --extra_toolchains, so the fake toolchain
only ever matches this test's own configuration. Reproduced locally
with bazel coverage over this test plus util/process_wrapper together
and confirmed process_wrapper now falls through to the real toolchain.
Assisted-by: Claude (Anthropic)