Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS: Crate symbols get discarded when crate appears unused #133491

Open
nvzqz opened this issue Nov 26, 2024 · 10 comments · May be fixed by #133832
Open

macOS: Crate symbols get discarded when crate appears unused #133491

nvzqz opened this issue Nov 26, 2024 · 10 comments · May be fixed by #133832
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nvzqz
Copy link
Contributor

nvzqz commented Nov 26, 2024

I tried this code:

Within the base crate, the #[divan::bench] macro is used for benchmarking crate internals. Its generated code translates to something similar to:

#[used]
#[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func,mod_init_funcs")]
#[cfg_attr(target_os = "linux", link_section = ".init_array")]
#[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
static CONSTRUCTOR: extern "C" fn() = constructor;

extern "C" fn constructor() {
    // Register benchmark...
}

This crate has a benchmark executable with only:

fn main() {
    divan::main();
}

I expected to see this happen: run the add benchmark and output the results. This runs correctly on Linux and Windows, but not macOS.

Instead, this happened: divan::main() cannot find the benchmark because the pre-main constructor function never ran.

In order to make the benchmarks visible to the executable, the base crate needs to have one of its items appear to be used, such as black_box-ing any item from the crate.

Note that this also happens with code like:

#[used(linker)]
#[link_section = "__DATA,__my_values,regular,no_dead_strip"]
static VALUE: u32 = 42;

This issue occurs even when doing extern crate my_crate or use my_crate as _, which is a known workaround for similar issues.

Meta

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: aarch64-apple-darwin
release: 1.82.0
LLVM version: 19.1.1
@nvzqz nvzqz added the C-bug Category: This is a bug. label Nov 26, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 26, 2024
@workingjubilee workingjubilee added A-linkage Area: linking into static, shared libraries and binaries O-macos Operating system: macOS O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 26, 2024
@workingjubilee
Copy link
Member

hey @madsmtm What do you think we have wrought upon ourselves today?

@bjorn3
Copy link
Member

bjorn3 commented Nov 26, 2024

Fair chance this is because of linkers only pulling in object files from archives when a symbol in them is referenced unless --whole-archive is used. In principle we should be generating a symbols.o to ensure that all symbols are actually referenced, but maybe something is going wrong with that? Can you try adding #[no_mangle] to CONSTRUCTOR and/or constructor just to see if that makes any difference?

@workingjubilee
Copy link
Member

I'd be surprised if no_mangle worked but used(linker) didn't?

@nvzqz
Copy link
Contributor Author

nvzqz commented Nov 26, 2024

It appears that #[no_mangle] does not fix this.

@bjorn3
Copy link
Member

bjorn3 commented Nov 26, 2024

Does -Clink-dead-code=yes work? That disables --gc-sections (as well as a couple of other things)

@nvzqz
Copy link
Contributor Author

nvzqz commented Nov 26, 2024

RUSTFLAGS='-Clink-dead-code=yes' cargo bench also does not make the benchmark appear. Nor does it make the symbol in the __DATA,__my_values,regular,no_dead_strip section appear in the final binary.

@madsmtm
Copy link
Contributor

madsmtm commented Nov 26, 2024

I think the issue is that the symbols.o trick introduced in #95604 just doesn't works at all with ld64.

If I use RUSTFLAGS="-Clinker=rust-lld" cargo bench, the code works as expected.

@madsmtm
Copy link
Contributor

madsmtm commented Nov 26, 2024

And RUSTFLAGS=-Clink-arg=-all_load cargo bench does also work (which shows that the issue is not in the rlib itself).

I'll see if I can figure out a way to make the symbols.o trick work.

@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 26, 2024
@madsmtm
Copy link
Contributor

madsmtm commented Nov 27, 2024

I've run out of time for now, but might continue work on this in perhaps a few days, perhaps weeks. Noting down my findings in the meantime:

This can be reproduced with just a library crate foo.rs that contains a __mod_init_func,mod_init_funcs static, and a binary crate that contains extern crate foo;.

Running rustc dep.rs && rustc main.rs --extern=foo=libfoo.rlib && ./main, we can see that the ctor/initializer/constructor isn't being run. Importing and using the static in main.rs makes things work.

// dep.rs
#![crate_type = "rlib"]
#![no_std]

extern "C" {
    fn printf(format: *const core::ffi::c_char, ...) -> i32;
}

#[used]
#[link_section = "__DATA,__mod_init_func,mod_init_funcs"]
#[no_mangle]
pub static INIT: extern "C" fn() = init;

pub extern "C" fn init() {
    unsafe { printf(b"inside initializer\n\0".as_ptr().cast()) };
}

#[no_mangle]
pub fn foo() {}
// main.rs
#![no_std]
#![feature(start)]
extern crate dep;

#[link(name = "System")]
extern "C" {}

extern "Rust" {
    fn foo();
}

#[panic_handler]
fn handle(_: &core::panic::PanicInfo<'_>) -> ! {
    loop {}
}

extern "C" {
    fn printf(format: *const core::ffi::c_char, ...) -> i32;
}

#[start]
fn main(a: isize, l: *const *const u8) -> isize {
    // dep::foo();
    // foo();
    unsafe { printf(b"in main\n\0".as_ptr().cast()) };
    return 0;
}

Adding -Csave_temps=true -Clink-arg=-why_load --print link-args allows inspecting the generated symbols.o. objdump -t path/to/symbols.o shows the desired symbols, but doing ld path/to/symbols.o doesn't fail on these undefined symbols (it just fails on a missing _main and dyld_stub_binder).

I've managed to produce a helper.o file which does work in the intended way if I add -Clink-arg=helper.o. Generated with rustc helper.rs -Cpanic=abort -Cdebuginfo=0 --emit obj. ld helper.o also fails as expected.

// helper.rs
#![crate_type = "lib"]
#![no_std]

extern "Rust" {
    fn foo();
}

pub fn use_foo() {
    unsafe { foo() };
}

The difference between that and symbols.o does not seem that big, so now I just need to figure out exactly what the differences are, and then change the generation of symbols.o.

Hypotheses:

  • We need to emit different info in our Mach-O load commands?
  • We need to emit actual sections?
  • There needs to be at least one non-undefined symbol?
  • The symbols need to actually have relocations?
  • Maybe something else is needed for the symbol to be considered an "atom" in ld64 terminology?

@madsmtm madsmtm linked a pull request Dec 4, 2024 that will close this issue
3 tasks
@madsmtm
Copy link
Contributor

madsmtm commented Dec 4, 2024

I managed to (basically) re-create the helper.o with the object crate, and then it was just a question of trimming it down to see that it is indeed the relocations that matter - ld64 (neither -ld_classic nor -ld_prime) doesn't care about an undefined symbol unless it has a relocation.

I have put up #133832, with that PR, simply doing cargo +stage1 bench works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants