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

Jump to bottom

linker: Reorder linker arguments #85086

Merged
merged 4 commits into from
Jun 6, 2021
+164 −165
Merged

linker: Reorder linker arguments #85086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dca3acb
linker: Inline `fn link_local_crate_native_libs_and_dependent_crate_l…
petrochenkov May 8, 2021
b7994f9
linker: Reorder linker arguments
petrochenkov May 8, 2021
5275bf1
linker: Restore the old order of linking native libraries
petrochenkov May 10, 2021
3eab280
linker: Re-apply Solaris fixes for `-z ignore`
petrochenkov Jun 6, 2021
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
296 changes: 160 additions & 136 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 @@ -1058,7 +1058,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>(
}
}

fn link_sanitizers(sess: &Session, crate_type: CrateType, linker: &mut dyn Linker) {
fn add_sanitizer_libraries(sess: &Session, crate_type: CrateType, linker: &mut dyn Linker) {
// On macOS the runtimes are distributed as dylibs which should be linked to
// both executables and dynamic shared objects. Everywhere else the runtimes
// are currently distributed as static liraries which should be linked to
Expand Down Expand Up @@ -1680,55 +1680,6 @@ fn add_local_crate_metadata_objects(
}
}

/// Link native libraries corresponding to the current crate and all libraries corresponding to
/// all its dependency crates.
/// FIXME: Consider combining this with the functions above adding object files for the local crate.
fn link_local_crate_native_libs_and_dependent_crate_libs<'a, B: ArchiveBuilder<'a>>(
cmd: &mut dyn Linker,
sess: &'a Session,
crate_type: CrateType,
codegen_results: &CodegenResults,
tmpdir: &Path,
) {
// Take careful note of the ordering of the arguments we pass to the linker
// here. Linkers will assume that things on the left depend on things to the
// right. Things on the right cannot depend on things on the left. This is
// all formally implemented in terms of resolving symbols (libs on the right
// resolve unknown symbols of libs on the left, but not vice versa).
//
// For this reason, we have organized the arguments we pass to the linker as
// such:
//
// 1. The local object that LLVM just generated
// 2. Local native libraries
// 3. Upstream rust libraries
// 4. Upstream native libraries
//
// The rationale behind this ordering is that those items lower down in the
// list can't depend on items higher up in the list. For example nothing can
// depend on what we just generated (e.g., that'd be a circular dependency).
// Upstream rust libraries are not allowed to depend on our local native
// libraries as that would violate the structure of the DAG, in that
// scenario they are required to link to them as well in a shared fashion.
//
// Note that upstream rust libraries may contain native dependencies as
// well, but they also can't depend on what we just started to add to the
// link line. And finally upstream native libraries can't depend on anything
// in this DAG so far because they're only dylibs and dylibs can only depend
// on other dylibs (e.g., other native deps).
//
// If -Zlink-native-libraries=false is set, then the assumption is that an
// external build system already has the native dependencies defined, and it
// will provide them to the linker itself.
if sess.opts.debugging_opts.link_native_libraries {
add_local_native_libraries(cmd, sess, codegen_results);
}
add_upstream_rust_crates::<B>(cmd, sess, codegen_results, crate_type, tmpdir);
if sess.opts.debugging_opts.link_native_libraries {
add_upstream_native_libraries(cmd, sess, codegen_results, crate_type);
}
}

/// Add sysroot and other globally set directories to the directory search list.
fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session, self_contained: bool) {
// The default library location, we need this to find the runtime.
Expand Down Expand Up @@ -1787,12 +1738,13 @@ fn add_rpath_args(
}

/// Produce the linker command line containing linker path and arguments.
/// `NO-OPT-OUT` marks the arguments that cannot be removed from the command line
/// by the user without creating a custom target specification.
/// `OBJECT-FILES` specify whether the arguments can add object files.
/// `CUSTOMIZATION-POINT` means that arbitrary arguments defined by the user
/// or by the target spec can be inserted here.
/// `AUDIT-ORDER` - need to figure out whether the option is order-dependent or not.
///
/// When comments in the function say "order-(in)dependent" they mean order-dependence between
/// options and libraries/object files. For example `--whole-archive` (order-dependent) applies
/// to specific libraries passed after it, and `-o` (output file, order-independent) applies
/// to the linking process as a whole.
/// Order-independent options may still override each other in order-dependent fashion,
/// e.g `--foo=yes --foo=no` may be equivalent to `--foo=no`.
fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
path: &Path,
flavor: LinkerFlavor,
Expand All @@ -1810,16 +1762,151 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
let cmd = &mut *codegen_results.linker_info.to_linker(base_cmd, &sess, flavor);
let link_output_kind = link_output_kind(sess, crate_type);

// NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT
// ------------ Early order-dependent options ------------

// If we're building something like a dynamic library then some platforms
// need to make sure that all symbols are exported correctly from the
// dynamic library.
// Must be passed before any libraries to prevent the symbols to export from being thrown away,
// at least on some platforms (e.g. windows-gnu).
cmd.export_symbols(tmpdir, crate_type);

// Can be used for adding custom CRT objects or overriding order-dependent options above.
// FIXME: In practice built-in target specs use this for arbitrary order-independent options,
// introduce a target spec option for order-independent linker options and migrate built-in
// specs to it.
add_pre_link_args(cmd, sess, flavor);

// NO-OPT-OUT, OBJECT-FILES-NO
// ------------ Object code and libraries, order-dependent ------------

// Pre-link CRT objects.
add_pre_link_objects(cmd, sess, link_output_kind, crt_objects_fallback);
Copy link
Member

Choose a reason for hiding this comment

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

AFAICT in the past we did not export_symbols from the CRT objects nor the sanitizer libraries. Are there bugs that are fixed by this re-ordering? And if so, can they be linked in the commit message, or at least this change called out if there isn't an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving export_symbols to before add_pre_link_objects doesn't fix any issues, it's just for gathering objects/libraries into one group and options to another.


// Sanitizer libraries.
add_sanitizer_libraries(sess, crate_type, cmd);

// Object code from the current crate.
// Take careful note of the ordering of the arguments we pass to the linker
// here. Linkers will assume that things on the left depend on things to the
// right. Things on the right cannot depend on things on the left. This is
// all formally implemented in terms of resolving symbols (libs on the right
// resolve unknown symbols of libs on the left, but not vice versa).
//
// For this reason, we have organized the arguments we pass to the linker as
// such:
//
// 1. The local object that LLVM just generated
// 2. Local native libraries
// 3. Upstream rust libraries
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
// 4. Upstream native libraries
//
// The rationale behind this ordering is that those items lower down in the
// list can't depend on items higher up in the list. For example nothing can
// depend on what we just generated (e.g., that'd be a circular dependency).
// Upstream rust libraries are not supposed to depend on our local native
// libraries as that would violate the structure of the DAG, in that
// scenario they are required to link to them as well in a shared fashion.
// (The current implementation still doesn't prevent it though, see the FIXME below.)
//
// Note that upstream rust libraries may contain native dependencies as
// well, but they also can't depend on what we just started to add to the
// link line. And finally upstream native libraries can't depend on anything
// in this DAG so far because they can only depend on other native libraries
// and such dependencies are also required to be specified.
add_local_crate_regular_objects(cmd, codegen_results);
add_local_crate_metadata_objects(cmd, crate_type, codegen_results);
add_local_crate_allocator_objects(cmd, codegen_results);

// Avoid linking to dynamic libraries unless they satisfy some undefined symbols
// at the point at which they are specified on the command line.
// Must be passed before any (dynamic) libraries to have effect on them.
// On Solaris-like systems, `-z ignore` acts as both `--as-needed` and `--gc-sections`
// so it will ignore unreferenced ELF sections from relocatable objects.
// For that reason, we put this flag after metadata objects as they would otherwise be removed.
// FIXME: Support more fine-grained dead code removal on Solaris/illumos
// and move this option back to the top.
cmd.add_as_needed();

// FIXME: Move this below to other native libraries
// (or alternatively link all native libraries after their respective crates).
// 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);
}

// Rust libraries.
add_upstream_rust_crates::<B>(cmd, sess, codegen_results, crate_type, tmpdir);

// Native libraries linked with `#[link]` attributes at and `-l` command line options.
// If -Zlink-native-libraries=false is set, then the assumption is that an
// external build system already has the native dependencies defined, and it
// will provide them to the linker itself.
if sess.opts.debugging_opts.link_native_libraries {
add_upstream_native_libraries(cmd, sess, codegen_results, crate_type);
}

// Library linking above uses some global state for things like `-Bstatic`/`-Bdynamic` to make
// command line shorter, reset it to default here before adding more libraries.
cmd.reset_per_library_state();

// FIXME: Built-in target specs occasionally use this for linking system libraries,
// eliminate all such uses by migrating them to `#[link]` attributes in `lib(std,c,unwind)`
// and remove the option.
add_late_link_args(cmd, sess, flavor, crate_type, codegen_results);

// ------------ Arbitrary order-independent options ------------

// Add order-independent options determined by rustc from its compiler options,
// target properties and source code.
add_order_independent_options(
cmd,
sess,
link_output_kind,
crt_objects_fallback,
flavor,
crate_type,
codegen_results,
out_filename,
tmpdir,
);

// Can be used for arbitrary order-independent options.
// In practice may also be occasionally used for linking native libraries.
// Passed after compiler-generated options to support manual overriding when necessary.
add_user_defined_link_args(cmd, sess);

// ------------ Object code and libraries, order-dependent ------------

// Post-link CRT objects.
add_post_link_objects(cmd, sess, link_output_kind, crt_objects_fallback);

// ------------ Late order-dependent options ------------

// Doesn't really make sense.
// FIXME: In practice built-in target specs use this for arbitrary order-independent options,
// introduce a target spec option for order-independent linker options, migrate built-in specs
// to it and remove the option.
add_post_link_args(cmd, sess, flavor);

cmd.take_cmd()
}

fn add_order_independent_options(
cmd: &mut dyn Linker,
sess: &Session,
link_output_kind: LinkOutputKind,
crt_objects_fallback: bool,
flavor: LinkerFlavor,
crate_type: CrateType,
codegen_results: &CodegenResults,
out_filename: &Path,
tmpdir: &Path,
) {
add_apple_sdk(cmd, sess, flavor);

// NO-OPT-OUT
add_link_script(cmd, sess, tmpdir, crate_type);

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
if sess.target.is_like_fuchsia && crate_type == CrateType::Executable {
let prefix = if sess.opts.debugging_opts.sanitizer.contains(SanitizerSet::ADDRESS) {
"asan/"
Expand All @@ -1829,36 +1916,17 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
cmd.arg(format!("--dynamic-linker={}ld.so.1", prefix));
}

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
if sess.target.eh_frame_header {
cmd.add_eh_frame_header();
}

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
// Make the binary compatible with data execution prevention schemes.
cmd.add_no_exec();

// OBJECT-FILES-YES
add_local_crate_metadata_objects(cmd, crate_type, codegen_results);

// NO-OPT-OUT, OBJECT-FILES-NO
// Avoid linking to dynamic libraries unless they satisfy some undefined symbols
// at the point at which they are specified on the command line.
// Must be passed before any dynamic libraries.
// On solaris-like systems, this also will ignore unreferenced ELF sections
// from relocatable objects. For that reason, we move the metadata objects
// to before this flag as they would otherwise be removed.
cmd.add_as_needed();

// NO-OPT-OUT, OBJECT-FILES-NO
if crt_objects_fallback {
cmd.no_crt_objects();
}

// NO-OPT-OUT, OBJECT-FILES-YES
add_pre_link_objects(cmd, sess, link_output_kind, crt_objects_fallback);

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
if sess.target.is_like_emscripten {
cmd.arg("-s");
cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort {
Expand All @@ -1868,108 +1936,64 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
});
}

// OBJECT-FILES-YES, AUDIT-ORDER
link_sanitizers(sess, crate_type, cmd);
if flavor == LinkerFlavor::PtxLinker {
// Provide the linker with fallback to internal `target-cpu`.
cmd.arg("--fallback-arch");
cmd.arg(&codegen_results.linker_info.target_cpu);
} else if flavor == LinkerFlavor::BpfLinker {
cmd.arg("--cpu");
cmd.arg(&codegen_results.linker_info.target_cpu);
cmd.arg("--cpu-features");
cmd.arg(match &sess.opts.cg.target_feature {
feat if !feat.is_empty() => feat,
_ => &sess.target.options.features,
});
}

// OBJECT-FILES-NO, AUDIT-ORDER
// Linker plugins should be specified early in the list of arguments
// FIXME: How "early" exactly?
cmd.linker_plugin_lto();

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
// FIXME: Order-dependent, at least relatively to other args adding searh directories.
add_library_search_dirs(cmd, sess, crt_objects_fallback);

// OBJECT-FILES-YES
add_local_crate_regular_objects(cmd, codegen_results);

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
cmd.output_filename(out_filename);

// OBJECT-FILES-NO, AUDIT-ORDER
if crate_type == CrateType::Executable && sess.target.is_like_windows {
if let Some(ref s) = codegen_results.windows_subsystem {
cmd.subsystem(s);
}
}

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
// If we're building something like a dynamic library then some platforms
// need to make sure that all symbols are exported correctly from the
// dynamic library.
cmd.export_symbols(tmpdir, crate_type);

// OBJECT-FILES-YES
add_local_crate_allocator_objects(cmd, codegen_results);

// OBJECT-FILES-NO, AUDIT-ORDER
// FIXME: Order dependent, applies to the following objects. Where should it be placed?
// Try to strip as much out of the generated object by removing unused
// sections if possible. See more comments in linker.rs
if !sess.link_dead_code() {
let keep_metadata = crate_type == CrateType::Dylib;
cmd.gc_sections(keep_metadata);
}

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
cmd.set_output_kind(link_output_kind, out_filename);

// OBJECT-FILES-NO, AUDIT-ORDER
add_relro_args(cmd, sess);

// OBJECT-FILES-NO, AUDIT-ORDER
// Pass optimization flags down to the linker.
cmd.optimize();

// OBJECT-FILES-NO, AUDIT-ORDER
// Pass debuginfo and strip flags down to the linker.
cmd.debuginfo(sess.opts.debugging_opts.strip);

// OBJECT-FILES-NO, AUDIT-ORDER
// We want to prevent the compiler from accidentally leaking in any system libraries,
// so by default we tell linkers not to link to any default libraries.
if !sess.opts.cg.default_linker_libraries && sess.target.no_default_libraries {
cmd.no_default_libraries();
}

// OBJECT-FILES-YES
link_local_crate_native_libs_and_dependent_crate_libs::<B>(
cmd,
sess,
crate_type,
codegen_results,
tmpdir,
);

// OBJECT-FILES-NO, AUDIT-ORDER
if sess.opts.cg.profile_generate.enabled() || sess.instrument_coverage() {
cmd.pgo_gen();
}

// OBJECT-FILES-NO, AUDIT-ORDER
if sess.opts.cg.control_flow_guard != CFGuard::Disabled {
cmd.control_flow_guard();
}

// OBJECT-FILES-NO, AUDIT-ORDER
add_rpath_args(cmd, sess, codegen_results, out_filename);

// OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT
add_user_defined_link_args(cmd, sess);

// NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER
cmd.finalize();

// NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT
add_late_link_args(cmd, sess, flavor, crate_type, codegen_results);

// NO-OPT-OUT, OBJECT-FILES-YES
add_post_link_objects(cmd, sess, link_output_kind, crt_objects_fallback);

// NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT
add_post_link_args(cmd, sess, flavor);

cmd.take_cmd()
}

/// # Native library linking
Expand Down
Loading