about summary refs log tree commit diff
path: root/compiler/rustc_codegen_ssa/src
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-07-13 22:33:25 +0200
committerGitHub <noreply@github.com>2023-07-13 22:33:25 +0200
commitfc1cb0459dfba992814566ffcccda48e50d09402 (patch)
treeafa4e6bb2f8065a5299752f5c295cad4b1ed9eff /compiler/rustc_codegen_ssa/src
parent017112f834c8ffea837a78dc7c994ab4326dbbf9 (diff)
parent2b61a5e17a6bcb552889966f8f932aa680826ab6 (diff)
downloadrust-fc1cb0459dfba992814566ffcccda48e50d09402.tar.gz
rust-fc1cb0459dfba992814566ffcccda48e50d09402.zip
Rollup merge of #113631 - lqd:fix-113597, r=petrochenkov
make MCP510 behavior opt-in to avoid conflicts between the CLI and target flavors

Fixes #113597, which contains more details on how this happens through the code, and showcases an unexpected `Gnu(Cc::Yes, Lld::Yes)` flavor.

#112910 added support to use `lld` when the flavor requests it, but didn't explicitly do so only when using `-Clink-self-contained=+linker` or one of the unstable `-Clinker-flavor`s.

The problem: some targets have a `lld` linker and flavor, e.g. `thumbv6m-none-eabi` from that issue. Users can override the linker but there are no linker flavors precise enough to describe the linker opting out of lld: when using `-Clinker=arm-none-eabi-gcc`, we infer this is a `Cc::Yes` linker flavor, but the `lld` component is unknown and therefore defaulted to the target's linker flavor, `Lld::Yes`.

<details>
<summary>Walkthrough of how this happens</summary>

The linker flavor used is a mix between what can be inferred from the CLI (`-C linker`) and the target's default linker flavor:

- there is no linker flavor on the CLI (and that also offers another workaround on nightly: `-C linker-flavor=gnu-cc -Zunstable-options`), so it will have to be inferred [from here](https://github.com/lqd/rust/blob/5dac6b320be868f898a3c753934eabc79ff2e406/compiler/rustc_codegen_ssa/src/back/link.rs#L1334-L1336) to [here](https://github.com/lqd/rust/blob/5dac6b320be868f898a3c753934eabc79ff2e406/compiler/rustc_codegen_ssa/src/back/link.rs#L1321-L1327).
- in [`infer_linker_hints`](https://github.com/lqd/rust/blob/5dac6b320be868f898a3c753934eabc79ff2e406/compiler/rustc_target/src/spec/mod.rs#L320-L352) `-C linker=arm-none-eabi-gcc` infers a `Some(Cc::Yes)` cc hint, and no hint about lld.
- the target's `linker_flavor` is combined in `with_cli_hints` with these hints. We have our `Cc::Yes`, but there is no hint about lld, [so the target's flavor `lld` component is used](https://github.com/lqd/rust/blob/5dac6b320be868f898a3c753934eabc79ff2e406/compiler/rustc_target/src/spec/mod.rs#L356-L358). It's [`Gnu(Cc::No, Lld::Yes)`](https://github.com/rust-lang/rust/blob/993deaa0bf8bab9dd3eadfd1fbeb093328e95afe/compiler/rustc_target/src/spec/thumb_base.rs#L35).
- so we now have our `Gnu(Cc::Yes, Lld::Yes)` flavor

</details>

This results in a `Gnu(Cc::Yes, Lld::Yes)` flavor on a non-lld linker, causing an additional unexpected `-fuse-ld=lld` argument to be passed.

I don't know if this target defaulting to `rust-lld` is expected, but until MCP510's new linker flavor are stable, when people will be able to describe their linker/flavor accurately, this PR keeps the stable behavior of not doing anything when the linker/flavor on the CLI unexpectedly conflict with the target's.

I've tested this on a `no_std` `-C linker=arm-none-eabi-gcc -C link-arg=-nostartfiles --target thumbv6m-none-eabi` example, trying to simulate one of `cortex-m`'s test mentioned in issue #113597 (I don't know how to build a local complete  `thumbv6m-none-eabi` toolchain to run the exact test), and checked that `-fuse-lld` was indeed gone and the error disappeared.

r? `````@petrochenkov`````
Diffstat (limited to 'compiler/rustc_codegen_ssa/src')
-rw-r--r--compiler/rustc_codegen_ssa/src/back/link.rs19
1 files changed, 17 insertions, 2 deletions
diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index c7925be0d7d..b603a878746 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -2970,10 +2970,25 @@ fn add_lld_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
         return;
     }
 
+    let self_contained_linker = sess.opts.cg.link_self_contained.linker();
+
+    // FIXME: some targets default to using `lld`, but users can only override the linker on the CLI
+    // and cannot yet select the precise linker flavor to opt out of that. See for example issue
+    // #113597 for the `thumbv6m-none-eabi` target: a driver is used, and its default linker
+    // conflicts with the target's flavor, causing unexpected arguments being passed.
+    //
+    // Until the new `LinkerFlavor`-like CLI options are stabilized, we only adopt MCP510's behavior
+    // if its dedicated unstable CLI flags are used, to keep the current sub-optimal stable
+    // behavior.
+    let using_mcp510 =
+        self_contained_linker || sess.opts.cg.linker_flavor.is_some_and(|f| f.is_unstable());
+    if !using_mcp510 && !unstable_use_lld {
+        return;
+    }
+
     // 1. Implement the "self-contained" part of this feature by adding rustc distribution
     //    directories to the tool's search path.
-    let self_contained_linker = sess.opts.cg.link_self_contained.linker() || unstable_use_lld;
-    if self_contained_linker {
+    if self_contained_linker || unstable_use_lld {
         for path in sess.get_tools_search_paths(false) {
             cmd.arg({
                 let mut arg = OsString::from("-B");