-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Copy libgcc as well after building musl #52157
Conversation
This is needed for rust-lang/libc#1034.
@bors: r+ |
📌 Commit 7ec188b has been approved by |
Copy libgcc as well after building musl This is needed for rust-lang/libc#1034. r? @alexcrichton
⌛ Testing commit 7ec188b with merge b3cc007073312b83a1d4dd0eeb65a0ed042631eb... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I believe that's rust-lang/libc#1037 |
I updated the libc submodule to include that change. |
@bors: r+ delegate=Amanieu |
✌️ @Amanieu can now approve this pull request |
📌 Commit d190482666cb031a04ef9067333dec55af380843 has been approved by |
@Amanieu you can r+ now. |
@bors r+ |
Thanks for tracking that down @Amanieu! Feel free to update libc and re-approve when the libc PR goes through |
@bors r+ |
📌 Commit 3ad6ff5 has been approved by |
⌛ Testing commit 3ad6ff5 with merge a6b301ecdd80bca567adfc4fef5c1107b4ce6152... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Hmm, so apparently just linking in libgcc leads to duplicated symbols with the ones in compiler-builtins, which causes a linker error. Any ideas? |
Hm I'm not sure how to solve that :( I thought this was tested ahead of time though to work? |
I only tested the aarch64 musl toolchain, which worked fine. It seems that the x86_64 one is failing. Maybe we should just include the needed symbols directly in compiler-builtins? |
We can, yes, but I think this is likely to eventually bite us for aarch64, no? |
Not really. This is the issue for aarch64: rust-lang/compiler-builtins#201 Another possibility would be to mark all the builtins in compiler-builtins as |
We may want to just have aarch64 specific build logic in that case (vendored into this repository) which statically links libgcc but a stripped down version (one that we don't ship as well). That would only contain the objects needed to get libc iteslf linking and compiling correctly, and it'd only be intrinsics that aren't already provided by compiler-builtins. Is that possible? |
This feels very fragile. What about other architectures that may depend on libgcc functions? I had a quick look at the musl source code, the only other architectures which need libgcc support for FWIW, the reason that |
Looking at aarch64 musl, it calls all of the following
It is purely by coincidence that when we link to libc we only need 3 of these for a small test program. |
It's true that it's fragile but I'm not really sure if there's a better solution here? I'm up for whatever if it's more robust |
Ping from triage! @Amanieu we haven't heard from you for a while, will you have time to work on this PR? |
I think that we will have to revert the libc changes and try another approach. I'm closing this PR. |
Revert "Link to libgcc when statically linking musl" This causes linker errors due to duplicated symbols. See rust-lang/rust#52157
This is needed for rust-lang/libc#1034.
r? @alexcrichton