Warn when gold was used as the linker

[Noratrieb]

gold has been deprecated recently and is known to behave incorrectly around Rust programs, including miscompiling #[used(linker)]. Tell people to switch to a different linker instead.

closes #141748

r? bjorn3

[rustbot]

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

[Noratrieb]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 7c8e56f with merge a1b2ee8...

[bors]

☀️ Try build successful - checks-actions
Build commit: a1b2ee8 (a1b2ee8ad9b4320eb93b75718abce55aefc79a6b)

[rust-timer]

Finished benchmarking commit (a1b2ee8): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -0.9%, secondary -1.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.3%	[0.5%, 2.5%]	3
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-1.6%	[-8.0%, -0.5%]	15
All ❌✅ (primary)	-0.9%	[-0.9%, -0.9%]	1

Cycles

Results (secondary 0.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.0%	[0.4%, 3.0%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.7%, -0.4%]	5
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 780.508s -> 779.064s (-0.19%)
Artifact size: 368.46 MiB -> 368.53 MiB (0.02%)

[bjorn3]

This seems to consistently regress linking time by about 1ms.

[Noratrieb]

I've thought that this sort of detection may also be useful in the future to emit a warning if #[used] was used with a version of bfd that doesn't support it.

[Noratrieb]

@bors2 try @rust-timer queue

[rust-bors]

⌛ Trying commit 70950cc with merge 389e842…

[rust-bors]

☀️ Try build successful

• CI ✅
  Build commit: 389e842 (389e842a17838b2376420ea608ff25de9e819e1a)

[rust-timer]

Finished benchmarking commit (389e842): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.4%	[0.4%, 0.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.9%	[0.4%, 3.0%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-2.6%, -0.4%]	3
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -0.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.9%	[0.5%, 1.6%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.7%	[-1.4%, -0.4%]	9
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: missing data
Artifact size: 370.19 MiB -> 370.19 MiB (0.00%)

[Noratrieb]

👍 on your commit, would be nice to squash before merge

[bjorn3]

@bors r+

[bors]

📌 Commit ba5a744 has been approved by bjorn3

It is now in the queue for this repository.

[Noratrieb]

@bors r-
I've thought about this again and investigated what the gold issue was exactly in the first place. It looks like the problem was that gold was essentially deleting .init_array entries that it saw if they had SHF_GNU_RETAIN (despite those being considered a GC root by the linker, so previously without SHF_GNU_RETAIN it worked fine). This was fixed in binutils 2.36 (incidentally the same version that also added SHF_GNU_RETAIN support to bfd ld in the first place), after which gold supports SHF_GNU_RETAIN just fine.
So I think this blanket warning doesn't really make sense, I think we should only warn on old gold versions. Binutils <2.36 is still around (for example on Debian 11), so a warning may still make sense, but I don't think the warning is really useful for most users. It is true that it's deprecated, but I don't think rustc should complain to users that they're using a deprecated linker, I think rustc should only protect them from miscompilations.

[bjorn3]

If rustc doesn't warn about newer gold versions anymore, I think the sanity check in bootstrap should instead because of #139425.

[Noratrieb]

We could be even more clever to detect whether #[used(linker)] was used anywhere in the crate graph, but I feel like that might be a bit too much complexity, as it would require encoding new metadata.

[Noratrieb]

Looks like the version embedded in the binary is the gold version, not the binutils version. And the way that is versioned, is uh

GNU gold (GNU Binutils for Debian 2.35.2) 1.16

GNU gold (GNU Binutils 2.44) 1.16

bad news. the gold version is very bad news.

[Noratrieb]

so I think just warning for every version is fine. it's deprecated (and apparently literally no longer included in the latest binutils source tarball), so we can't really have a "false positive" in that sense. no one should be using it.

[bjorn3]

Given the above, let's approve it again :)

@bors r+

[bors]

📌 Commit ba5a744 has been approved by bjorn3

It is now in the queue for this repository.