Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) #106489

jschwe · 2023-01-05T13:54:32Z

Linker (drivers) such as clang / gcc or lld often have a version postfix matching the regex "-\d+$".
Previously, linker detection did not account for the possible version postfix and the fallback value was used, which can cause linker errors due to wrong arguments.
Also remove the check for -clang, since there are no architecture specific variants of clang (to my knowledge).

Fixes #106454

rustbot · 2023-01-05T13:54:42Z

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

jackh726 · 2023-01-05T15:00:10Z

r? compiler

compiler/rustc_codegen_ssa/src/back/link.rs

oli-obk · 2023-01-09T10:53:05Z

@bors r+ rollup

bors · 2023-01-09T10:53:07Z

📌 Commit 41672bf5e5e2946e684ec5b0639863b0f789621f has been approved by oli-obk

It is now in the queue for this repository.

oli-obk · 2023-01-09T10:57:38Z

@bors r-

oli-obk · 2023-01-09T15:33:05Z

r? @petrochenkov

jschwe · 2023-01-11T10:36:15Z

Would it make sense to add detection for g++ and clang++ (which should be treated the same as gcc/clang) in this PR, or should that be a seperate PR?

petrochenkov · 2023-01-11T22:12:42Z

@jschwe

Would it make sense to add detection for g++ and clang++ (which should be treated the same as gcc/clang) in this PR, or should that be a seperate PR?

Yeah, these can also be added.
This stuff is not strictly necessary because -C linker-flavor can be added explicitly if it's not inferred correctly, so not everything is added, but adding obvious cases like g++ or clang++ should be fine.

compiler/rustc_codegen_ssa/src/back/link.rs

jschwe · 2023-01-12T12:47:04Z

This stuff is not strictly necessary because -C linker-flavor can be added explicitly if it's not inferred correctly

This requires using either RUSTFLAGS (or one of the other ways to modify the rustflags for the crate and ALL dependencies) or cargo rustc right? I.e. there is no way to set -C linker-flavor for cargo test without also setting the flag for all dependencies (where it might not be wanted).

jschwe · 2023-01-12T14:20:41Z

I pushed a commit which strips the possible version postfix, but without using a regex this time. This makes the matching a bit nicer.

petrochenkov · 2023-01-13T16:08:55Z

Ok, let's just land this as is.
r=me after squashing commits.
@rustbot author

Linker drivers such as gcc, clang or lld often have a version postfix, e.g clang-12. The previous logic would not account for this and would fall back to guessing the linker flavor to be the default linker flavor for the target, which causes linker errors when this is not the case. By accounting for the possible version postfix and also considering g++ and clang++, we considerably reduce the amount of times the fallback guess has to be used. To simplify matching check for a version postfix and match against the linker stem without any version postfix. In contrast to gcc, clang supports all architectures in one binary. This means there are no variants like `aarch64-linux-gnu-clang` and there is no need to check for `-clang` variants.

jschwe · 2023-01-13T16:37:31Z

Rebased, squashed and adjusted the commit message.

@rustbot label -S-waiting-on-author +S-waiting-on-review

petrochenkov · 2023-01-13T16:55:56Z

@bors r+ rollup

bors · 2023-01-13T16:55:57Z

📌 Commit 3bc2970 has been approved by petrochenkov

It is now in the queue for this repository.

…rochenkov Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) Linker (drivers) such as clang / gcc or lld often have a version postfix matching the regex "-\d+$". Previously, linker detection did not account for the possible version postfix and the fallback value was used, which can cause linker errors due to wrong arguments. Also remove the check for `-clang`, since there are no architecture specific variants of clang (to my knowledge). Fixes rust-lang#106454

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#104645 (Add log-backtrace option to show backtraces along with logging) - rust-lang#106465 (Bump `IMPLIED_BOUNDS_ENTAILMENT` to Deny + ReportNow) - rust-lang#106489 (Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang)) - rust-lang#106585 (When suggesting writing a fully qualified path probe for appropriate types) - rust-lang#106641 (Provide help on closures capturing self causing borrow checker errors) - rust-lang#106678 (Warn when using panic-strategy abort for proc-macro crates) - rust-lang#106701 (Fix `mpsc::SyncSender` spinning behavior) - rust-lang#106793 (Normalize test output more thoroughly) - rust-lang#106797 (riscv: Fix ELF header flags) - rust-lang#106813 (Remove redundant session field) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Rust 1.68 (rust-lang/rust#106489) broke this: ``` = note: clang-15: error: unsupported option '--export' clang-15: error: unsupported option '--stack-first' clang-15: error: unsupported option '--allow-undefined' clang-15: error: unsupported option '--fatal-warnings' clang-15: error: unsupported option '--no-demangle' clang-15: error: unsupported option '--gc-sections' clang-15: error: unknown argument: '-flavor' clang-15: error: no such file or directory: 'wasm' clang-15: error: no such file or directory: '__main_void' ```

…henkov Fix linker detection for clang with prefix rust-lang#106489 removed check for clang with prefix. It says: > Also remove the check for -clang, since there are no architecture specific variants of clang (to my knowledge). However, when doing cross-compilation, a wrapper script for clang with the target name as a prefix is sometimes used. https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L62 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/scripts/freebsd-toolchain.sh#L76-L80 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L40 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/compiler/rustc_target/src/spec/aarch64_pc_windows_gnullvm.rs#L7 It seems the regression did not occur on the targets mentioned above because the default linker flavor is gcc, but it did occur on targets where the default linker flavor is not gcc (taiki-e/setup-cross-toolchain-action@fd352f3). r? `@petrochenkov`

…henkov Fix linker detection for clang with prefix rust-lang#106489 removed check for clang with prefix. It says: > Also remove the check for -clang, since there are no architecture specific variants of clang (to my knowledge). However, when doing cross-compilation, a wrapper script for clang with the target name as a prefix is sometimes used. https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L62 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/scripts/freebsd-toolchain.sh#L76-L80 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L40 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/compiler/rustc_target/src/spec/aarch64_pc_windows_gnullvm.rs#L7 It seems the regression did not occur on the targets mentioned above because the default linker flavor is gcc, but it did occur on targets where the default linker flavor is not gcc (taiki-e/setup-cross-toolchain-action@fd352f3). r? ``@petrochenkov``

…henkov Fix linker detection for clang with prefix rust-lang#106489 removed check for clang with prefix. It says: > Also remove the check for -clang, since there are no architecture specific variants of clang (to my knowledge). However, when doing cross-compilation, a wrapper script for clang with the target name as a prefix is sometimes used. https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L62 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/scripts/freebsd-toolchain.sh#L76-L80 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L40 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/compiler/rustc_target/src/spec/aarch64_pc_windows_gnullvm.rs#L7 It seems the regression did not occur on the targets mentioned above because the default linker flavor is gcc, but it did occur on targets where the default linker flavor is not gcc (taiki-e/setup-cross-toolchain-action@fd352f3). r? ```@petrochenkov```

…henkov Fix linker detection for clang with prefix rust-lang#106489 removed check for clang with prefix. It says: > Also remove the check for -clang, since there are no architecture specific variants of clang (to my knowledge). However, when doing cross-compilation, a wrapper script for clang with the target name as a prefix is sometimes used. https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L62 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/scripts/freebsd-toolchain.sh#L76-L80 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/src/ci/docker/host-x86_64/dist-various-2/Dockerfile#L40 https://github.com/rust-lang/rust/blob/1716932743a7b3705cbf0c34db0c4e070ed1930d/compiler/rustc_target/src/spec/aarch64_pc_windows_gnullvm.rs#L7 It seems the regression did not occur on the targets mentioned above because the default linker flavor is gcc, but it did occur on targets where the default linker flavor is not gcc (taiki-e/setup-cross-toolchain-action@fd352f3). r? ````@petrochenkov````

rustbot assigned jackh726 Jan 5, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 5, 2023

rustbot assigned oli-obk and unassigned jackh726 Jan 5, 2023

oli-obk reviewed Jan 9, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/back/link.rs Outdated Show resolved Hide resolved

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2023

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 9, 2023

jschwe force-pushed the fix_linker_detection branch from 5d8b2e9 to 3dc41f1 Compare January 9, 2023 15:30

rustbot assigned petrochenkov and unassigned oli-obk Jan 9, 2023

This comment was marked as resolved.

Sign in to view

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2023

jschwe force-pushed the fix_linker_detection branch from 3dc41f1 to 08f5108 Compare January 11, 2023 09:13

petrochenkov reviewed Jan 11, 2023

View reviewed changes

compiler/rustc_codegen_ssa/src/back/link.rs Outdated Show resolved Hide resolved

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2023

petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 12, 2023

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 12, 2023

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2023

jschwe force-pushed the fix_linker_detection branch from 3a8d527 to 3bc2970 Compare January 13, 2023 16:29

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 13, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2023

matthiaskrgr mentioned this pull request Jan 13, 2023

Rollup of 11 pull requests #106818

Closed

matthiaskrgr mentioned this pull request Jan 13, 2023

Rollup of 10 pull requests #106822

Merged

bors merged commit f9dde54 into rust-lang:master Jan 14, 2023

rustbot added this to the 1.68.0 milestone Jan 14, 2023

jschwe deleted the fix_linker_detection branch January 18, 2023 13:34

taiki-e mentioned this pull request Mar 15, 2023

Fix linker detection for clang with prefix #109156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) #106489

Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) #106489

jschwe commented Jan 5, 2023

rustbot commented Jan 5, 2023

jackh726 commented Jan 5, 2023

oli-obk commented Jan 9, 2023

bors commented Jan 9, 2023

oli-obk commented Jan 9, 2023

oli-obk commented Jan 9, 2023

This comment was marked as resolved.

jschwe commented Jan 11, 2023

petrochenkov commented Jan 11, 2023

jschwe commented Jan 12, 2023

jschwe commented Jan 12, 2023

petrochenkov commented Jan 13, 2023

jschwe commented Jan 13, 2023

petrochenkov commented Jan 13, 2023

bors commented Jan 13, 2023

Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) #106489

Fix linker detection for linker (drivers) with a version postfix (e.g. clang-12 instead of clang) #106489

Conversation

jschwe commented Jan 5, 2023

rustbot commented Jan 5, 2023

jackh726 commented Jan 5, 2023

oli-obk commented Jan 9, 2023

bors commented Jan 9, 2023

oli-obk commented Jan 9, 2023

oli-obk commented Jan 9, 2023

This comment was marked as resolved.

jschwe commented Jan 11, 2023

petrochenkov commented Jan 11, 2023

jschwe commented Jan 12, 2023

jschwe commented Jan 12, 2023

petrochenkov commented Jan 13, 2023

jschwe commented Jan 13, 2023

petrochenkov commented Jan 13, 2023

bors commented Jan 13, 2023