-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Link object files that use #[used]
#137426
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Weird. The tests failed on macOS after I fixed various test cases. I'll look into it tomorrow. |
I think we should link all rlibs with |
I found that I switched from traversing the object file to using the symbol table. I'm unsure why rustc doesn't add the symbol table when building static libraries for macOS. I have implemented a more robust approach that doesn't rely on the static library. BTW, I included an experimental commit that doesn't need to be part of the PR. |
Beyond the overhead to compile time, this appears to subtly alter the semantics of rlibs during symbol resolution? In C, at least, we only typically load undefined symbols. I'm uncertain what impact this modification might have on real-world projects. |
You already can't depend on this behavior in Rust. Rustc changes the codegen unit partitioning between rustc versions, so if in one rustc version you wouldn't get a function pulled in, the next rustc version may cause it to be pulled in anyway if the function ends up co-located with another function that is used. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
I'm testing this at #137481. |
[perf experiment] Link rlibs with `--whole-archive` For rust-lang#137426 (comment). r? ghost
IIUC, if all libs are well-defined, no issue here? So this is a bad experience when encountering an undefined symbol error? |
[perf experiment] Link rlibs with `--whole-archive` For rust-lang#137426 (comment). r? ghost
[perf experiment] Link rlibs with `--whole-archive` For rust-lang#137426 (comment). r? ghost
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
Link object files that use `#[used]` By directly linking the object files that use `#[used]`, we ensure the linker can see them. This approach allows `#[used]` to avoid modifying symbol visibility, preserving local symbols. A similar example in C would be: ```c // foo.c __attribute__((constructor)) static void foo() {} // main.c void main(void) {} ``` If `foo.c` is placed in a static library, it will never be loaded unless the entire static library is fully loaded by `--whole-archive`. This pull request removes some of the symbols in `symbols.o`. We can remove more symbols in a follow-up PR.
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Correct. If all libs are well-defined, adding |
If we do |
When creating a staticlib we need to keep it merged as for those there is no way to tell the build system to link a separate import library. |
☔ The latest upstream changes (presumably #135726) made this pull request unmergeable. Please resolve the merge conflicts. |
By directly linking the object files that use
#[used]
, we ensure the linker can see them.This approach allows
#[used]
to avoid modifying symbol visibility, preserving local symbols. A similar example in C would be:If
foo.c
is placed in a static library, it will never be loaded unless the entire static library is fully loaded by--whole-archive
.This pull request removes some of the symbols in
symbols.o
. We can remove more symbols in a follow-up PR.