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.

Sign up for GitHub

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

The illumos linker does not support --strip-debug #102418

Merged
merged 1 commit into from
Oct 18, 2022
Merged

The illumos linker does not support --strip-debug #102418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 33 additions & 8 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,16 +1034,36 @@ fn link_natively<'a>(

if sess.target.is_like_osx {
match (strip, crate_type) {
(Strip::Debuginfo, _) => strip_symbols_in_osx(sess, &out_filename, Some("-S")),
(Strip::Debuginfo, _) => {
strip_symbols_with_external_utility(sess, "strip", &out_filename, Some("-S"))
}
// Per the manpage, `-x` is the maximum safe strip level for dynamic libraries. (#93988)
(Strip::Symbols, CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro) => {
strip_symbols_in_osx(sess, &out_filename, Some("-x"))
strip_symbols_with_external_utility(sess, "strip", &out_filename, Some("-x"))
}
(Strip::Symbols, _) => {
strip_symbols_with_external_utility(sess, "strip", &out_filename, None)
}
(Strip::Symbols, _) => strip_symbols_in_osx(sess, &out_filename, None),
(Strip::None, _) => {}
}
}

if sess.target.os == "illumos" {
// Many illumos systems will have both the native 'strip' utility and
// the GNU one. Use the native version explicitly and do not rely on
// what's in the path.
let stripcmd = "/usr/bin/strip";
match strip {
// Always preserve the symbol table (-x).
Strip::Debuginfo => {
strip_symbols_with_external_utility(sess, stripcmd, &out_filename, Some("-x"))
}
// Strip::Symbols is handled via the --strip-all linker option.
Strip::Symbols => {}
Strip::None => {}
}
}

Ok(())
}

Expand All @@ -1055,8 +1075,13 @@ fn strip_value(sess: &Session) -> Strip {
}
}

fn strip_symbols_in_osx<'a>(sess: &'a Session, out_filename: &Path, option: Option<&str>) {
let mut cmd = Command::new("strip");
fn strip_symbols_with_external_utility<'a>(
sess: &'a Session,
util: &str,
out_filename: &Path,
option: Option<&str>,
) {
let mut cmd = Command::new(util);
if let Some(option) = option {
cmd.arg(option);
}
Expand All @@ -1067,14 +1092,14 @@ fn strip_symbols_in_osx<'a>(sess: &'a Session, out_filename: &Path, option: Opti
let mut output = prog.stderr.clone();
output.extend_from_slice(&prog.stdout);
sess.struct_warn(&format!(
"stripping debug info with `strip` failed: {}",
prog.status
"stripping debug info with `{}` failed: {}",
util, prog.status
))
.note(&escape_string(&output))
.emit();
}
}
Err(e) => sess.fatal(&format!("unable to run `strip`: {}", e)),
Err(e) => sess.fatal(&format!("unable to run `{}`: {}", util, e)),
}
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/linker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,13 @@ impl<'a> Linker for GccLinker<'a> {
match strip {
Strip::None => {}
Strip::Debuginfo => {
self.linker_arg("--strip-debug");
// The illumos linker does not support --strip-debug although
// it does support --strip-all as a compatibility alias for -s.
// The --strip-debug case is handled by running an external
// `strip` utility as a separate step after linking.
if self.sess.target.os != "illumos" {
self.linker_arg("--strip-debug");
}
Comment on lines +624 to +626
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the intended solution here would be to invoke an external tool after linking that could do this, similar to the handling for macOS as introduced in #82037.

You probably could just reuse the code path there, for the time being. That way when somebody gets to fixing the way we represent linkers in rustc, illumos will get automatically handled the mostly “right” way alongside with macOS (though you should still keep the comment to the effect that illumos does support --strip-all and could be improved to use that too)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this. I've pushed a new commit which uses the linker's --strip-all but invokes an external strip utility for the --strip-debug case. After talking to people who know more about linkers than I do, they advised that it is best to let the linker do this when possible, hence the hybrid approach.
It may be that the illumos linker can be extended in the future to support --strip-debug but until then this seems like the best approach, and I'd like people to be able to successfully run the tests on illumos again.

}
Strip::Symbols => {
self.linker_arg("--strip-all");
Expand Down