From 67991b49f870a3331a7da0868af1d459b06e90aa Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 18 Jan 2025 16:55:31 -0500 Subject: [PATCH] Shorten linker output even more when `--verbose` is not present - Don't show environment variables. Seeing PATH is almost never useful, and it can be extremely long. - For .rlibs in the sysroot, replace crate hashes with a `"-*"` string. This will expand to the full crate name when pasted into the shell. - Move `.rlib` to outside the glob. - Abbreviate the sysroot path to `` wherever it appears in the arguments. This also adds an example of the linker output as a run-make test. Currently it only runs on x86_64-unknown-linux-gnu, because each platform has its own linker arguments. So that it's stable across machines, pass BUILD_ROOT as an argument through compiletest through to run-make tests. --- Cargo.lock | 1 + compiler/rustc_codegen_ssa/Cargo.toml | 1 + .../rustc_codegen_ssa/src/back/command.rs | 17 ++++- compiler/rustc_codegen_ssa/src/back/link.rs | 1 + compiler/rustc_codegen_ssa/src/errors.rs | 64 ++++++++++++++----- src/tools/compiletest/src/runtest/run_make.rs | 2 + src/tools/run-make-support/src/lib.rs | 2 +- .../run-make-support/src/path_helpers.rs | 6 ++ src/tools/tidy/src/deps.rs | 1 + tests/run-make/linker-warning/rmake.rs | 39 +++++++---- tests/run-make/linker-warning/short-error.txt | 9 +++ 11 files changed, 114 insertions(+), 29 deletions(-) create mode 100644 tests/run-make/linker-warning/short-error.txt diff --git a/Cargo.lock b/Cargo.lock index a08d43a014c06..2917b9ef5d91a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3536,6 +3536,7 @@ dependencies = [ "ar_archive_writer", "arrayvec", "bitflags", + "bstr", "cc", "either", "itertools", diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml index 3254b5d38e7a4..904297b987268 100644 --- a/compiler/rustc_codegen_ssa/Cargo.toml +++ b/compiler/rustc_codegen_ssa/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" ar_archive_writer = "0.4.2" arrayvec = { version = "0.7", default-features = false } bitflags = "2.4.1" +bstr = "1.11.3" # Pinned so `cargo update` bumps don't cause breakage. Please also update the # `cc` in `rustc_llvm` if you update the `cc` here. cc = "=1.2.7" diff --git a/compiler/rustc_codegen_ssa/src/back/command.rs b/compiler/rustc_codegen_ssa/src/back/command.rs index b3c5b86ccf430..63023fdba20fc 100644 --- a/compiler/rustc_codegen_ssa/src/back/command.rs +++ b/compiler/rustc_codegen_ssa/src/back/command.rs @@ -13,6 +13,7 @@ pub(crate) struct Command { args: Vec, env: Vec<(OsString, OsString)>, env_remove: Vec, + env_clear: bool, } #[derive(Clone)] @@ -36,7 +37,13 @@ impl Command { } fn _new(program: Program) -> Command { - Command { program, args: Vec::new(), env: Vec::new(), env_remove: Vec::new() } + Command { + program, + args: Vec::new(), + env: Vec::new(), + env_remove: Vec::new(), + env_clear: false, + } } pub(crate) fn arg>(&mut self, arg: P) -> &mut Command { @@ -79,6 +86,11 @@ impl Command { self } + pub(crate) fn env_clear(&mut self) -> &mut Command { + self.env_clear = true; + self + } + fn _env_remove(&mut self, key: &OsStr) { self.env_remove.push(key.to_owned()); } @@ -106,6 +118,9 @@ impl Command { for k in &self.env_remove { ret.env_remove(k); } + if self.env_clear { + ret.env_clear(); + } ret } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index df35b5e8426f1..ba59bd1266af7 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -991,6 +991,7 @@ fn link_natively( command: cmd, escaped_output, verbose: sess.opts.verbose, + sysroot_dir: sess.sysroot.clone(), }; sess.dcx().emit_err(err); // If MSVC's `link.exe` was expected but the return code diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index c7213bbc801f9..872c6b945ec9e 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -351,6 +351,7 @@ pub(crate) struct LinkingFailed<'a> { pub command: Command, pub escaped_output: String, pub verbose: bool, + pub sysroot_dir: PathBuf, } impl Diagnostic<'_, G> for LinkingFailed<'_> { @@ -364,6 +365,8 @@ impl Diagnostic<'_, G> for LinkingFailed<'_> { if self.verbose { diag.note(format!("{:?}", self.command)); } else { + self.command.env_clear(); + enum ArgGroup { Regular(OsString), Objects(usize), @@ -384,7 +387,7 @@ impl Diagnostic<'_, G> for LinkingFailed<'_> { } else if arg.as_encoded_bytes().ends_with(b".rlib") { let rlib_path = Path::new(&arg); let dir = rlib_path.parent().unwrap(); - let filename = rlib_path.file_name().unwrap().to_owned(); + let filename = rlib_path.file_stem().unwrap().to_owned(); if let Some(ArgGroup::Rlibs(parent, rlibs)) = args.last_mut() { if parent == dir { rlibs.push(filename); @@ -398,26 +401,55 @@ impl Diagnostic<'_, G> for LinkingFailed<'_> { args.push(ArgGroup::Regular(arg)); } } - self.command.args(args.into_iter().map(|arg_group| match arg_group { - ArgGroup::Regular(arg) => arg, - ArgGroup::Objects(n) => OsString::from(format!("<{n} object files omitted>")), - ArgGroup::Rlibs(dir, rlibs) => { - let mut arg = dir.into_os_string(); - arg.push("/{"); - let mut first = true; - for rlib in rlibs { - if !first { - arg.push(","); + let crate_hash = regex::bytes::Regex::new(r"-[0-9a-f]+$").unwrap(); + self.command.args(args.into_iter().map(|arg_group| { + match arg_group { + // SAFETY: we are only matching on ASCII, not any surrogate pairs, so any replacements we do will still be valid. + ArgGroup::Regular(arg) => unsafe { + use bstr::ByteSlice; + OsString::from_encoded_bytes_unchecked( + arg.as_encoded_bytes().replace( + self.sysroot_dir.as_os_str().as_encoded_bytes(), + b"", + ), + ) + }, + ArgGroup::Objects(n) => OsString::from(format!("<{n} object files omitted>")), + ArgGroup::Rlibs(mut dir, rlibs) => { + let is_sysroot_dir = match dir.strip_prefix(&self.sysroot_dir) { + Ok(short) => { + dir = Path::new("").join(short); + true + } + Err(_) => false, + }; + let mut arg = dir.into_os_string(); + arg.push("/{"); + let mut first = true; + for mut rlib in rlibs { + if !first { + arg.push(","); + } + first = false; + if is_sysroot_dir { + // SAFETY: Regex works one byte at a type, and our regex will not match surrogate pairs (because it only matches ascii). + rlib = unsafe { + OsString::from_encoded_bytes_unchecked( + crate_hash + .replace(rlib.as_encoded_bytes(), b"-*") + .into_owned(), + ) + }; + } + arg.push(rlib); } - first = false; - arg.push(rlib); + arg.push("}.rlib"); + arg } - arg.push("}"); - arg } })); - diag.note(format!("{:?}", self.command)); + diag.note(format!("{:?}", self.command).trim_start_matches("env -i").to_owned()); diag.note("some arguments are omitted. use `--verbose` to show all linker arguments"); } diff --git a/src/tools/compiletest/src/runtest/run_make.rs b/src/tools/compiletest/src/runtest/run_make.rs index 8a49d63053521..7ef16e4a9667b 100644 --- a/src/tools/compiletest/src/runtest/run_make.rs +++ b/src/tools/compiletest/src/runtest/run_make.rs @@ -414,6 +414,8 @@ impl TestCx<'_> { // Provide path to checkout root. This is the top-level directory containing // rust-lang/rust checkout. .env("SOURCE_ROOT", &source_root) + // Path to the build directory. This is usually the same as `source_root.join("build").join("host")`. + .env("BUILD_ROOT", &build_root) // Provide path to stage-corresponding rustc. .env("RUSTC", &self.config.rustc_path) // Provide the directory to libraries that are needed to run the *compiler*. This is not diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index ffd4ca22a0086..7316244b3841d 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -90,7 +90,7 @@ pub use artifact_names::{ /// Path-related helpers. pub use path_helpers::{ cwd, filename_contains, filename_not_in_denylist, has_extension, has_prefix, has_suffix, - not_contains, path, shallow_find_files, source_root, + not_contains, path, shallow_find_files, build_root, source_root, }; /// Helpers for scoped test execution where certain properties are attempted to be maintained. diff --git a/src/tools/run-make-support/src/path_helpers.rs b/src/tools/run-make-support/src/path_helpers.rs index 87901793a921e..1c59f2feea4c7 100644 --- a/src/tools/run-make-support/src/path_helpers.rs +++ b/src/tools/run-make-support/src/path_helpers.rs @@ -34,6 +34,12 @@ pub fn source_root() -> PathBuf { env_var("SOURCE_ROOT").into() } +/// Path to the build directory root. +#[must_use] +pub fn build_root() -> PathBuf { + env_var("BUILD_ROOT").into() +} + /// Browse the directory `path` non-recursively and return all files which respect the parameters /// outlined by `closure`. #[track_caller] diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index d4cbc37f6d2b3..e9e2deb240f5f 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -253,6 +253,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "bitflags", "blake3", "block-buffer", + "bstr", "byteorder", // via ruzstd in object in thorin-dwp "cc", "cfg-if", diff --git a/tests/run-make/linker-warning/rmake.rs b/tests/run-make/linker-warning/rmake.rs index 4d21c5ea5690a..43422eac9320a 100644 --- a/tests/run-make/linker-warning/rmake.rs +++ b/tests/run-make/linker-warning/rmake.rs @@ -1,8 +1,14 @@ -use run_make_support::{Rustc, rustc}; +use run_make_support::{Rustc, diff, regex, rustc}; fn run_rustc() -> Rustc { let mut rustc = rustc(); - rustc.arg("main.rs").output("main").linker("./fake-linker"); + // NOTE: `link-self-contained` can vary depending on config.toml. Make sure we use a consistent value. + rustc + .arg("main.rs") + .arg("-Zunstable-options") + .arg("-Clink-self-contained=+linker") + .output("main") + .linker("./fake-linker"); rustc } @@ -11,18 +17,29 @@ fn main() { rustc().arg("fake-linker.rs").output("fake-linker").run(); // Make sure we don't show the linker args unless `--verbose` is passed - run_rustc() - .link_arg("run_make_error") - .verbose() - .run_fail() - .assert_stderr_contains_regex("fake-linker.*run_make_error") + let out = run_rustc().link_arg("run_make_error").verbose().run_fail(); + out.assert_stderr_contains_regex("fake-linker.*run_make_error") .assert_stderr_not_contains("object files omitted") + .assert_stderr_contains("PATH=\"") .assert_stderr_contains_regex(r"lib(/|\\\\)libstd"); - run_rustc() - .link_arg("run_make_error") - .run_fail() - .assert_stderr_contains("fake-linker") + + let out = run_rustc().link_arg("run_make_error").run_fail(); + out.assert_stderr_contains("fake-linker") .assert_stderr_contains("object files omitted") .assert_stderr_contains_regex(r"\{") + .assert_stderr_not_contains("PATH=\"") .assert_stderr_not_contains_regex(r"lib(/|\\\\)libstd"); + + // FIXME: we should have a version of this for mac and windows + if run_make_support::target() == "x86_64-unknown-linux-gnu" { + diff() + .expected_file("short-error.txt") + .actual_text("(linker error)", out.stderr()) + .normalize(r#"/rustc[^/]*/"#, "/rustc/") + .normalize( + regex::escape(run_make_support::build_root().to_str().unwrap()), + "/build-root", + ) + .run(); + } } diff --git a/tests/run-make/linker-warning/short-error.txt b/tests/run-make/linker-warning/short-error.txt new file mode 100644 index 0000000000000..e985570672682 --- /dev/null +++ b/tests/run-make/linker-warning/short-error.txt @@ -0,0 +1,9 @@ +error: linking with `./fake-linker` failed: exit status: 1 + | + = note: "./fake-linker" "-m64" "/tmp/rustc/symbols.o" "<2 object files omitted>" "-Wl,--as-needed" "-Wl,-Bstatic" "/lib/rustlib/x86_64-unknown-linux-gnu/lib/{libstd-*,libpanic_unwind-*,libobject-*,libmemchr-*,libaddr2line-*,libgimli-*,librustc_demangle-*,libstd_detect-*,libhashbrown-*,librustc_std_workspace_alloc-*,libminiz_oxide-*,libadler2-*,libunwind-*,libcfg_if-*,liblibc-*,liballoc-*,librustc_std_workspace_core-*,libcore-*,libcompiler_builtins-*}.rlib" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-B/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld" "-fuse-ld=lld" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/build-root/test/run-make/linker-warning/rmake_out" "-L" "/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs" "run_make_error" + = note: some arguments are omitted. use `--verbose` to show all linker arguments + = note: error: baz + + +error: aborting due to 1 previous error +