Auto merge of rust-lang#123656 - lqd:linker-features, r=petrochenkov

Linker flavors next steps: linker features

This is my understanding of the first step towards `@petrochenkov's` vision for the future of linker flavors, described in rust-lang#119906 (comment) and the discussion that followed.

To summarize: having `Cc` and `Lld` embedded in linker flavors creates tension about naming, and a combinatorial explosion of flavors for each new linker feature we'd want to use. Linker features are an extension mechanism that is complementary to principal flavors, with benefits described in rust-lang#119906.

The most immediate use of this flag would be to turn self-contained linking on and off via features instead of flavors. For example, `-Clinker-features=+/-lld` would toggle using lld instead of selecting a precise flavor, and would be "generic" and work cross-platform (whereas linker flavors are currently more tied to targets). Under this scheme, MCP510 is expected to be `-Clink-self-contained=+linker -Zlinker-features=+lld -Zunstable-options` (though for the time being, the original flags using lld-cc flavors still work).

I purposefully didn't add or document CLI support for `+/-cc`, as it would be a noop right now. I only expect that we'd initially want to stabilize `+/-lld` to begin with.

r? `@petrochenkov`

You had requested that minimal churn would be done to the 230 target specs and this does none yet: the linker features are inferred from the flavor since they're currently isomorphic. We of course expect this to change sooner rather than later.

In the future, we can allow targets to define linker features independently from their flavor, and remove the cc and lld components from the flavors to use the features instead, this actually doesn't need to block stabilization, as we discussed.

(Best reviewed per commit)

Author: bors

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -11,6 +11,7 @@ use rustc_metadata::fs::{copy_to_stdout, emit_wrapper_file, METADATA_FILENAME};
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::SymbolExportKind;
+use rustc_session::config::LinkerFeaturesCli;
 use rustc_session::config::{self, CFGuard, CrateType, DebugInfo, OutFileName, Strip};
 use rustc_session::config::{OutputFilenames, OutputType, PrintKind, SplitDwarfKind};
 use rustc_session::cstore::DllImport;
@@ -22,10 +23,10 @@ use rustc_session::utils::NativeLibKind;
 use rustc_session::{filesearch, Session};
 use rustc_span::symbol::Symbol;
 use rustc_target::spec::crt_objects::CrtObjects;
-use rustc_target::spec::LinkSelfContainedComponents;
 use rustc_target::spec::LinkSelfContainedDefault;
 use rustc_target::spec::LinkerFlavorCli;
 use rustc_target::spec::{Cc, LinkOutputKind, LinkerFlavor, Lld, PanicStrategy};
+use rustc_target::spec::{LinkSelfContainedComponents, LinkerFeatures};
 use rustc_target::spec::{RelocModel, RelroLevel, SanitizerSet, SplitDebuginfo};
 
 use super::archive::{ArchiveBuilder, ArchiveBuilderBuilder};
@@ -76,8 +77,8 @@ pub fn ensure_removed(dcx: &DiagCtxt, path: &Path) {
 
 /// Performs the linkage portion of the compilation phase. This will generate all
 /// of the requested outputs for this compilation session.
-pub fn link_binary<'a>(
-    sess: &'a Session,
+pub fn link_binary(
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     codegen_results: &CodegenResults,
     outputs: &OutputFilenames,
@@ -465,9 +466,9 @@ fn link_rlib<'a>(
 /// then the CodegenResults value contains one NativeLib instance for each block. However, the
 /// linker appears to expect only a single import library for each library used, so we need to
 /// collate the symbols together by library name before generating the import libraries.
-fn collate_raw_dylibs<'a, 'b>(
-    sess: &'a Session,
-    used_libraries: impl IntoIterator<Item = &'b NativeLib>,
+fn collate_raw_dylibs<'a>(
+    sess: &Session,
+    used_libraries: impl IntoIterator<Item = &'a NativeLib>,
 ) -> Result<Vec<(String, Vec<DllImport>)>, ErrorGuaranteed> {
     // Use index maps to preserve original order of imports and libraries.
     let mut dylib_table = FxIndexMap::<String, FxIndexMap<Symbol, &DllImport>>::default();
@@ -514,8 +515,8 @@ fn collate_raw_dylibs<'a, 'b>(
 ///
 /// There's no need to include metadata in a static archive, so ensure to not link in the metadata
 /// object file (and also don't prepare the archive with a metadata file).
-fn link_staticlib<'a>(
-    sess: &'a Session,
+fn link_staticlib(
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     codegen_results: &CodegenResults,
     out_filename: &Path,
@@ -627,11 +628,7 @@ fn link_staticlib<'a>(
 
 /// Use `thorin` (rust implementation of a dwarf packaging utility) to link DWARF objects into a
 /// DWARF package.
-fn link_dwarf_object<'a>(
-    sess: &'a Session,
-    cg_results: &CodegenResults,
-    executable_out_filename: &Path,
-) {
+fn link_dwarf_object(sess: &Session, cg_results: &CodegenResults, executable_out_filename: &Path) {
     let mut dwp_out_filename = executable_out_filename.to_path_buf().into_os_string();
     dwp_out_filename.push(".dwp");
     debug!(?dwp_out_filename, ?executable_out_filename);
@@ -735,8 +732,8 @@ fn link_dwarf_object<'a>(
 ///
 /// This will invoke the system linker/cc to create the resulting file. This links to all upstream
 /// files as well.
-fn link_natively<'a>(
-    sess: &'a Session,
+fn link_natively(
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     crate_type: CrateType,
     out_filename: &Path,
@@ -1100,8 +1097,8 @@ fn link_natively<'a>(
     Ok(())
 }
 
-fn strip_symbols_with_external_utility<'a>(
-    sess: &'a Session,
+fn strip_symbols_with_external_utility(
+    sess: &Session,
     util: &str,
     out_filename: &Path,
     option: Option<&str>,
@@ -1338,7 +1335,9 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
         sess: &Session,
         linker: Option<PathBuf>,
         flavor: Option<LinkerFlavor>,
+        features: LinkerFeaturesCli,
     ) -> Option<(PathBuf, LinkerFlavor)> {
+        let flavor = flavor.map(|flavor| adjust_flavor_to_features(flavor, features));
         match (linker, flavor) {
             (Some(linker), Some(flavor)) => Some((linker, flavor)),
             // only the linker flavor is known; use the default linker for the selected flavor
@@ -1386,12 +1385,33 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
                     sess.dcx().emit_fatal(errors::LinkerFileStem);
                 });
                 let flavor = sess.target.linker_flavor.with_linker_hints(stem);
+                let flavor = adjust_flavor_to_features(flavor, features);
                 Some((linker, flavor))
             }
             (None, None) => None,
         }
     }
 
+    // While linker flavors and linker features are isomorphic (and thus targets don't need to
+    // define features separately), we use the flavor as the root piece of data and have the
+    // linker-features CLI flag influence *that*, so that downstream code does not have to check for
+    // both yet.
+    fn adjust_flavor_to_features(
+        flavor: LinkerFlavor,
+        features: LinkerFeaturesCli,
+    ) -> LinkerFlavor {
+        // Note: a linker feature cannot be both enabled and disabled on the CLI.
+        if features.enabled.contains(LinkerFeatures::LLD) {
+            flavor.with_lld_enabled()
+        } else if features.disabled.contains(LinkerFeatures::LLD) {
+            flavor.with_lld_disabled()
+        } else {
+            flavor
+        }
+    }
+
+    let features = sess.opts.unstable_opts.linker_features;
+
     // linker and linker flavor specified via command line have precedence over what the target
     // specification specifies
     let linker_flavor = match sess.opts.cg.linker_flavor {
@@ -1405,14 +1425,15 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
             .linker_flavor
             .map(|flavor| sess.target.linker_flavor.with_cli_hints(flavor)),
     };
-    if let Some(ret) = infer_from(sess, sess.opts.cg.linker.clone(), linker_flavor) {
+    if let Some(ret) = infer_from(sess, sess.opts.cg.linker.clone(), linker_flavor, features) {
         return ret;
     }
 
     if let Some(ret) = infer_from(
         sess,
         sess.target.linker.as_deref().map(PathBuf::from),
         Some(sess.target.linker_flavor),
+        features,
     ) {
         return ret;
     }
@@ -2105,10 +2126,10 @@ fn add_rpath_args(
 /// 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>(
+fn linker_with_args(
     path: &Path,
     flavor: LinkerFlavor,
-    sess: &'a Session,
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     crate_type: CrateType,
     tmpdir: &Path,
@@ -2640,9 +2661,9 @@ fn add_local_native_libraries(
     );
 }
 
-fn add_upstream_rust_crates<'a>(
+fn add_upstream_rust_crates(
     cmd: &mut dyn Linker,
-    sess: &'a Session,
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     codegen_results: &CodegenResults,
     crate_type: CrateType,
@@ -2779,7 +2800,7 @@ fn add_upstream_native_libraries(
 // file generated by the MSVC linker. See https://github.com/rust-lang/rust/issues/112586.
 //
 // The returned path will always have `fix_windows_verbatim_for_gcc()` applied to it.
-fn rehome_sysroot_lib_dir<'a>(sess: &'a Session, lib_dir: &Path) -> PathBuf {
+fn rehome_sysroot_lib_dir(sess: &Session, lib_dir: &Path) -> PathBuf {
     let sysroot_lib_path = sess.target_filesearch(PathKind::All).get_lib_path();
     let canonical_sysroot_lib_path =
         { try_canonicalize(&sysroot_lib_path).unwrap_or_else(|_| sysroot_lib_path.clone()) };
@@ -2812,9 +2833,9 @@ fn rehome_sysroot_lib_dir<'a>(sess: &'a Session, lib_dir: &Path) -> PathBuf {
 // Note, however, that if we're not doing LTO we can just pass the rlib
 // blindly to the linker (fast) because it's fine if it's not actually
 // included as we're at the end of the dependency chain.
-fn add_static_crate<'a>(
+fn add_static_crate(
     cmd: &mut dyn Linker,
-    sess: &'a Session,
+    sess: &Session,
     archive_builder_builder: &dyn ArchiveBuilderBuilder,
     codegen_results: &CodegenResults,
     tmpdir: &Path,

compiler/rustc_session/src/config.rs

@@ -18,7 +18,7 @@ use rustc_feature::UnstableFeatures;
 use rustc_span::edition::{Edition, DEFAULT_EDITION, EDITION_NAME_LIST, LATEST_STABLE_EDITION};
 use rustc_span::source_map::FilePathMapping;
 use rustc_span::{FileName, FileNameDisplayPreference, RealFileName, SourceFileHashAlgorithm};
-use rustc_target::spec::LinkSelfContainedComponents;
+use rustc_target::spec::{LinkSelfContainedComponents, LinkerFeatures};
 use rustc_target::spec::{SplitDebuginfo, Target, TargetTriple};
 use std::collections::btree_map::{
     Iter as BTreeMapIter, Keys as BTreeMapKeysIter, Values as BTreeMapValuesIter,
@@ -292,6 +292,48 @@ impl LinkSelfContained {
     }
 }
 
+/// The different values that `-Z linker-features` can take on the CLI: a list of individually
+/// enabled or disabled features used during linking.
+///
+/// There is no need to enable or disable them in bulk. Each feature is fine-grained, and can be
+/// used to turn `LinkerFeatures` on or off, without needing to change the linker flavor:
+/// - using the system lld, or the self-contained `rust-lld` linker
+/// - using a C/C++ compiler to drive the linker (not yet exposed on the CLI)
+/// - etc.
+#[derive(Default, Copy, Clone, PartialEq, Debug)]
+pub struct LinkerFeaturesCli {
+    /// The linker features that are enabled on the CLI, using the `+feature` syntax.
+    pub enabled: LinkerFeatures,
+
+    /// The linker features that are disabled on the CLI, using the `-feature` syntax.
+    pub disabled: LinkerFeatures,
+}
+
+impl LinkerFeaturesCli {
+    /// Accumulates an enabled or disabled feature as specified on the CLI, if possible.
+    /// For example: `+lld`, and `-lld`.
+    pub(crate) fn handle_cli_feature(&mut self, feature: &str) -> Option<()> {
+        // Duplicate flags are reduced as we go, the last occurrence wins:
+        // `+feature,-feature,+feature` only enables the feature, and does not record it as both
+        // enabled and disabled on the CLI.
+        // We also only expose `+/-lld` at the moment, as it's currenty the only implemented linker
+        // feature and toggling `LinkerFeatures::CC` would be a noop.
+        match feature {
+            "+lld" => {
+                self.enabled.insert(LinkerFeatures::LLD);
+                self.disabled.remove(LinkerFeatures::LLD);
+                Some(())
+            }
+            "-lld" => {
+                self.disabled.insert(LinkerFeatures::LLD);
+                self.enabled.remove(LinkerFeatures::LLD);
+                Some(())
+            }
+            _ => None,
+        }
+    }
+}
+
 /// Used with `-Z assert-incr-state`.
 #[derive(Clone, Copy, PartialEq, Hash, Debug)]
 pub enum IncrementalStateAssertion {

compiler/rustc_session/src/options.rs

@@ -426,6 +426,8 @@ mod desc {
         "one of supported split dwarf modes (`split` or `single`)";
     pub const parse_link_self_contained: &str = "one of: `y`, `yes`, `on`, `n`, `no`, `off`, or a list of enabled (`+` prefix) and disabled (`-` prefix) \
         components: `crto`, `libc`, `unwind`, `linker`, `sanitizers`, `mingw`";
+    pub const parse_linker_features: &str =
+        "a list of enabled (`+` prefix) and disabled (`-` prefix) features: `lld`";
     pub const parse_polonius: &str = "either no value or `legacy` (the default), or `next`";
     pub const parse_stack_protector: &str =
         "one of (`none` (default), `basic`, `strong`, or `all`)";
@@ -1269,6 +1271,22 @@ mod parse {
         true
     }
 
+    /// Parse a comma-separated list of enabled and disabled linker features.
+    pub(crate) fn parse_linker_features(slot: &mut LinkerFeaturesCli, v: Option<&str>) -> bool {
+        match v {
+            Some(s) => {
+                for feature in s.split(',') {
+                    if slot.handle_cli_feature(feature).is_none() {
+                        return false;
+                    }
+                }
+
+                true
+            }
+            None => false,
+        }
+    }
+
     pub(crate) fn parse_wasi_exec_model(slot: &mut Option<WasiExecModel>, v: Option<&str>) -> bool {
         match v {
             Some("command") => *slot = Some(WasiExecModel::Command),
@@ -1721,6 +1739,8 @@ options! {
         "link native libraries in the linker invocation (default: yes)"),
     link_only: bool = (false, parse_bool, [TRACKED],
         "link the `.rlink` file generated by `-Z no-link` (default: no)"),
+    linker_features: LinkerFeaturesCli = (LinkerFeaturesCli::default(), parse_linker_features, [UNTRACKED],
+        "a comma-separated list of linker features to enable (+) or disable (-): `lld`"),
     lint_mir: bool = (false, parse_bool, [UNTRACKED],
         "lint MIR before and after each transformation"),
     llvm_module_flag: Vec<(String, u32, String)> = (Vec::new(), parse_llvm_module_flag, [TRACKED],

compiler/rustc_target/src/spec/mod.rs

@@ -448,6 +448,28 @@ impl LinkerFlavor {
             | LinkerFlavor::Ptx => false,
         }
     }
+
+    /// For flavors with an `Lld` component, ensure it's enabled. Otherwise, returns the given
+    /// flavor unmodified.
+    pub fn with_lld_enabled(self) -> LinkerFlavor {
+        match self {
+            LinkerFlavor::Gnu(cc, Lld::No) => LinkerFlavor::Gnu(cc, Lld::Yes),
+            LinkerFlavor::Darwin(cc, Lld::No) => LinkerFlavor::Darwin(cc, Lld::Yes),
+            LinkerFlavor::Msvc(Lld::No) => LinkerFlavor::Msvc(Lld::Yes),
+            _ => self,
+        }
+    }
+
+    /// For flavors with an `Lld` component, ensure it's disabled. Otherwise, returns the given
+    /// flavor unmodified.
+    pub fn with_lld_disabled(self) -> LinkerFlavor {
+        match self {
+            LinkerFlavor::Gnu(cc, Lld::Yes) => LinkerFlavor::Gnu(cc, Lld::No),
+            LinkerFlavor::Darwin(cc, Lld::Yes) => LinkerFlavor::Darwin(cc, Lld::No),
+            LinkerFlavor::Msvc(Lld::Yes) => LinkerFlavor::Msvc(Lld::No),
+            _ => self,
+        }
+    }
 }
 
 macro_rules! linker_flavor_cli_impls {
@@ -698,6 +720,58 @@ impl ToJson for LinkSelfContainedComponents {
     }
 }
 
+bitflags::bitflags! {
+    /// The `-Z linker-features` components that can individually be enabled or disabled.
+    ///
+    /// They are feature flags intended to be a more flexible mechanism than linker flavors, and
+    /// also to prevent a combinatorial explosion of flavors whenever a new linker feature is
+    /// required. These flags are "generic", in the sense that they can work on multiple targets on
+    /// the CLI. Otherwise, one would have to select different linkers flavors for each target.
+    ///
+    /// Here are some examples of the advantages they offer:
+    /// - default feature sets for principal flavors, or for specific targets.
+    /// - flavor-specific features: for example, clang offers automatic cross-linking with
+    ///   `--target`, which gcc-style compilers don't support. The *flavor* is still a C/C++
+    ///   compiler, and we don't need to multiply the number of flavors for this use-case. Instead,
+    ///   we can have a single `+target` feature.
+    /// - umbrella features: for example if clang accumulates more features in the future than just
+    ///   the `+target` above. That could be modeled as `+clang`.
+    /// - niche features for resolving specific issues: for example, on Apple targets the linker
+    ///   flag implementing the `as-needed` native link modifier (#99424) is only possible on
+    ///   sufficiently recent linker versions.
+    /// - still allows for discovery and automation, for example via feature detection. This can be
+    ///   useful in exotic environments/build systems.
+    #[derive(Clone, Copy, PartialEq, Eq, Default)]
+    pub struct LinkerFeatures: u8 {
+        /// Invoke the linker via a C/C++ compiler (e.g. on most unix targets).
+        const CC  = 1 << 0;
+        /// Use the lld linker, either the system lld or the self-contained linker `rust-lld`.
+        const LLD = 1 << 1;
+    }
+}
+rustc_data_structures::external_bitflags_debug! { LinkerFeatures }
+
+impl LinkerFeatures {
+    /// Parses a single `-Z linker-features` well-known feature, not a set of flags.
+    pub fn from_str(s: &str) -> Option<LinkerFeatures> {
+        Some(match s {
+            "cc" => LinkerFeatures::CC,
+            "lld" => LinkerFeatures::LLD,
+            _ => return None,
+        })
+    }
+
+    /// Returns whether the `lld` linker feature is enabled.
+    pub fn is_lld_enabled(self) -> bool {
+        self.contains(LinkerFeatures::LLD)
+    }
+
+    /// Returns whether the `cc` linker feature is enabled.
+    pub fn is_cc_enabled(self) -> bool {
+        self.contains(LinkerFeatures::CC)
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Hash, Encodable, Decodable, HashStable_Generic)]
 pub enum PanicStrategy {
     Unwind,