Skip to content

Commit

Permalink
Shorten linker output even more when --verbose is not present
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
jyn514 committed Jan 18, 2025
1 parent 8321f00 commit 9c01e98
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 28 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3536,6 +3536,7 @@ dependencies = [
"ar_archive_writer",
"arrayvec",
"bitflags",
"bstr",
"cc",
"either",
"itertools",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub(crate) struct Command {
args: Vec<OsString>,
env: Vec<(OsString, OsString)>,
env_remove: Vec<OsString>,
env_clear: bool,
}

#[derive(Clone)]
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -106,6 +118,9 @@ impl Command {
for k in &self.env_remove {
ret.env_remove(k);
}
if self.env_clear {
ret.env_clear();
}
ret
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 48 additions & 16 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_> {
Expand All @@ -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),
Expand All @@ -384,7 +387,7 @@ impl<G: EmissionGuarantee> 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);
Expand All @@ -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]+$").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");
}

Expand Down
2 changes: 2 additions & 0 deletions src/tools/compiletest/src/runtest/run_make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/tools/run-make-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/path_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
32 changes: 22 additions & 10 deletions tests/run-make/linker-warning/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use run_make_support::{Rustc, rustc};
// //@ revisions: linux not-linux
use run_make_support::{Rustc, diff, regex, rustc};

fn run_rustc() -> Rustc {
let mut rustc = rustc();
Expand All @@ -11,18 +12,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();
}
}
9 changes: 9 additions & 0 deletions tests/run-make/linker-warning/short-error.txt
Original file line number Diff line number Diff line change
@@ -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" "-B<sysroot>/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" "<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

0 comments on commit 9c01e98

Please sign in to comment.