diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index a51c21e4b2..81ad53200d 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -883,6 +883,33 @@ def _will_emit_object_file(emit): def _remove_codegen_units(flag): return None if flag.startswith("-Ccodegen-units") else flag +def _should_add_oso_prefix(feature_configuration, ld_is_direct_driver, toolchain): + """Whether to add -oso_prefix to strip absolute paths from N_OSO entries. + + On macOS, ld64 embeds absolute paths in N_OSO stab entries which breaks + build reproducibility. The -oso_prefix flag strips a prefix from these + entries. This function gates the flag on linker support. + + Args: + feature_configuration (FeatureConfiguration): Feature configuration to be queried. + ld_is_direct_driver (bool): Whether the linker is invoked directly (e.g. rust-lld). + toolchain (rust_toolchain): The current Rust toolchain. + + Returns: + bool: True if -oso_prefix should be added. + """ + if not toolchain.target_os.startswith(("mac", "darwin", "ios")): + return False + + if not ld_is_direct_driver: + return feature_configuration and cc_common.is_enabled( + feature_configuration = feature_configuration, + feature_name = "oso_prefix_is_pwd", + ) + + # lld-macho has supported -oso_prefix since LLVM 14 (2021). + return True + def construct_arguments( *, ctx, @@ -939,7 +966,9 @@ def construct_arguments( include_link_flags (bool, optional): Whether to include flags like `-l` that instruct the linker to search for a library. stamp (bool, optional): Whether or not workspace status stamping is enabled. For more details see https://docs.bazel.build/versions/main/user-manual.html#flag--stamp - remap_path_prefix (str, optional): A value used to remap `${pwd}` to. If set to None, no prefix will be set. + remap_path_prefix (str, optional): A value used to remap `${pwd}`, `${exec_root}`, and `${output_base}` to. + If set to None, no remapping will be applied. On macOS, also adds `-oso_prefix` to strip absolute paths + from N_OSO linker entries. use_json_output (bool): Have rustc emit json and process_wrapper parse json messages to output rendered output. build_metadata (bool): Generate CLI arguments for building *only* .rmeta files. This requires use_json_output. force_depend_on_objects (bool): Force using `.rlib` object files instead of metadata (`.rmeta`) files even if they are available. @@ -989,6 +1018,8 @@ def construct_arguments( # Since we cannot get the `exec_root` from starlark, we cheat a little and # use `${pwd}` which resolves the `exec_root` at action execution time. process_wrapper_flags.add("--subst", "pwd=${pwd}") + process_wrapper_flags.add("--subst", "exec_root=${exec_root}") + process_wrapper_flags.add("--subst", "output_base=${output_base}") # If stamping is enabled, enable the functionality in the process wrapper if stamp: @@ -1076,6 +1107,8 @@ def construct_arguments( # For determinism to help with build distribution and such if remap_path_prefix != None: rustc_flags.add("--remap-path-prefix=${{pwd}}={}".format(remap_path_prefix)) + rustc_flags.add("--remap-path-prefix=${{exec_root}}={}".format(remap_path_prefix)) + rustc_flags.add("--remap-path-prefix=${{output_base}}={}".format(remap_path_prefix)) emit_without_paths = [] for kind in emit: @@ -1140,6 +1173,17 @@ def construct_arguments( # Additional context: https://github.com/rust-lang/rust/pull/36574 rustc_flags.add_all(link_args, format_each = "--codegen=link-arg=%s") + if remap_path_prefix != None and _should_add_oso_prefix( + feature_configuration, + ld_is_direct_driver, + toolchain, + ): + if ld_is_direct_driver: + rustc_flags.add("--codegen=link-arg=-oso_prefix") + rustc_flags.add("${pwd}/", format = "--codegen=link-arg=%s") + else: + rustc_flags.add("--codegen=link-arg=-Wl,-oso_prefix,${pwd}/") + _add_native_link_flags( rustc_flags, dep_info, diff --git a/test/unit/remap_path_prefix/BUILD.bazel b/test/unit/remap_path_prefix/BUILD.bazel index 03739da675..fa77ac8256 100644 --- a/test/unit/remap_path_prefix/BUILD.bazel +++ b/test/unit/remap_path_prefix/BUILD.bazel @@ -1,5 +1,6 @@ load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") load(":debug_transition.bzl", "dbg_rust_binary") +load(":remap_path_prefix_test.bzl", "remap_path_prefix_test_suite") rust_library( name = "dep", @@ -35,3 +36,7 @@ rust_test( }, deps = ["//rust/runfiles"], ) + +remap_path_prefix_test_suite( + name = "remap_path_prefix_test_suite", +) diff --git a/test/unit/remap_path_prefix/integration_test.rs b/test/unit/remap_path_prefix/integration_test.rs index dcac30d61b..8a4a56e991 100644 --- a/test/unit/remap_path_prefix/integration_test.rs +++ b/test/unit/remap_path_prefix/integration_test.rs @@ -19,8 +19,8 @@ fn test_backtrace() { let mut check_next = false; for line in stderr.split('\n') { if check_next { - if !line.contains("./test/unit/remap_path_prefix/panic_bin.rs:6:5") { - panic!("Expected line to contain ./test/unit/remap_path_prefix/panic_bin.rs:6:5 but was {}", line); + if !line.contains("test/unit/remap_path_prefix/panic_bin.rs:6:5") { + panic!("Expected line to contain test/unit/remap_path_prefix/panic_bin.rs:6:5 but was {}", line); } return; } diff --git a/test/unit/remap_path_prefix/remap_path_prefix_test.bzl b/test/unit/remap_path_prefix/remap_path_prefix_test.bzl new file mode 100644 index 0000000000..431852cf51 --- /dev/null +++ b/test/unit/remap_path_prefix/remap_path_prefix_test.bzl @@ -0,0 +1,110 @@ +"""Analysis tests verifying remap_path_prefix flags.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("//rust:defs.bzl", "rust_binary", "rust_library") +load( + "//test/unit:common.bzl", + "assert_action_mnemonic", + "assert_argv_contains", + "assert_list_contains_adjacent_elements", +) + +def _remap_path_prefix_test_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + + action = target.actions[0] + assert_action_mnemonic(env, action, "Rustc") + + assert_argv_contains(env, action, "--remap-path-prefix=${pwd}=.") + assert_argv_contains(env, action, "--remap-path-prefix=${exec_root}=.") + assert_argv_contains(env, action, "--remap-path-prefix=${output_base}=.") + + return analysistest.end(env) + +_remap_path_prefix_test = analysistest.make(_remap_path_prefix_test_impl) + +def _subst_flags_test_impl(ctx): + """Verify that process wrapper --subst flags are present.""" + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + + action = target.actions[0] + assert_action_mnemonic(env, action, "Rustc") + + assert_list_contains_adjacent_elements(env, action.argv, ["--subst", "pwd=${pwd}"]) + assert_list_contains_adjacent_elements(env, action.argv, ["--subst", "exec_root=${exec_root}"]) + assert_list_contains_adjacent_elements(env, action.argv, ["--subst", "output_base=${output_base}"]) + + return analysistest.end(env) + +_subst_flags_test = analysistest.make(_subst_flags_test_impl) + +def remap_path_prefix_test_suite(name): + """Entry-point macro called from the BUILD file. + + Args: + name (str): The name of the test suite. + """ + write_file( + name = "remap_lib_src", + out = "remap_lib.rs", + content = [ + "pub fn hello() {}", + "", + ], + ) + + rust_library( + name = "remap_lib", + srcs = [":remap_lib.rs"], + edition = "2021", + ) + + write_file( + name = "remap_bin_src", + out = "remap_bin.rs", + content = [ + "fn main() {}", + "", + ], + ) + + rust_binary( + name = "remap_bin", + srcs = [":remap_bin.rs"], + edition = "2021", + ) + + _remap_path_prefix_test( + name = "remap_path_prefix_lib_test", + target_under_test = ":remap_lib", + ) + + _remap_path_prefix_test( + name = "remap_path_prefix_bin_test", + target_under_test = ":remap_bin", + ) + + _subst_flags_test( + name = "subst_flags_lib_test", + target_under_test = ":remap_lib", + ) + + _subst_flags_test( + name = "subst_flags_bin_test", + target_under_test = ":remap_bin", + ) + + tests = [ + ":remap_path_prefix_lib_test", + ":remap_path_prefix_bin_test", + ":subst_flags_lib_test", + ":subst_flags_bin_test", + ] + + native.test_suite( + name = name, + tests = tests, + ) diff --git a/util/collect_coverage/BUILD.bazel b/util/collect_coverage/BUILD.bazel index c3aa8e1d13..24ed5be45d 100644 --- a/util/collect_coverage/BUILD.bazel +++ b/util/collect_coverage/BUILD.bazel @@ -4,5 +4,14 @@ rust_binary( name = "collect_coverage", srcs = ["collect_coverage.rs"], edition = "2018", + # To ensure this tool is produced deterministically, debug info + # is stripped out. On macOS, full symbol stripping is needed to + # also remove N_OSO stab entries whose timestamps and sandbox + # paths vary between builds. + rustc_flags = select({ + "@platforms//os:linux": ["-Cstrip=debuginfo"], + "@platforms//os:macos": ["-Cstrip=symbols"], + "//conditions:default": [], + }), visibility = ["//visibility:public"], ) diff --git a/util/collect_coverage/collect_coverage.rs b/util/collect_coverage/collect_coverage.rs index 7aba667885..b7e154ecf6 100644 --- a/util/collect_coverage/collect_coverage.rs +++ b/util/collect_coverage/collect_coverage.rs @@ -147,28 +147,37 @@ fn main() { .arg("-format=lcov") .arg("-instr-profile") .arg(&profdata_file) - .arg("-ignore-filename-regex='.*external/.+'") - .arg("-ignore-filename-regex='/tmp/.+'") - .arg(format!("-path-equivalence=.,'{}'", execroot.display())) + .arg("-ignore-filename-regex=.*external/.+") + .arg("-ignore-filename-regex=/tmp/.+") + .arg(format!("-path-equivalence=.,{}", execroot.display())) .arg(test_binary) .stdout(process::Stdio::piped()); debug_log!("Spawning {:#?}", llvm_cov_cmd); - let child = llvm_cov_cmd - .spawn() + let output = llvm_cov_cmd + .stderr(process::Stdio::piped()) + .output() .expect("Failed to spawn llvm-cov process"); - let output = child.wait_with_output().expect("llvm-cov process failed"); + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + eprintln!( + "llvm-cov export failed with status {}: {}", + output.status, stderr + ); + process::exit(output.status.code().unwrap_or(1)); + } - // Parse the child process's stdout to a string now that it's complete. debug_log!("Parsing llvm-cov output"); let report_str = std::str::from_utf8(&output.stdout).expect("Failed to parse llvm-cov output"); debug_log!("Writing output to {}", coverage_output_file.display()); + let execroot_prefix = format!("{}/", execroot.display()); fs::write( coverage_output_file, report_str .replace("#/proc/self/cwd/", "") + .replace(&execroot_prefix, "") .replace(&execroot.display().to_string(), ""), ) .unwrap(); diff --git a/util/process_wrapper/BUILD.bazel b/util/process_wrapper/BUILD.bazel index af56264e4c..172d94bef3 100644 --- a/util/process_wrapper/BUILD.bazel +++ b/util/process_wrapper/BUILD.bazel @@ -1,32 +1,7 @@ -load("@bazel_skylib//lib:selects.bzl", "selects") - # buildifier: disable=bzl-visibility load("//rust/private:rust.bzl", "rust_binary_without_process_wrapper", "rust_test_without_process_wrapper_test") load("//util/process_wrapper/private:bootstrap_process_wrapper.bzl", "bootstrap_process_wrapper") -config_setting( - name = "compilation_mode_opt", - values = {"compilation_mode": "opt"}, -) - -selects.config_setting_group( - name = "opt_linux", - match_all = [ - ":compilation_mode_opt", - "@platforms//os:linux", - ], - visibility = ["@rules_rust_tinyjson//:__pkg__"], -) - -selects.config_setting_group( - name = "opt_macos", - match_all = [ - ":compilation_mode_opt", - "@platforms//os:macos", - ], - visibility = ["@rules_rust_tinyjson//:__pkg__"], -) - rust_binary_without_process_wrapper( name = "process_wrapper", srcs = glob(["*.rs"]), @@ -37,10 +12,12 @@ rust_binary_without_process_wrapper( edition = "2018", # To ensure the process wrapper is produced deterministically # debug info, which is known to sometimes have host specific - # paths embedded in this section, is stripped out. + # paths embedded in this section, is stripped out. On macOS, + # full symbol stripping is needed to also remove N_OSO stab + # entries whose timestamps vary between builds. rustc_flags = select({ - ":opt_linux": ["-Cstrip=debuginfo"], - ":opt_macos": ["-Cstrip=debuginfo"], + "@platforms//os:linux": ["-Cstrip=debuginfo"], + "@platforms//os:macos": ["-Cstrip=symbols"], "//conditions:default": [], }), visibility = ["//visibility:public"], diff --git a/util/process_wrapper/BUILD.tinyjson.bazel b/util/process_wrapper/BUILD.tinyjson.bazel index 7a7bcb3367..27b12c1890 100644 --- a/util/process_wrapper/BUILD.tinyjson.bazel +++ b/util/process_wrapper/BUILD.tinyjson.bazel @@ -10,8 +10,8 @@ rust_library_without_process_wrapper( # debug info, which is known to sometimes have host specific # paths embedded in this section, is stripped out. rustc_flags = select({ - "@rules_rust//util/process_wrapper:opt_linux": ["-Cstrip=debuginfo"], - "@rules_rust//util/process_wrapper:opt_macos": ["-Cstrip=debuginfo"], + "@platforms//os:linux": ["-Cstrip=debuginfo"], + "@platforms//os:macos": ["-Cstrip=debuginfo"], "//conditions:default": [], }), visibility = ["@rules_rust//util/process_wrapper:__pkg__"], diff --git a/util/process_wrapper/options.rs b/util/process_wrapper/options.rs index 2f252cadc7..2e139f7ccc 100644 --- a/util/process_wrapper/options.rs +++ b/util/process_wrapper/options.rs @@ -137,6 +137,34 @@ pub(crate) fn options() -> Result { .to_str() .ok_or_else(|| OptionError::Generic("current directory not utf-8".to_owned()))? .to_owned(); + let output_base = { + let external = std::path::Path::new(¤t_dir).join("external"); + match std::fs::canonicalize(external) { + Ok(canonical) => canonical + .parent() + .and_then(|p| p.to_str()) + .unwrap_or(¤t_dir) + .to_owned(), + Err(_) => match std::fs::canonicalize(¤t_dir) { + Ok(canonical) => canonical + .parent() + .and_then(|p| p.parent()) + .and_then(|p| p.to_str()) + .unwrap_or(¤t_dir) + .to_owned(), + Err(_) => current_dir.clone(), + }, + } + }; + + let exec_root = { + let workspace_name = std::path::Path::new(¤t_dir) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("_main"); + format!("{}/execroot/{}", output_base, workspace_name) + }; + let subst_mappings = subst_mapping_raw .unwrap_or_default() .into_iter() @@ -146,6 +174,10 @@ pub(crate) fn options() -> Result { })?; let v = if val == "${pwd}" { current_dir.as_str() + } else if val == "${output_base}" { + output_base.as_str() + } else if val == "${exec_root}" { + exec_root.as_str() } else { val } diff --git a/util/process_wrapper/private/process_wrapper.bat b/util/process_wrapper/private/process_wrapper.bat index 36fff8699a..70e9067fb7 100755 --- a/util/process_wrapper/private/process_wrapper.bat +++ b/util/process_wrapper/private/process_wrapper.bat @@ -6,6 +6,15 @@ SET command=%* :: Resolve the `${pwd}` placeholders SET command=!command:${pwd}=%CD%! +:: Resolve the `${output_base}` and `${exec_root}` placeholders. +:: The external directory is a junction/symlink to output_base\external. +:: This mirrors the logic in options.rs used by the real process wrapper. +FOR /F "delims=" %%i IN ('cd external\.. ^& cd') DO SET output_base=%%i +FOR %%i IN ("%CD%") DO SET workspace_name=%%~nxi +SET exec_root=!output_base!\execroot\!workspace_name! +SET command=!command:${output_base}=%output_base%! +SET command=!command:${exec_root}=%exec_root%! + :: Strip out the leading `--` argument. SET command=!command:~3! diff --git a/util/process_wrapper/private/process_wrapper.sh b/util/process_wrapper/private/process_wrapper.sh index 27fb44bbd8..a5089eddfe 100755 --- a/util/process_wrapper/private/process_wrapper.sh +++ b/util/process_wrapper/private/process_wrapper.sh @@ -5,15 +5,41 @@ set -eu # Skip the first argument which is expected to be `--` shift +# Derive output_base and exec_root so we can expand their placeholders +# in rustc flags (e.g. --remap-path-prefix, -oso_prefix). This mirrors +# the logic in options.rs used by the real process wrapper. +phys_pwd=$(cd -P . && pwd) +if [ -d "external" ]; then + output_base=$(cd -P external/.. && pwd) +else + output_base="${phys_pwd%/*}" + output_base="${output_base%/*}" +fi +workspace_name="${phys_pwd##*/}" +exec_root="${output_base}/execroot/${workspace_name}" + for arg in "$@"; do case "$arg" in *'${pwd}'*) - # Split on '${pwd}' and rejoin with the actual PWD value prefix="${arg%%\$\{pwd\}*}" suffix="${arg#*\$\{pwd\}}" arg="${prefix}${PWD}${suffix}" ;; esac + case "$arg" in + *'${output_base}'*) + prefix="${arg%%\$\{output_base\}*}" + suffix="${arg#*\$\{output_base\}}" + arg="${prefix}${output_base}${suffix}" + ;; + esac + case "$arg" in + *'${exec_root}'*) + prefix="${arg%%\$\{exec_root\}*}" + suffix="${arg#*\$\{exec_root\}}" + arg="${prefix}${exec_root}${suffix}" + ;; + esac set -- "$@" "$arg" shift done