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 build is failing: SLICE.is_empty() #41

Closed
dtolnay opened this issue Aug 22, 2021 · 7 comments · Fixed by #46
Closed

macOS build is failing: SLICE.is_empty() #41

dtolnay opened this issue Aug 22, 2021 · 7 comments · Fixed by #46
Labels
help wanted Extra attention is needed

Comments

@dtolnay
Copy link
Owner

dtolnay commented Aug 22, 2021

running 2 tests
test declaration::test_slice ... FAILED
test declaration::test_functions ... FAILED

failures:

---- declaration::test_slice stdout ----
thread 'declaration::test_slice' panicked at 'assertion failed: !SLICE.is_empty()', tests/custom_linkme_path.rs:12:9

---- declaration::test_functions stdout ----
thread 'declaration::test_functions' panicked at 'assertion failed: !FUNCTIONS.is_empty()', tests/custom_linkme_path.rs:21:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    declaration::test_functions
    declaration::test_slice
@dtolnay
Copy link
Owner Author

dtolnay commented Aug 22, 2021

This bisects to rust-lang/rust#87570 so possibly the same issue as #40.

dtolnay added a commit that referenced this issue Aug 22, 2021
@dtolnay dtolnay added the help wanted Extra attention is needed label Sep 4, 2021
@mystor
Copy link

mystor commented Sep 25, 2021

It seems somewhat likely to me that this regression is somehow related to the change mentioned in rust-lang/rust#87570, specifically:

Switch #[used] attribute from llvm.used to llvm.compiler.used to avoid SHF_GNU_RETAIN flag introduced in https://reviews.llvm.org/D97448, which appears to trigger a bug in older versions of gold.

In the llvm docs the llvm.compiler.used attribute is noted as allowing the linker to dispose of the symbol, in contrast to llvm.used, which appears to be what is happening here: https://llvm.org/docs/LangRef.html#the-llvm-used-global-variable

Perhaps rustc should continue to use llvm.used, instead of llvm.compiler.used, and only fall back to that version if using one of the broken versions of gold?

@dtolnay
Copy link
Owner Author

dtolnay commented Sep 26, 2021

^ FYI @nikic @nagisa. Any thoughts on what the right move would be if llvm.compiler.used doesn't work for this use case?

@nikic
Copy link

nikic commented Sep 26, 2021

From the rustc side, we could do the same as clang and use llvm.compiler.used for ELF and llvm.used for MachO and COFF -- it's weirdly inconsistent, but I guess doing the same thing as clang has a consistency of its own ;)

Though from #40 it sounds like this issue also affects ELF targets? My understanding here was that #[used] on ELF targets never had an effect on the linker prior to LLVM 13 either, so I wouldn't have expected that case to be affected.

Ideally we'd expose both possibilities, e.g. #[used] for llvm.compiler.used and #[used(retain)] for llvm.used.

@mystor
Copy link

mystor commented Sep 26, 2021

Though from #40 it sounds like this issue also affects ELF targets? My understanding here was that #[used] on ELF targets never had an effect on the linker prior to LLVM 13 either, so I wouldn't have expected that case to be affected.

After a bit more investigation into this, I think this is related to the specific linker (disclaimer: I know very little about linkers, and am probably wrong about this). Specifically, it appears that lld recently changed whether they consider a __start_Section or __stop_Section reference to be a strong reference keeping items in that section alive in llvm/llvm-project@6d2d3bd. They mention that code which needs the old behaviour should use the SHF_GNU_RETAIN strategy, so linkme and other rust crates using this technique will need rust support for #[used(retain)] or similar to continue working.

Based on that commit message, it seems like modern GNU ld changed from the behaviour which old GNU ld and lld use, which is why strategies like this have worked for the last 6 years (https://sourceware.org/bugzilla/show_bug.cgi?id=19167).

Alternatively, it appears one could build with the linker flags -z no-start-stop-gc, which should also cause these sections to be retained.

hawkw added a commit to hawkw/mycelium that referenced this issue Sep 26, 2021
this fixes the build on recent nightlies --- it appears the linker hax
we use for tests breaks on recent `rust-lld`s due to upstream changes.
see dtolnay/linkme#41 for details on a similar
issue.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@dtolnay
Copy link
Owner Author

dtolnay commented Sep 26, 2021

I thought about just pretending to be libc (see summary and implementation of llvm/llvm-project@6d2d3bd) but that 7 extra chars which exacerbates #35.

We could introduce a hash to turn the original symbol name into 9 a-zA-Z0-9 chars. That's 62^9 state space so by birthday paradox a good hash would have room for 62^4.5 = 116 million symbols, which seems fine.

@cynecx
Copy link

cynecx commented Dec 3, 2021

From the rustc side, we could do the same as clang and use llvm.compiler.used for ELF and llvm.used for MachO and COFF -- it's weirdly inconsistent, but I guess doing the same thing as clang has a consistency of its own ;

But wouldn't linkme still have "undefined behaviour" on ELF based platforms since llvm.compiler.used doesn't guarantee retain section semantics? Isn't that the situation right now? At least on rust stable, linkme is/should be broken on all platforms.

nvzqz added a commit to nvzqz/linkme that referenced this issue Jan 9, 2022
This adds the `S_ATTR_NO_DEAD_STRIP` section attribute, which forces
"unused" code to remain in the binary. This is paired with `S_REGULAR`
to provide a required section type.

This fixes dtolnay#41.
helce pushed a commit to helce/rust that referenced this issue Aug 22, 2023
helce pushed a commit to helce/rust that referenced this issue Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants