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

linker: Stop using whole-archive on dependencies of dylibs #96436

Merged
merged 2 commits into from
May 2, 2022
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
37 changes: 6 additions & 31 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1920,7 +1920,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
// This change is somewhat breaking in practice due to local static libraries being linked
// as whole-archive (#85144), so removing whole-archive may be a pre-requisite.
if sess.opts.debugging_opts.link_native_libraries {
add_local_native_libraries(cmd, sess, codegen_results, crate_type);
add_local_native_libraries(cmd, sess, codegen_results);
}

// Upstream rust libraries and their nobundle static libraries
Expand Down Expand Up @@ -2092,16 +2092,6 @@ fn add_order_independent_options(
add_rpath_args(cmd, sess, codegen_results, out_filename);
}

// A dylib may reexport symbols from the linked rlib or native static library.
// Even if some symbol is reexported it's still not necessarily counted as used and may be
// dropped, at least with `ld`-like ELF linkers. So we have to link some rlibs and static
// libraries as whole-archive to avoid losing reexported symbols.
// FIXME: Find a way to mark reexported symbols as used and avoid this use of whole-archive.
fn default_to_whole_archive(sess: &Session, crate_type: CrateType, cmd: &dyn Linker) -> bool {
crate_type == CrateType::Dylib
&& !(sess.target.limit_rdylib_exports && cmd.exported_symbol_means_used_symbol())
}

/// # Native library linking
///
/// User-supplied library search paths (-L on the command line). These are the same paths used to
Expand All @@ -2115,7 +2105,6 @@ fn add_local_native_libraries(
cmd: &mut dyn Linker,
sess: &Session,
codegen_results: &CodegenResults,
crate_type: CrateType,
) {
let filesearch = sess.target_filesearch(PathKind::All);
for search_path in filesearch.search_paths() {
Expand Down Expand Up @@ -2157,7 +2146,6 @@ fn add_local_native_libraries(
}
NativeLibKind::Static { whole_archive, bundle, .. } => {
if whole_archive == Some(true)
|| (whole_archive == None && default_to_whole_archive(sess, crate_type, cmd))
// Backward compatibility case: this can be a rlib (so `+whole-archive` cannot
// be added explicitly if necessary, see the error in `fn link_rlib`) compiled
// as an executable due to `--test`. Use whole-archive implicitly, like before
Expand Down Expand Up @@ -2276,7 +2264,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
let src = &codegen_results.crate_info.used_crate_source[&cnum];
match data[cnum.as_usize() - 1] {
_ if codegen_results.crate_info.profiler_runtime == Some(cnum) => {
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, crate_type, cnum);
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, cnum);
}
// compiler-builtins are always placed last to ensure that they're
// linked correctly.
Expand All @@ -2286,7 +2274,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
}
Linkage::NotLinked | Linkage::IncludedFromDylib => {}
Linkage::Static => {
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, crate_type, cnum);
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, cnum);

// Link static native libs with "-bundle" modifier only if the crate they originate from
// is being linked statically to the current crate. If it's linked dynamically
Expand Down Expand Up @@ -2317,10 +2305,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
lib.kind
{
let verbatim = lib.verbatim.unwrap_or(false);
if whole_archive == Some(true)
|| (whole_archive == None
&& default_to_whole_archive(sess, crate_type, cmd))
{
if whole_archive == Some(true) {
cmd.link_whole_staticlib(
name,
verbatim,
Expand All @@ -2347,7 +2332,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
// was already "included" in a dylib (e.g., `libstd` when `-C prefer-dynamic`
// is used)
if let Some(cnum) = compiler_builtins {
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, crate_type, cnum);
add_static_crate::<B>(cmd, sess, codegen_results, tmpdir, cnum);
}

// Converts a library file-stem into a cc -l argument
Expand Down Expand Up @@ -2378,23 +2363,13 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
sess: &'a Session,
codegen_results: &CodegenResults,
tmpdir: &Path,
crate_type: CrateType,
cnum: CrateNum,
) {
let src = &codegen_results.crate_info.used_crate_source[&cnum];
let cratepath = &src.rlib.as_ref().unwrap().0;

let mut link_upstream = |path: &Path| {
// We don't want to include the whole compiler-builtins crate (e.g., compiler-rt)
// regardless of the default because it'll get repeatedly linked anyway.
let path = fix_windows_verbatim_for_gcc(path);
if default_to_whole_archive(sess, crate_type, cmd)
&& codegen_results.crate_info.compiler_builtins != Some(cnum)
{
cmd.link_whole_rlib(&path);
} else {
cmd.link_rlib(&path);
}
cmd.link_rlib(&fix_windows_verbatim_for_gcc(path));
};

// See the comment above in `link_staticlib` and `link_rlib` for why if
Expand Down
15 changes: 2 additions & 13 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,6 @@ pub trait Linker {
fn no_crt_objects(&mut self);
fn no_default_libraries(&mut self);
fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType, symbols: &[String]);
fn exported_symbol_means_used_symbol(&self) -> bool {
true
}
fn subsystem(&mut self, subsystem: &str);
fn group_start(&mut self);
fn group_end(&mut self);
Expand Down Expand Up @@ -728,10 +725,6 @@ impl<'a> Linker for GccLinker<'a> {
}
}

fn exported_symbol_means_used_symbol(&self) -> bool {
self.sess.target.is_like_windows || self.sess.target.is_like_osx
}

fn subsystem(&mut self, subsystem: &str) {
self.linker_arg("--subsystem");
self.linker_arg(&subsystem);
Expand Down Expand Up @@ -1479,10 +1472,6 @@ impl<'a> Linker for L4Bender<'a> {
return;
}

fn exported_symbol_means_used_symbol(&self) -> bool {
false
}

fn subsystem(&mut self, subsystem: &str) {
self.cmd.arg(&format!("--subsystem {}", subsystem));
}
Expand Down Expand Up @@ -1564,8 +1553,8 @@ pub(crate) fn linked_symbols(
crate_type: CrateType,
) -> Vec<(String, SymbolExportKind)> {
match crate_type {
CrateType::Executable | CrateType::Cdylib => (),
CrateType::Staticlib | CrateType::ProcMacro | CrateType::Rlib | CrateType::Dylib => {
CrateType::Executable | CrateType::Cdylib | CrateType::Dylib => (),
CrateType::Staticlib | CrateType::ProcMacro | CrateType::Rlib => {
return Vec::new();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/doc/rustc/src/command-line-arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ to `/WHOLEARCHIVE` for `link.exe`, and to `-force_load` for `ld64`.
The modifier does nothing for linkers that don't support it.

The default for this modifier is `-whole-archive`. \
NOTE: The default may currently be different when building dylibs for some targets,
but it is not guaranteed.
NOTE: The default may currently be different in some cases for backward compatibility,
but it is not guaranteed. If you need whole archive semantics use `+whole-archive` explicitly.

<a id="option-crate-type"></a>
## `--crate-type`: a list of types of crates for the compiler to emit
Expand Down