linker: Reorder linker arguments

- Combine all native library arguments together, to simplify potential support for library deduplication and similar things
- Split arguments into order-independent and order-dependent, to define more precisely what (pre,post,late)-link-args mean

2 files changed, 151 insertions, 146 deletions

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 6116013aa84..34d70843a23 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -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
@@ -1738,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,
@@ -1761,16 +1762,138 @@ 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 ------------
+
+    // 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.
+    cmd.add_as_needed();
+
+    // 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);
+
+    // 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. Upstream rust libraries
+    // 3. Local native 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 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);
+
+    // 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_local_native_libraries(cmd, sess, codegen_results);
+        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/"
@@ -1780,36 +1903,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 {
@@ -1819,42 +1923,32 @@ 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() {
@@ -1862,95 +1956,31 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
         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-NO, AUDIT-ORDER
-    // 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);
-    }
-
-    // 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
diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs
index 9152a69e7a1..1d53e9ad8b3 100644
--- a/compiler/rustc_codegen_ssa/src/back/linker.rs
+++ b/compiler/rustc_codegen_ssa/src/back/linker.rs
@@ -37,7 +37,7 @@ pub fn disable_localization(linker: &mut Command) {
 /// need out of the shared crate context before we get rid of it.
 #[derive(Encodable, Decodable)]
 pub struct LinkerInfo {
-    target_cpu: String,
+    pub(super) target_cpu: String,
     exports: FxHashMap<CrateType, Vec<String>>,
 }
 
@@ -81,9 +81,7 @@ impl LinkerInfo {
                 Box::new(WasmLd::new(cmd, sess, self)) as Box<dyn Linker>
             }
 
-            LinkerFlavor::PtxLinker => {
-                Box::new(PtxLinker { cmd, sess, info: self }) as Box<dyn Linker>
-            }
+            LinkerFlavor::PtxLinker => Box::new(PtxLinker { cmd, sess }) as Box<dyn Linker>,
 
             LinkerFlavor::BpfLinker => {
                 Box::new(BpfLinker { cmd, sess, info: self }) as Box<dyn Linker>
@@ -132,7 +130,7 @@ pub trait Linker {
     fn add_eh_frame_header(&mut self) {}
     fn add_no_exec(&mut self) {}
     fn add_as_needed(&mut self) {}
-    fn finalize(&mut self);
+    fn reset_per_library_state(&mut self) {}
 }
 
 impl dyn Linker + '_ {
@@ -653,7 +651,7 @@ impl<'a> Linker for GccLinker<'a> {
         self.linker_arg(&subsystem);
     }
 
-    fn finalize(&mut self) {
+    fn reset_per_library_state(&mut self) {
         self.hint_dynamic(); // Reset to default before returning the composed command line.
     }
 
@@ -937,8 +935,6 @@ impl<'a> Linker for MsvcLinker<'a> {
         }
     }
 
-    fn finalize(&mut self) {}
-
     // MSVC doesn't need group indicators
     fn group_start(&mut self) {}
     fn group_end(&mut self) {}
@@ -1099,8 +1095,6 @@ impl<'a> Linker for EmLinker<'a> {
         // noop
     }
 
-    fn finalize(&mut self) {}
-
     // Appears not necessary on Emscripten
     fn group_start(&mut self) {}
     fn group_end(&mut self) {}
@@ -1281,8 +1275,6 @@ impl<'a> Linker for WasmLd<'a> {
 
     fn subsystem(&mut self, _subsystem: &str) {}
 
-    fn finalize(&mut self) {}
-
     // Not needed for now with LLD
     fn group_start(&mut self) {}
     fn group_end(&mut self) {}
@@ -1336,7 +1328,6 @@ fn exported_symbols(tcx: TyCtxt<'_>, crate_type: CrateType) -> Vec<String> {
 pub struct PtxLinker<'a> {
     cmd: Command,
     sess: &'a Session,
-    info: &'a LinkerInfo,
 }
 
 impl<'a> Linker for PtxLinker<'a> {
@@ -1380,11 +1371,6 @@ impl<'a> Linker for PtxLinker<'a> {
         self.cmd.arg("-o").arg(path);
     }
 
-    fn finalize(&mut self) {
-        // Provide the linker with fallback to internal `target-cpu`.
-        self.cmd.arg("--fallback-arch").arg(&self.info.target_cpu);
-    }
-
     fn link_dylib(&mut self, _lib: Symbol, _verbatim: bool, _as_needed: bool) {
         panic!("external dylibs not supported")
     }
@@ -1486,17 +1472,6 @@ impl<'a> Linker for BpfLinker<'a> {
         self.cmd.arg("-o").arg(path);
     }
 
-    fn finalize(&mut self) {
-        self.cmd.arg("--cpu").arg(match self.sess.opts.cg.target_cpu {
-            Some(ref s) => s,
-            None => &self.sess.target.options.cpu,
-        });
-        self.cmd.arg("--cpu-features").arg(match &self.sess.opts.cg.target_feature {
-            feat if !feat.is_empty() => feat,
-            _ => &self.sess.target.options.features,
-        });
-    }
-
     fn link_dylib(&mut self, _lib: Symbol, _verbatim: bool, _as_needed: bool) {
         panic!("external dylibs not supported")
     }