Only compile #[used] as llvm.compiler.used for ELF targets

[thomcc]

This returns #[used] to how it worked prior to the LLVM 13 update. The intention is not that this is a stable promise.

I'll add tests later today. The tests will test things that we don't actually promise, though.

It's a deliberately small patch, mostly comments. And assuming it's reviewed and lands in time, IMO it should at least be considered for uplifting to beta (so that it can be in 1.59), as the change broke many crates in the ecosystem, even if they are relying on behavior that is not guaranteed.

Background

LLVM has two ways of preventing removal of an unused variable: llvm.compiler.used, which must be present in object files, but allows the linker to remove the value, and llvm.used which is supposed to apply to the linker as well, if possible.

Prior to LLVM 13, llvm.used and llvm.compiler.used were the same on ELF targets, although they were different elsewhere. Prior to our update to LLVM 13, we compiled #[used] using llvm.used unconditionally, even though we only ever promised behavior like llvm.compiler.used.

In LLVM 13, ELF targets gained some support for preventing linker removal of llvm.used via the SHF_RETAIN section flag. This has some compatibility issues though: Concretely: some older versions ld.gold (specifically ones prior to v2.36, released in Jan 2021) had a bug where it would fail to place a #[used] #[link_section = ".init_array"] static in between __init_array_start/__init_array_end, leading to code that does this failing to run a static constructor. This is technically not a thing we guarantee will work, is a common use case, and is needed in libstd (for example, to get access to std::env::args() even if Rust does not control main, such as when in a cdylib crate).

As a result, when updating to LLVM 13, we unconditionally switched to using llvm.compiler.used, which mirror the guarantees we make for #[used] and doesn't require the latest ld.gold. Unfortunately, this happened to break quite a bit of things in the ecosystem, as non-ELF targets had come to rely on #[used] being slightly stronger. In particular, there are cases where it will even break static constructors on these targets¹ (and in fact, breaks way more use cases, as Mach-O uses special sections as an interface to the OS/linker/loader in many places).

As a result, we only switch to llvm.compiler.used on ELF² targets. The rationale here is:

1. It is (hopefully) identical to the semantics we used prior to the LLVM13 update as prior to that update we unconditionally used llvm.used, but on ELF llvm.used was the same as llvm.compiler.used.

2. It seems to be how Clang compiles this, and given that they have similar (but stronger) compatibility promises, that makes sense.

This is pretty ad-hoc and hacky! We probably should have a firmer set of guarantees here, but this change should relax the pressure on coming up with that considerably, returning it to previous levels.

---

Unsure who should review so leaving it open, but for sure CC @nikic

Footnotes

1. For Mach-O targets: It is not always guaranteed that __DATA,__mod_init_func is a GC root if it does not have the S_MOD_INIT_FUNC_POINTERS flag which we cannot add. In most cases, when ld64 transformed this section into __DATA_CONST,__mod_init_func it gets applied, but it's not clear that that is intentional (let alone guaranteed), and the logic is complex enough that it probably happens sometimes, and people in the wild report it occurring. ↩

2. Actually, there's not a great way to tell if it's ELF, so I've approximated it. ↩

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[thomcc]

See the discussion in #47384 starting around #47384 (comment) for more info.

Generally that bug is a unrelated problem, but it's kind of evolved into "traking issue for cases where #[used] is flakey" (and it got way flakier in LLVM 13).

[thomcc]

Another case that's probably broken without this is

rust/compiler/rustc/src/main.rs

Lines 48 to 61 in f624427

	// On OSX, jemalloc doesn't directly override malloc/free, but instead
	// registers itself with the allocator's zone APIs in a ctor. However,
	// the linker doesn't seem to consider ctors as "used" when statically
	// linking, so we need to explicitly depend on the function.
	#[cfg(target_os = "macos")]
	{
	extern "C" {
	fn _rjem_je_zone_register();
	}
	#[used]
	static _F7: unsafe extern "C" fn() = _rjem_je_zone_register;
	}
	}

, so this might improve performance too, unless something else is keeping it around (or if we don't use tikv-jemalloc-sys on macOS).

The comment also helps back up my assertion that static ctors aren't guaranteed to be GC roots on Darwin, even though simple cases (single object executables/dylibs, etc) generally will tend to work.

[thomcc]

I'm not sure this is a good way to test this (although it does work as a test in that it fails without my patch, and passes with it). But I gather run-make tests are a last resort, and I've done this mostly because IDK how to use the right tool 😅.

Another option would be to check that some static symbols end up in @llvm.used not @llvm.compiler.used when building with --emit=llvm-ir. This which could be run on more targets, and is more obviously an implementation detail (since obviously the particulars of the IR we emit).

I've manually verified that works, and can see how to add it to a run-make test like this one, and maybe that would be better... but it feels like a big hack to do it that way, and really it should use filecheck... But at the same time, it's not obvious to me how to do it in filecheck, since the place the @llvm.used/@llvm.compiler.used seems inconsistent. (But... probably I'm just doing it wrong).

Let me know what you'd prefer, or if this is fine.

[joshtriplett]

This seems like a reasonable regression fix, based on the discussions from today's @rust-lang/lang meeting.

👍 for the semantics of this. The implementation seems reasonable, but I'll leave that to someone on @rust-lang/compiler to r+.

[estebank]

r? rust-lang/t-compiler

[pnkfelix]

cc #47384

@rustbot claim

[nikic]

As there are separate USED and USED_LINKER flags now, this should be handled at a higher level (where used is defaulted to either used(compiler) or used(linker)).

[thomcc]

Damn. I won’t have time to look into this for a week or two, sadly

[pnkfelix]

okay, based on this dialogue, I'm going to revise the state of this PR to S-waiting-on-author

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

[thomcc]

I moved this but I don't think this is a good place for it. It seems strange to have these checks in rustc_typeck, since I'd expect it not to care so much about target details like this.

That said, I'm not familiar enough with the compiler to say for sure.

[thomcc]

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

[wesleywiser]

r? rust-lang/compiler

[pnkfelix]

If I'm understanding all this correctly, it sounds like CodegenFnAttrFlags::USED corresponds to llvm.compiler.used, while CodegenFnAttrFlags::USED_LINKER corresponds to llvm.used.

If the above is true, then I personally would prefer USED to have a more specific name (e.g. USED_COMPILER.

But having said that, I don't want to delay this PR even longer than it already has been delayed based on a cleanup task like that. Its just a passing comment on how I found this code a little confusing as written.

[pnkfelix]

@bors r+

[bors]

📌 Commit a64b2a9 has been approved by pnkfelix

It is now in the queue for this repository.

[bors]

⌛ Testing commit a64b2a9 with merge ceeb5ad...

[bors]

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing ceeb5ad to master...

[rust-timer]

Finished benchmarking commit (ceeb5ad): comparison url.

Instruction count

• Primary benchmarks: 😿 relevant regressions found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	0.4%	0.4%	2
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.4%	0.4%	2

Max RSS (memory usage)

Results

• Primary benchmarks: 😿 relevant regression found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	3.2%	3.2%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	3.2%	3.2%	1

Cycles

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

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

1. the arithmetic mean of the percent change ↩ ↩²

2. number of relevant changes ↩ ↩²