Fix OUT_DIR sanitization in cargo_build_script

cargo/private/cargo_build_script_runner/lib.rs

@@ -222,40 +222,46 @@ impl BuildScriptOutput {
 
         CompileAndLinkFlags {
             compile_flags: compile_flags.join("\n"),
-            link_flags: Self::redact_paths(&link_flags.join("\n"), exec_root, out_dir),
-            link_search_paths: Self::redact_paths(
+            link_flags: Self::redact_flags(&link_flags.join("\n"), exec_root, out_dir),
+            link_search_paths: Self::redact_flags(
                 &link_search_paths.join("\n"),
                 exec_root,
                 out_dir,
             ),
         }
     }
 
-    /// Replace the absolute exec-root with `${pwd}` and the relative
-    /// configuration-dependent `out_dir` path (e.g.
-    /// `bazel-out/<config>/bin/.../_bs.out_dir`) with `${out_dir}`.
-    ///
-    /// Both tokens are substituted by `process_wrapper` at action
-    /// execution time. Routing the `out_dir` portion through
-    /// `${out_dir}` lets Bazel's path mapping
-    /// (`--experimental_output_paths=strip`) rewrite it: the consumer
-    /// `Rustc` action passes the directory to `process_wrapper` via
-    /// `--out-dir <File>` from a `File`-typed `Args` entry, so the value
-    /// is the mapped `bazel-out/cfg/bin/...` path under path mapping and
-    /// the un-mapped path otherwise. Without this redaction,
-    /// build-script-emitted env vars (e.g. `cargo::rustc-env=FOO=$OUT_DIR/bar`)
-    /// would carry the un-mapped path through the `_bs.env` file and
-    /// cause the path-mapped Rustc action to look in the wrong location
-    /// at runtime.
+    fn redact_exec_root(value: &str, exec_root: &str) -> String {
+        value.replace(exec_root, "${pwd}")
+    }
+
+    /// Redact for env vars: uses the generic `${out_dir}` token, resolved
+    /// by `process_wrapper`'s `--out-dir` flag. Safe because env files are
+    /// only consumed by the target that directly owns the build script.
     fn redact_paths(value: &str, exec_root: &str, out_dir: &str) -> String {
-        let with_pwd = value.replace(exec_root, "${pwd}");
+        let with_pwd = Self::redact_exec_root(value, exec_root);
         if out_dir.is_empty() {
             with_pwd
         } else {
             with_pwd.replace(out_dir, "${out_dir}")
         }
     }
 
+    /// Redact for flags (link flags, link search paths): uses the full
+    /// `out_dir` relative path as the substitution key so each build
+    /// script gets a unique token. This avoids collisions when flag files
+    /// are consumed transitively by a target whose `--out-dir` points to
+    /// a different build script. The corresponding `--subst` entries are
+    /// added on the Starlark side for every transitive build info.
+    fn redact_flags(value: &str, exec_root: &str, out_dir: &str) -> String {
+        let with_pwd = Self::redact_exec_root(value, exec_root);
+        if out_dir.is_empty() {
+            with_pwd
+        } else {
+            with_pwd.replace(out_dir, &format!("${{{out_dir}}}"))
+        }
+    }
+
     // The process-wrapper treats trailing backslashes as escapes for following newlines.
     // If the env var ends with a backslash (and accordingly doesn't have a following newline),
     // escape it so that it doesn't get turned into a newline by the process-wrapper.
@@ -457,4 +463,33 @@ cargo::rustc-env=BAR=/abs/exec_root/elsewhere/file.rs
             "FOO=${pwd}/${out_dir}/op.rs\nBAR=${pwd}/elsewhere/file.rs"
         );
     }
+
+    /// Link search paths use the full `out_dir` path as the substitution
+    /// key so each build script gets a unique token. This avoids
+    /// collisions when the flag file is consumed transitively by a target
+    /// whose `--out-dir` points to a different build script.
+    #[test]
+    fn out_dir_in_flags_uses_full_path_as_substitution_key() {
+        let buff = Cursor::new(
+            "
+cargo::rustc-link-search=/abs/exec_root/bazel-out/cfg/bin/pkg/_bs.out_dir
+cargo::rustc-link-search=/abs/exec_root/other/path
+",
+        );
+        let reader = BufReader::new(buff);
+        let result = BuildScriptOutput::outputs_from_reader(reader);
+        assert_eq!(
+            BuildScriptOutput::outputs_to_flags(
+                &result,
+                "/abs/exec_root",
+                "bazel-out/cfg/bin/pkg/_bs.out_dir",
+            ),
+            CompileAndLinkFlags {
+                compile_flags: "".to_owned(),
+                link_flags: "".to_owned(),
+                link_search_paths:
+                    "-L${pwd}/${bazel-out/cfg/bin/pkg/_bs.out_dir}\n-L${pwd}/other/path".to_owned(),
+            }
+        );
+    }
 }

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/BUILD.bazel

@@ -0,0 +1,3 @@
+load(":cross_crate_out_dir_files_test_suite.bzl", "cross_crate_out_dir_files_test_suite")
+
+cross_crate_out_dir_files_test_suite(name = "cross_crate_out_dir_files_test_suite")

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello!")
+}

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin_link_build.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("cargo:rustc-link-lib=static=cross_crate_lib");
+}

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl

@@ -0,0 +1,94 @@
+"""Unit tests for transitive OUT_DIR interactions."""
+
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("//cargo:defs.bzl", "cargo_build_script")
+load("//rust:defs.bzl", "rust_binary", "rust_library")
+load(
+    "//test/unit:common.bzl",
+    "assert_action_mnemonic",
+)
+
+def _assert_has_out_dir_subst(env, argv, out_dir_name):
+    """Assert argv has a --subst key=value pair where both sides end with out_dir_name."""
+    for i in range(len(argv) - 1):
+        if argv[i] != "--subst":
+            continue
+        parts = argv[i + 1].split("=", 1)
+        if len(parts) == 2 and parts[0].endswith(out_dir_name) and parts[1].endswith(out_dir_name):
+            return
+    asserts.true(
+        env,
+        False,
+        "Expected --subst key=value where both sides end with '{}'".format(out_dir_name),
+    )
+
+def _transitive_out_dir_subst_test_impl(ctx):
+    """Verify --subst entries exist for each transitive build script out_dir."""
+    env = analysistest.begin(ctx)
+    tut = analysistest.target_under_test(env)
+
+    action = tut.actions[0]
+    assert_action_mnemonic(env, action, "Rustc")
+
+    _assert_has_out_dir_subst(env, action.argv, "lib_build_script.out_dir")
+    _assert_has_out_dir_subst(env, action.argv, "bin_link_build_script.out_dir")
+
+    return analysistest.end(env)
+
+_transitive_out_dir_subst_test = analysistest.make(
+    _transitive_out_dir_subst_test_impl,
+    doc = """\
+Test that `--subst` values were passed as they are used to dereference command line
+files that point to the OUT_DIR of a transitive cargo_build_script dependencies. `--subst`
+is an implementation detail but as build script flags are written to a file, a starlark
+unit test cannot test this. Instead the combination of a successfully built target and the
+tests here provide sufficient regression testing.
+""",
+)
+
+def cross_crate_out_dir_files_test_suite(name):
+    """Test suite for cross-crate build script output directory resolution.
+
+    Args:
+        name: Name of the test suite.
+    """
+    cargo_build_script(
+        name = "lib_build_script",
+        srcs = ["lib_build.rs"],
+        tags = ["manual"],
+    )
+
+    rust_library(
+        name = "lib",
+        srcs = ["lib.rs"],
+        deps = [":lib_build_script"],
+        tags = ["manual"],
+    )
+
+    cargo_build_script(
+        name = "bin_link_build_script",
+        srcs = ["bin_link_build.rs"],
+        tags = ["manual"],
+    )
+
+    rust_binary(
+        name = "bin_link",
+        srcs = ["bin.rs"],
+        deps = [
+            ":bin_link_build_script",
+            ":lib",
+        ],
+        tags = ["manual"],
+    )
+
+    _transitive_out_dir_subst_test(
+        name = "transitive_out_dir_subst_test",
+        target_under_test = ":bin_link",
+    )
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":transitive_out_dir_subst_test",
+        ],
+    )

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib.rs

@@ -0,0 +1 @@
+

cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib_build.rs

@@ -0,0 +1,39 @@
+use std::path::{Path, PathBuf};
+use std::{env, error::Error, fs};
+
+fn is_msvc_linker(ld: &Path) -> bool {
+    ld.file_stem()
+        .and_then(|s| s.to_str())
+        .is_some_and(|name| name.eq_ignore_ascii_case("link"))
+}
+
+fn main() -> Result<(), Box<dyn Error>> {
+    let out = &PathBuf::from(env::var("OUT_DIR")?);
+
+    // Write an empty static archive into OUT_DIR. The downstream
+    // `bin_link` target links against it via `-l`, forcing the linker to
+    // locate the file through the transitive link search path. If the
+    // path resolves incorrectly, linking fails with "library not found".
+    let ld = env::var("LD").unwrap_or_default();
+    if is_msvc_linker(Path::new(&ld)) {
+        // MSVC's link.exe requires a first-linker-member (symbol
+        // directory) after the ar magic.
+        let mut ar = Vec::new();
+        ar.extend_from_slice(b"!<arch>\n");
+        ar.extend_from_slice(b"/               "); // name  (16 bytes)
+        ar.extend_from_slice(b"0           "); // mtime (12 bytes)
+        ar.extend_from_slice(b"0     "); // uid   ( 6 bytes)
+        ar.extend_from_slice(b"0     "); // gid   ( 6 bytes)
+        ar.extend_from_slice(b"100644  "); // mode  ( 8 bytes)
+        ar.extend_from_slice(b"4         "); // size  (10 bytes)
+        ar.extend_from_slice(b"`\n"); // end   ( 2 bytes)
+        ar.extend_from_slice(&[0, 0, 0, 0]); // 0 symbols (big-endian u32)
+        fs::write(out.join("libcross_crate_lib.a"), &ar)?;
+    } else {
+        // GNU ld / gold / lld / Apple ld all accept bare ar magic.
+        fs::write(out.join("libcross_crate_lib.a"), b"!<arch>\n")?;
+    }
+
+    println!("cargo:rustc-link-search={}", out.display());
+    Ok(())
+}

rust/private/rustc.bzl

@@ -1106,6 +1106,29 @@ def construct_arguments(
             expand_directories = False,
         )
 
+    # Build-script flag files (link search paths, link flags) embed the
+    # build script's `out_dir` as a `${<path>}` substitution token, using
+    # the full analysis-time path as the key so each build script gets a
+    # unique token. Add a `--subst` entry for every direct and transitive
+    # build script `out_dir` so `process_wrapper` can resolve each token.
+    # Routing through `File`-typed `Args` entries lets Bazel path mapping
+    # rewrite the value at execution time.
+    for dep_build_info in dep_info.transitive_build_infos.to_list():
+        if dep_build_info.out_dir:
+            process_wrapper_flags.add_all(
+                [dep_build_info.out_dir],
+                before_each = "--subst",
+                format_each = dep_build_info.out_dir.path + "=%s",
+                expand_directories = False,
+            )
+    if out_dir != None:
+        process_wrapper_flags.add_all(
+            [out_dir],
+            before_each = "--subst",
+            format_each = out_dir.path + "=%s",
+            expand_directories = False,
+        )
+
     # Arguments for launching rustc from the process wrapper. When a `File` is
     # provided via `tool_file`, add it directly so Bazel's path mapping can
     # rewrite the location; otherwise fall back to the bare string `tool_path`.