Auto merge of #116278 - Kobzol:bootstrap-lld-mode, r=albertlarsan68,p…

…etrochenkov

Generalize LLD usage in bootstrap

The current usage of using LLD (`rust.use-lld = true`) in bootstrap is a bit messy. What it claimed:

> Indicates whether LLD will be used to link Rust crates during bootstrap on
> supported platforms. The LLD from the bootstrap distribution will be used
> and not the LLD compiled during the bootstrap.

What it did:
1) On MSVC, it did indeed use the snapshot compiler's `rust-lld`, but at the same time it was invoking a global `lld` binary (since #102101), therefore it wouldn't work if `lld` wasn't available.
2) On other targets, it was just straight up using a global `lld` linker. If it wasn't available, it would fail.

This PR (hopefully) cleans up handling of LLD in bootstrap. It introduces a new enum called `LldMode`, which explicitly distinguishes between no LLD, external LLD and self-contained LLD. Since it's non-trivial to provide a custom path to LLD, if an external `lld` is used, the linker binary has to be named exactly `lld` and it has to be available in PATH.

In addition, this PR also dog-foods [MCP510](rust-lang/compiler-team#510) in bootstrap.

To keep backwards compatibility somewhat, I kept the original `use-lld` flag and mapped the `true` value to `"external"`, which is how it behaved before on Linux and other non-MSVC targets.

Having the option to use an external `lld` on Linux should come in handy for testing on CI once MCP510 sets the default linker on Linux to `lld`.

Note that thanks to MCP510, currently "self-contained" means that `lld` is used from the stage N-1 compiler (before, we always used `lld` from the snapshot/stage0 compiler).

Best reviewed commit by commit.

CC `@petrochenkov`

config.example.toml

@@ -648,10 +648,12 @@ change-id = 117813
 #lld = false
 
 # Indicates whether LLD will be used to link Rust crates during bootstrap on
-# supported platforms. The LLD from the bootstrap distribution will be used
-# and not the LLD compiled during the bootstrap.
+# supported platforms.
+# If set to `true` or `"external"`, a global `lld` binary that has to be in $PATH
+# will be used.
+# If set to `"self-contained"`, rust-lld from the snapshot compiler will be used.
 #
-# LLD will not be used if we're cross linking.
+# On MSVC, LLD will not be used if we're cross linking.
 #
 # Explicitly setting the linker for a target will override this option when targeting MSVC.
 #use-lld = false

src/bootstrap/src/core/build_steps/compile.rs

@@ -870,7 +870,7 @@ impl Step for Rustc {
         // is already on by default in MSVC optimized builds, which is interpreted as --icf=all:
         // https://github.com/llvm/llvm-project/blob/3329cec2f79185bafd678f310fafadba2a8c76d2/lld/COFF/Driver.cpp#L1746
         // https://github.com/rust-lang/rust/blob/f22819bcce4abaff7d1246a56eec493418f9f4ee/compiler/rustc_codegen_ssa/src/back/linker.rs#L827
-        if builder.config.use_lld && !compiler.host.contains("msvc") {
+        if builder.config.lld_mode.is_used() && !compiler.host.is_msvc() {
             cargo.rustflag("-Clink-args=-Wl,--icf=all");
         }
 
@@ -1089,7 +1089,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect
     // found. This is to avoid the linker errors about undefined references to
     // `__llvm_profile_instrument_memop` when linking `rustc_driver`.
     let mut llvm_linker_flags = String::new();
-    if builder.config.llvm_profile_generate && target.contains("msvc") {
+    if builder.config.llvm_profile_generate && target.is_msvc() {
         if let Some(ref clang_cl_path) = builder.config.llvm_clang_cl {
             // Add clang's runtime library directory to the search path
             let clang_rt_dir = get_clang_cl_resource_dir(clang_cl_path);
@@ -1114,7 +1114,7 @@ fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelect
     // not for MSVC or macOS
     if builder.config.llvm_static_stdcpp
         && !target.contains("freebsd")
-        && !target.contains("msvc")
+        && !target.is_msvc()
         && !target.contains("apple")
         && !target.contains("solaris")
     {

src/bootstrap/src/core/build_steps/llvm.rs

@@ -98,7 +98,7 @@ pub fn prebuilt_llvm_config(
     let out_dir = builder.llvm_out(target);
 
     let mut llvm_config_ret_dir = builder.llvm_out(builder.config.build);
-    if !builder.config.build.contains("msvc") || builder.ninja() {
+    if !builder.config.build.is_msvc() || builder.ninja() {
         llvm_config_ret_dir.push("build");
     }
     llvm_config_ret_dir.push("bin");
@@ -411,7 +411,7 @@ impl Step for Llvm {
             ldflags.shared.push(" -latomic");
         }
 
-        if target.contains("msvc") {
+        if target.is_msvc() {
             cfg.define("LLVM_USE_CRT_DEBUG", "MT");
             cfg.define("LLVM_USE_CRT_RELEASE", "MT");
             cfg.define("LLVM_USE_CRT_RELWITHDEBINFO", "MT");
@@ -644,7 +644,7 @@ fn configure_cmake(
     }
 
     let sanitize_cc = |cc: &Path| {
-        if target.contains("msvc") {
+        if target.is_msvc() {
             OsString::from(cc.to_str().unwrap().replace("\\", "/"))
         } else {
             cc.as_os_str().to_owned()
@@ -654,7 +654,7 @@ fn configure_cmake(
     // MSVC with CMake uses msbuild by default which doesn't respect these
     // vars that we'd otherwise configure. In that case we just skip this
     // entirely.
-    if target.contains("msvc") && !builder.ninja() {
+    if target.is_msvc() && !builder.ninja() {
         return;
     }
 
@@ -664,7 +664,7 @@ fn configure_cmake(
     };
 
     // Handle msvc + ninja + ccache specially (this is what the bots use)
-    if target.contains("msvc") && builder.ninja() && builder.config.ccache.is_some() {
+    if target.is_msvc() && builder.ninja() && builder.config.ccache.is_some() {
         let mut wrap_cc = env::current_exe().expect("failed to get cwd");
         wrap_cc.set_file_name("sccache-plus-cl.exe");
 
@@ -768,7 +768,7 @@ fn configure_cmake(
     // For distribution we want the LLVM tools to be *statically* linked to libstdc++.
     // We also do this if the user explicitly requested static libstdc++.
     if builder.config.llvm_static_stdcpp
-        && !target.contains("msvc")
+        && !target.is_msvc()
         && !target.contains("netbsd")
         && !target.contains("solaris")
     {
@@ -874,7 +874,7 @@ impl Step for Lld {
         // when doing PGO on CI, cmake or clang-cl don't automatically link clang's
         // profiler runtime in. In that case, we need to manually ask cmake to do it, to avoid
         // linking errors, much like LLVM's cmake setup does in that situation.
-        if builder.config.llvm_profile_generate && target.contains("msvc") {
+        if builder.config.llvm_profile_generate && target.is_msvc() {
             if let Some(clang_cl_path) = builder.config.llvm_clang_cl.as_ref() {
                 // Find clang's runtime library directory and push that as a search path to the
                 // cmake linker flags.

src/bootstrap/src/core/build_steps/test.rs

@@ -29,8 +29,9 @@ use crate::utils;
 use crate::utils::cache::{Interned, INTERNER};
 use crate::utils::exec::BootstrapCommand;
 use crate::utils::helpers::{
-    self, add_link_lib_path, add_rustdoc_cargo_lld_flags, add_rustdoc_lld_flags, dylib_path,
-    dylib_path_var, output, t, target_supports_cranelift_backend, up_to_date, LldThreads,
+    self, add_link_lib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var,
+    linker_args, linker_flags, output, t, target_supports_cranelift_backend, up_to_date,
+    LldThreads,
 };
 use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
 use crate::{envify, CLang, DocTests, GitRepo, Mode};
@@ -277,7 +278,7 @@ impl Step for Cargotest {
             .args(builder.config.test_args())
             .env("RUSTC", builder.rustc(compiler))
             .env("RUSTDOC", builder.rustdoc(compiler));
-        add_rustdoc_cargo_lld_flags(&mut cmd, builder, compiler.host, LldThreads::No);
+        add_rustdoc_cargo_linker_args(&mut cmd, builder, compiler.host, LldThreads::No);
         builder.run_delaying_failure(cmd);
     }
 }
@@ -863,7 +864,7 @@ impl Step for RustdocTheme {
             .env("CFG_RELEASE_CHANNEL", &builder.config.channel)
             .env("RUSTDOC_REAL", builder.rustdoc(self.compiler))
             .env("RUSTC_BOOTSTRAP", "1");
-        add_rustdoc_lld_flags(&mut cmd, builder, self.compiler.host, LldThreads::No);
+        cmd.args(linker_args(builder, self.compiler.host, LldThreads::No));
 
         builder.run_delaying_failure(&mut cmd);
     }
@@ -1039,7 +1040,7 @@ impl Step for RustdocGUI {
         cmd.env("RUSTDOC", builder.rustdoc(self.compiler))
             .env("RUSTC", builder.rustc(self.compiler));
 
-        add_rustdoc_cargo_lld_flags(&mut cmd, builder, self.compiler.host, LldThreads::No);
+        add_rustdoc_cargo_linker_args(&mut cmd, builder, self.compiler.host, LldThreads::No);
 
         for path in &builder.paths {
             if let Some(p) = helpers::is_valid_test_suite_arg(path, "tests/rustdoc-gui", builder) {
@@ -1746,14 +1747,14 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
 
         let mut hostflags = flags.clone();
         hostflags.push(format!("-Lnative={}", builder.test_helpers_out(compiler.host).display()));
-        hostflags.extend(builder.lld_flags(compiler.host));
+        hostflags.extend(linker_flags(builder, compiler.host, LldThreads::No));
         for flag in hostflags {
             cmd.arg("--host-rustcflags").arg(flag);
         }
 
         let mut targetflags = flags;
         targetflags.push(format!("-Lnative={}", builder.test_helpers_out(target).display()));
-        targetflags.extend(builder.lld_flags(target));
+        targetflags.extend(linker_flags(builder, compiler.host, LldThreads::No));
         for flag in targetflags {
             cmd.arg("--target-rustcflags").arg(flag);
         }
@@ -1931,7 +1932,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
         //
         // Note that if we encounter `PATH` we make sure to append to our own `PATH`
         // rather than stomp over it.
-        if !builder.config.dry_run() && target.contains("msvc") {
+        if !builder.config.dry_run() && target.is_msvc() {
             for &(ref k, ref v) in builder.cc.borrow()[&target].env() {
                 if k != "PATH" {
                     cmd.env(k, v);
@@ -3070,7 +3071,7 @@ impl Step for TestHelpers {
         // We may have found various cross-compilers a little differently due to our
         // extra configuration, so inform cc of these compilers. Note, though, that
         // on MSVC we still need cc's detection of env vars (ugh).
-        if !target.contains("msvc") {
+        if !target.is_msvc() {
             if let Some(ar) = builder.ar(target) {
                 cfg.archiver(ar);
             }

src/bootstrap/src/core/build_steps/tool.rs

@@ -833,7 +833,7 @@ impl<'a> Builder<'a> {
         // On MSVC a tool may invoke a C compiler (e.g., compiletest in run-make
         // mode) and that C compiler may need some extra PATH modification. Do
         // so here.
-        if compiler.host.contains("msvc") {
+        if compiler.host.is_msvc() {
             let curpaths = env::var_os("PATH").unwrap_or_default();
             let curpaths = env::split_paths(&curpaths).collect::<Vec<_>>();
             for &(ref k, ref v) in self.cc.borrow()[&compiler.host].env() {

src/bootstrap/src/core/builder.rs

@@ -19,8 +19,8 @@ use crate::core::build_steps::{check, clean, compile, dist, doc, install, run, s
 use crate::core::config::flags::{Color, Subcommand};
 use crate::core::config::{DryRun, SplitDebuginfo, TargetSelection};
 use crate::utils::cache::{Cache, Interned, INTERNER};
-use crate::utils::helpers::{self, add_dylib_path, add_link_lib_path, add_rustdoc_lld_flags, exe};
-use crate::utils::helpers::{libdir, output, t, LldThreads};
+use crate::utils::helpers::{self, add_dylib_path, add_link_lib_path, exe, linker_args};
+use crate::utils::helpers::{libdir, linker_flags, output, t, LldThreads};
 use crate::Crate;
 use crate::EXTRA_CHECK_CFGS;
 use crate::{Build, CLang, DocTests, GitRepo, Mode};
@@ -1174,7 +1174,7 @@ impl<'a> Builder<'a> {
         cmd.env_remove("MAKEFLAGS");
         cmd.env_remove("MFLAGS");
 
-        add_rustdoc_lld_flags(&mut cmd, self, compiler.host, LldThreads::Yes);
+        cmd.args(linker_args(self, compiler.host, LldThreads::Yes));
         cmd
     }
 
@@ -1673,23 +1673,22 @@ impl<'a> Builder<'a> {
             rustflags.arg(&format!("-Zstack-protector={stack_protector}"));
         }
 
-        if let Some(host_linker) = self.linker(compiler.host) {
-            hostflags.arg(format!("-Clinker={}", host_linker.display()));
-        }
-        if self.is_fuse_ld_lld(compiler.host) {
-            hostflags.arg("-Clink-args=-fuse-ld=lld");
+        for arg in linker_args(self, compiler.host, LldThreads::Yes) {
+            hostflags.arg(&arg);
         }
 
         if let Some(target_linker) = self.linker(target) {
             let target = crate::envify(&target.triple);
             cargo.env(&format!("CARGO_TARGET_{target}_LINKER"), target_linker);
         }
-        if self.is_fuse_ld_lld(target) {
-            rustflags.arg("-Clink-args=-fuse-ld=lld");
+        // We want to set -Clinker using Cargo, therefore we only call `linker_flags` and not
+        // `linker_args` here.
+        for flag in linker_flags(self, target, LldThreads::Yes) {
+            rustflags.arg(&flag);
+        }
+        for arg in linker_args(self, target, LldThreads::Yes) {
+            rustdocflags.arg(&arg);
         }
-        self.lld_flags(target).for_each(|flag| {
-            rustdocflags.arg(&flag);
-        });
 
         if !(["build", "check", "clippy", "fix", "rustc"].contains(&cmd)) && want_rustdoc {
             cargo.env("RUSTDOC_LIBDIR", self.rustc_libdir(compiler));
@@ -1729,8 +1728,7 @@ impl<'a> Builder<'a> {
 
         let split_debuginfo_is_stable = target.contains("linux")
             || target.contains("apple")
-            || (target.contains("msvc")
-                && self.config.rust_split_debuginfo == SplitDebuginfo::Packed)
+            || (target.is_msvc() && self.config.rust_split_debuginfo == SplitDebuginfo::Packed)
             || (target.contains("windows")
                 && self.config.rust_split_debuginfo == SplitDebuginfo::Off);
 
@@ -1909,7 +1907,7 @@ impl<'a> Builder<'a> {
         // the options through environment variables that are fetched and understood by both.
         //
         // FIXME: the guard against msvc shouldn't need to be here
-        if target.contains("msvc") {
+        if target.is_msvc() {
             if let Some(ref cl) = self.config.llvm_clang_cl {
                 cargo.env("CC", cl).env("CXX", cl);
             }

src/bootstrap/src/core/config/config.rs

@@ -105,6 +105,39 @@ impl Display for DebuginfoLevel {
     }
 }
 
+/// LLD in bootstrap works like this:
+/// - Self-contained lld: use `rust-lld` from the compiler's sysroot
+/// - External: use an external `lld` binary
+///
+/// It is configured depending on the target:
+/// 1) Everything except MSVC
+/// - Self-contained: `-Clinker-flavor=gnu-lld-cc -Clink-self-contained=+linker`
+/// - External: `-Clinker-flavor=gnu-lld-cc`
+/// 2) MSVC
+/// - Self-contained: `-Clinker=<path to rust-lld>`
+/// - External: `-Clinker=lld`
+#[derive(Default, Copy, Clone)]
+pub enum LldMode {
+    /// Do not use LLD
+    #[default]
+    Unused,
+    /// Use `rust-lld` from the compiler's sysroot
+    SelfContained,
+    /// Use an externally provided `lld` binary.
+    /// Note that the linker name cannot be overridden, the binary has to be named `lld` and it has
+    /// to be in $PATH.
+    External,
+}
+
+impl LldMode {
+    pub fn is_used(&self) -> bool {
+        match self {
+            LldMode::SelfContained | LldMode::External => true,
+            LldMode::Unused => false,
+        }
+    }
+}
+
 /// Global configuration for the entire build and/or bootstrap.
 ///
 /// This structure is parsed from `config.toml`, and some of the fields are inferred from `git` or build-time parameters.
@@ -199,7 +232,7 @@ pub struct Config {
     pub llvm_from_ci: bool,
     pub llvm_build_config: HashMap<String, String>,
 
-    pub use_lld: bool,
+    pub lld_mode: LldMode,
     pub lld_enabled: bool,
     pub llvm_tools_enabled: bool,
 
@@ -489,6 +522,10 @@ impl TargetSelection {
     pub fn is_synthetic(&self) -> bool {
         self.synthetic
     }
+
+    pub fn is_msvc(&self) -> bool {
+        self.contains("msvc")
+    }
 }
 
 impl fmt::Display for TargetSelection {
@@ -977,6 +1014,44 @@ enum StringOrInt<'a> {
     String(&'a str),
     Int(i64),
 }
+
+impl<'de> Deserialize<'de> for LldMode {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        struct LldModeVisitor;
+
+        impl<'de> serde::de::Visitor<'de> for LldModeVisitor {
+            type Value = LldMode;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+                formatter.write_str("one of true, 'self-contained' or 'external'")
+            }
+
+            fn visit_bool<E>(self, v: bool) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                Ok(if v { LldMode::External } else { LldMode::Unused })
+            }
+
+            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                match v {
+                    "external" => Ok(LldMode::External),
+                    "self-contained" => Ok(LldMode::SelfContained),
+                    _ => Err(E::custom("unknown mode {v}")),
+                }
+            }
+        }
+
+        deserializer.deserialize_any(LldModeVisitor)
+    }
+}
+
 define_config! {
     /// TOML representation of how the Rust build is configured.
     struct Rust {
@@ -1014,7 +1089,7 @@ define_config! {
         save_toolstates: Option<String> = "save-toolstates",
         codegen_backends: Option<Vec<String>> = "codegen-backends",
         lld: Option<bool> = "lld",
-        use_lld: Option<bool> = "use-lld",
+        lld_mode: Option<LldMode> = "use-lld",
         llvm_tools: Option<bool> = "llvm-tools",
         deny_warnings: Option<bool> = "deny-warnings",
         backtrace_on_ice: Option<bool> = "backtrace-on-ice",
@@ -1442,8 +1517,18 @@ impl Config {
             if let Some(true) = rust.incremental {
                 config.incremental = true;
             }
-            set(&mut config.use_lld, rust.use_lld);
+            set(&mut config.lld_mode, rust.lld_mode);
             set(&mut config.lld_enabled, rust.lld);
+
+            if matches!(config.lld_mode, LldMode::SelfContained)
+                && !config.lld_enabled
+                && flags.stage.unwrap_or(0) > 0
+            {
+                panic!(
+                    "Trying to use self-contained lld as a linker, but LLD is not being added to the sysroot. Enable it with rust.lld = true."
+                );
+            }
+
             set(&mut config.llvm_tools_enabled, rust.llvm_tools);
             config.rustc_parallel = rust
                 .parallel_compiler