Skip to content

Commit a4dc1b5

Browse files
authored
Rollup merge of rust-lang#113631 - lqd:fix-113597, r=petrochenkov
make MCP510 behavior opt-in to avoid conflicts between the CLI and target flavors Fixes rust-lang#113597, which contains more details on how this happens through the code, and showcases an unexpected `Gnu(Cc::Yes, Lld::Yes)` flavor. rust-lang#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 rust-lang#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```
2 parents 92f20fe + 2b61a5e commit a4dc1b5

File tree

1 file changed

+17
-2
lines changed
  • compiler/rustc_codegen_ssa/src/back

1 file changed

+17
-2
lines changed

compiler/rustc_codegen_ssa/src/back/link.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -2970,10 +2970,25 @@ fn add_lld_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
29702970
return;
29712971
}
29722972

2973+
let self_contained_linker = sess.opts.cg.link_self_contained.linker();
2974+
2975+
// FIXME: some targets default to using `lld`, but users can only override the linker on the CLI
2976+
// and cannot yet select the precise linker flavor to opt out of that. See for example issue
2977+
// #113597 for the `thumbv6m-none-eabi` target: a driver is used, and its default linker
2978+
// conflicts with the target's flavor, causing unexpected arguments being passed.
2979+
//
2980+
// Until the new `LinkerFlavor`-like CLI options are stabilized, we only adopt MCP510's behavior
2981+
// if its dedicated unstable CLI flags are used, to keep the current sub-optimal stable
2982+
// behavior.
2983+
let using_mcp510 =
2984+
self_contained_linker || sess.opts.cg.linker_flavor.is_some_and(|f| f.is_unstable());
2985+
if !using_mcp510 && !unstable_use_lld {
2986+
return;
2987+
}
2988+
29732989
// 1. Implement the "self-contained" part of this feature by adding rustc distribution
29742990
// directories to the tool's search path.
2975-
let self_contained_linker = sess.opts.cg.link_self_contained.linker() || unstable_use_lld;
2976-
if self_contained_linker {
2991+
if self_contained_linker || unstable_use_lld {
29772992
for path in sess.get_tools_search_paths(false) {
29782993
cmd.arg({
29792994
let mut arg = OsString::from("-B");

0 commit comments

Comments
 (0)