Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't show the full linker args unless --verbose is passed #133633

Merged
merged 1 commit into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,12 +992,12 @@ fn link_natively(
let mut output = prog.stderr.clone();
output.extend_from_slice(&prog.stdout);
let escaped_output = escape_linker_output(&output, flavor);
// FIXME: Add UI tests for this error.
let err = errors::LinkingFailed {
linker_path: &linker_path,
exit_status: prog.status,
command: &cmd,
command: cmd,
escaped_output,
verbose: sess.opts.verbose,
};
sess.dcx().emit_err(err);
// If MSVC's `link.exe` was expected but the return code
Expand Down
70 changes: 66 additions & 4 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Errors emitted by codegen_ssa

use std::borrow::Cow;
use std::ffi::OsString;
use std::io::Error;
use std::num::ParseIntError;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -345,21 +346,82 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for ThorinErrorWrapper {
}

pub(crate) struct LinkingFailed<'a> {
pub linker_path: &'a PathBuf,
pub linker_path: &'a Path,
pub exit_status: ExitStatus,
pub command: &'a Command,
pub command: Command,
pub escaped_output: String,
pub verbose: bool,
}

impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
fn into_diag(mut self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
let mut diag = Diag::new(dcx, level, fluent::codegen_ssa_linking_failed);
diag.arg("linker_path", format!("{}", self.linker_path.display()));
diag.arg("exit_status", format!("{}", self.exit_status));

let contains_undefined_ref = self.escaped_output.contains("undefined reference to");

diag.note(format!("{:?}", self.command)).note(self.escaped_output);
if self.verbose {
diag.note(format!("{:?}", self.command));
} else {
enum ArgGroup {
Regular(OsString),
Objects(usize),
Rlibs(PathBuf, Vec<OsString>),
}

// Omit rust object files and fold rlibs in the error by default to make linker errors a
// bit less verbose.
let orig_args = self.command.take_args();
let mut args: Vec<ArgGroup> = vec![];
for arg in orig_args {
if arg.as_encoded_bytes().ends_with(b".rcgu.o") {
if let Some(ArgGroup::Objects(n)) = args.last_mut() {
*n += 1;
} else {
args.push(ArgGroup::Objects(1));
}
} 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();
if let Some(ArgGroup::Rlibs(parent, rlibs)) = args.last_mut() {
if parent == dir {
rlibs.push(filename);
} else {
args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename]));
}
} else {
args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename]));
}
} else {
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(",");
}
first = false;
arg.push(rlib);
}
arg.push("}");
arg
}
}));

diag.note(format!("{:?}", self.command));
diag.note("some arguments are omitted. use `--verbose` to show all linker arguments");
}

diag.note(self.escaped_output);

// Trying to match an error from OS linkers
// which by now we have no way to translate.
Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/external_deps/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,12 @@ impl Rustc {
self
}

/// Pass the `--verbose` flag.
pub fn verbose(&mut self) -> &mut Self {
self.cmd.arg("--verbose");
self
}

/// `EXTRARSCXXFLAGS`
pub fn extra_rs_cxx_flags(&mut self) -> &mut Self {
// Adapted from tools.mk (trimmed):
Expand Down
6 changes: 4 additions & 2 deletions tests/run-make/link-args-order/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@ fn main() {
.link_args("b c")
.link_args("d e")
.link_arg("f")
.arg("--print=link-args")
.run_fail()
.assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
.assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#);
rustc()
.input("empty.rs")
.linker_flavor(linker)
.arg("-Zpre-link-arg=a")
.arg("-Zpre-link-args=b c")
.arg("-Zpre-link-args=d e")
.arg("-Zpre-link-arg=f")
.arg("--print=link-args")
.run_fail()
.assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
.assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#);
}
12 changes: 6 additions & 6 deletions tests/run-make/link-dedup/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ fn main() {
rustc().input("depb.rs").run();
rustc().input("depc.rs").run();

let output = rustc().input("empty.rs").cfg("bar").run_fail();
output.assert_stderr_contains(needle_from_libs(&["testa", "testb", "testa"]));
let output = rustc().input("empty.rs").cfg("bar").arg("--print=link-args").run_fail();
output.assert_stdout_contains(needle_from_libs(&["testa", "testb", "testa"]));

let output = rustc().input("empty.rs").run_fail();
output.assert_stderr_contains(needle_from_libs(&["testa"]));
output.assert_stderr_not_contains(needle_from_libs(&["testb"]));
output.assert_stderr_not_contains(needle_from_libs(&["testa", "testa", "testa"]));
let output = rustc().input("empty.rs").arg("--print=link-args").run_fail();
output.assert_stdout_contains(needle_from_libs(&["testa"]));
output.assert_stdout_not_contains(needle_from_libs(&["testb"]));
output.assert_stdout_not_contains(needle_from_libs(&["testa", "testa", "testa"]));
// Adjacent identical native libraries are no longer deduplicated if
// they come from different crates (https://github.com/rust-lang/rust/pull/103311)
// so the following will fail:
Expand Down
13 changes: 13 additions & 0 deletions tests/run-make/linker-warning/fake-linker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn main() {
for arg in std::env::args() {
match &*arg {
"run_make_info" => println!("foo"),
"run_make_warn" => eprintln!("warning: bar"),
"run_make_error" => {
eprintln!("error: baz");
std::process::exit(1);
}
_ => (),
}
}
}
1 change: 1 addition & 0 deletions tests/run-make/linker-warning/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
28 changes: 28 additions & 0 deletions tests/run-make/linker-warning/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
use run_make_support::{Rustc, rustc};

fn run_rustc() -> Rustc {
let mut rustc = rustc();
rustc.arg("main.rs").output("main").linker("./fake-linker");
rustc
}

fn main() {
// first, compile our linker
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")
.assert_stderr_not_contains("object files omitted")
.assert_stderr_contains_regex(r"lib(/|\\\\)libstd");
run_rustc()
.link_arg("run_make_error")
.run_fail()
.assert_stderr_contains("fake-linker")
.assert_stderr_contains("object files omitted")
.assert_stderr_contains_regex(r"\{")
.assert_stderr_not_contains_regex(r"lib(/|\\\\)libstd");
}
Loading