make tests/run-make/used-proc-macro more reliable#155436
make tests/run-make/used-proc-macro more reliable#155436japaric wants to merge 1 commit intorust-lang:mainfrom
Conversation
rustbot
added
A-run-make
|
r? @mejrs rustbot has assigned @mejrs. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
| #[used] | ||
| #[no_mangle] // so we can refer to this variable by name from the proc-macro library |
There was a problem hiding this comment.
Doesn't #[no_mangle] imply #[used]?
This is not my area of expertise but it looks like you can drop the #[used] attribute (and adjust the comments appropriately). Please correct me if I'm wrong.
There was a problem hiding this comment.
Yeah, #[used] would no longer have much of an effect now.
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
mejrs
added
S-waiting-on-author
| // read the symbol otherwise the _linker_ may discard it | ||
| // `#[used]` only preserves the symbol up to the compiler output (.o file) | ||
| // which is then passed to the linker. the linker is free to discard the | ||
| // `#[used]` symbol if it's not accessed/referred-to by other object | ||
| // files (crates) | ||
| let symbol_value = unsafe { core::ptr::addr_of!(VERY_IMPORTANT_SYMBOL).read_volatile() }; |
There was a problem hiding this comment.
#[used] is an alias for #[used(linker)] since #140872, which should prevent the linker from discarding it. In either case now that you added a reference to it in another crate, this test is no longer testing #[used] as it should. The linker error that you observed is a genuine bug that this test caught. This bug should be fixed instead of changing the test.
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
the test was relying on the linker not discarding the `#[used]` symbol this commit modifies the proc macro to read the `#[used]` symbol to ensure it is not discarded by the linker cherry-picked from upstream, see rust-lang/rust#155436
after looking further into this, I think whether the symbol ends in the final artifact may still be linker dependent. As I understand the underlying mechanism, the compiler creates a the linker is supposed to exhaustively scan the input files (linker arguments) until it finds those undefined symbols in on revision 27dbdb5
I haven't tested other GCC versions so I can't say for sure that this is just a bug on older GCC versions. However, manually discarding the rlib from the linker invocation to keep the linker from finding the "undefined" symbol does not produce a linker error with either LLD or GNU LD (LLD repro below) so leaving some symbols "undefined" appears to not be treated as a fatal error, by either linker, that warrants aborting the linking process. I tried a few linker flags that should turn "unresolved undefined" symbols into hard errors but only The only other alternative to Details$ cargo new --bin repro
$ cd repro
$ cargo new --lib dep
$ cat dep/src/lib.rs#![feature(used_with_arg)]
#![no_std]
#[used(linker)]
static VERY_IMPORTANT_SYMBOL: usize = 42;$ cat Cargo.toml[package]
name = "repro"
edition = "2024"
[dependencies]
dep.path = "dep"$ cat src/main.rs#![no_main]
#![no_std]
use dep as _;
#[unsafe(no_mangle)]
fn _start() {}
#[panic_handler]
fn panic(_: &core::panic::PanicInfo) -> ! {
loop {}
}NOTE: the compilation target is not really important but a $ cargo clean
$ cargo rustc --target aarch64-unknown-none -v -- --print link-args -C save-temps
$ nm -CSn target/aarch64-unknown-none/debug/deps/rust*/symbols.o
U __rustc::rust_begin_unwind
U __rustc::rust_begin_unwind
U dep::VERY_IMPORTANT_SYMBOL
U _start
$ nm -CSn target/aarch64-unknown-none/debug/repro | tail -n4
0000000000200158 0000000000000008 R dep::VERY_IMPORTANT_SYMBOL
0000000000200178 r $d
00000000002101a8 t $x
00000000002101a8 0000000000000004 T _startrepat the linker invocation printed by $ (..) "rust-lld" "-flavor" "gnu" (..) && echo OK
OK
$ nm -CSn target/aarch64-unknown-none/debug/deps/repro-* | tail -n4
000000000000017c N $d
0000000000200170 r $d
00000000002101a0 t $x
00000000002101a0 0000000000000004 T _startThe modified linker invocation can be repeated with GNU LD (e.g. |
|
The symbol is retained with recent linkers thanks to .section .rodata.example[caa99c3a58b05bc]::VERY_IMPORTANT_SYMBOL,"aR",@progbits
.globl example[caa99c3a58b05bc]::VERY_IMPORTANT_SYMBOLNote the uppercase This functionality appears to be added to IMO the right call here would be to ignore the test if linked with ld.bfd < 2.36. |
putting aside the technical detail of how to encode that condition in bootstrap or the run-make test itself, I think that if this language feature relies on the linker implementing certain features then that should be noted in the feature documentation. the guarantee of the feature is that the symbol makes it to the output object. additionally, a hint is provided to the linker to also preserve the symbol but that behavior is not guaranteed. it can also be noted that ELF linkers that support SHF_GNU_RETAIN are known to respect the emitted hint. after all, even if the default linker for a certain target is the |
|
The language doesn't guarantee that |
Fair point about
Hmm, I think we may have to agree to disagree. SHF_GNU_RETAIN is a "GNU extension" (says the GNU documentation), not part of the ELF specification like say SHF_ALLOC, so it being implemented by other linkers is not a given I'd think. Putting aside that the feature is unstable and that it's perfectly fine that no thorough documentation is yet in place; as the end user, if the If, instead, the documentation says that (a) the compiler provides the linker a hint and that the symbol surviving the linker is not guaranteed by the compiler, which does not have the final say on the linker selection -- the user has the final say; OR (b) that it's a requirement that an ELF linker that supports |
|
I agree that linker requirements should be documented, especially given Binutils 2.36 is old but not yet ancient. But then I think |
There are a ton of things that are officially GNU extensions of the ELF spec, yet are mandatory on Linux according to the LSB. Both major C++ runtimes on Linux depend on STB_GNU_UNIQUE, everyone uses DT_GNU_HASH for the dylib symbol table rather than the ELF spec mandated DT_HASH. Unwind tables are located using |
5 participants
the test was relying on the linker not discarding the
#[used]symbolthis commit modifies the proc macro to read the
#[used]symbol to ensure it is not discarded by the linkerfixes #155434