Auto merge of #135707 - jyn514:linker-messages-2, r=bjorn3

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 `<sysroot>` 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.

r? `@bjorn3`

try-job: aarch64-apple

Cargo.lock

@@ -3537,6 +3537,7 @@ dependencies = [
  "ar_archive_writer",
  "arrayvec",
  "bitflags",
+ "bstr",
  "cc",
  "either",
  "itertools",

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"

compiler/rustc_codegen_ssa/src/back/command.rs

@@ -13,6 +13,7 @@ pub(crate) struct Command {
     args: Vec<OsString>,
     env: Vec<(OsString, OsString)>,
     env_remove: Vec<OsString>,
+    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<P: AsRef<OsStr>>(&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
     }
 

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1007,6 +1007,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

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<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
@@ -364,6 +365,8 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
         if self.verbose {
             diag.note(format!("{:?}", self.command));
         } else {
+            self.command.env_clear();
+
             enum ArgGroup {
                 Regular(OsString),
                 Objects(usize),
@@ -398,26 +401,55 @@ impl<G: EmissionGuarantee> 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]+\.rlib$").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"<sysroot>",
+                            ),
+                        )
+                    },
+                    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("<sysroot>").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");
         }
 

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

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.

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]

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",

tests/run-make/linker-warning/rmake.rs

@@ -1,8 +1,19 @@
-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");
+    rustc
+        .arg("main.rs")
+        // NOTE: `link-self-contained` can vary depending on config.toml.
+        // Make sure we use a consistent value.
+        .arg("-Clink-self-contained=-linker")
+        .arg("-Zunstable-options")
+        .output("main")
+        .linker("./fake-linker");
+    if run_make_support::target() == "x86_64-unknown-linux-gnu" {
+        // The value of `rust.lld` is different between CI and locally. Override it explicitly.
+        rustc.arg("-Clinker-flavor=gnu-cc");
+    }
     rustc
 }
 
@@ -25,21 +36,32 @@ fn main() {
     run_rustc().link_arg("run_make_error").run_fail().assert_stderr_contains("note: error: baz");
 
     // 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(r".rcgu.o")
         .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(r".rcgu.o")
         .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();
+    }
+
     // Make sure we show linker warnings even across `-Z no-link`
     rustc()
         .arg("-Zno-link")

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" "<sysroot>/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" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/build-root/test/run-make/linker-warning/rmake_out" "-L" "<sysroot>/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
+