-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Fix -Zgcc-ld=lld
#101504
Fix -Zgcc-ld=lld
#101504
Conversation
I tested it on windows-gnu in the previous PR. |
Thanks! |
for both windows and unixes
now that CI correctly detects rust-lld in run-make tests, we ignore this test since it relies on `-Zgcc-ld=lld` which is not made to work on the windows-msvc targets: it requires a gcc flavor.
@bors r=petrochenkov rollup=never Could be safe to rollup, but you never know with run-make tests here, especially since they were ignored before, and may require new directives for some platforms. |
🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (44adfcc): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
Using the latest nightly build, |
@Wyvern Also, does the same error happen if you use |
Interesting. I've tested the PR on an M1 with macOS 12.5.1: it correctly linked a helloworld. CI should also test it now on the osx builders, and the one test using it passes locally. I've now also checked the latest nightly on the same machine, and the helloworld still runs.
(You can see this is our Something more subtle is probably happening. I feel we've seen some similar errors on CI when linking with clang and LTO recently. I've also tested more complicated examples using tokio ( @Wyvern if you could open a dedicated issue with an example that reproduces your error, that'd be great. We'll look into it: if we need to ping the osx experts, a reproducible example, with OS/Xcode/LLVM details (for example I know |
Please ping me on the new issue. I have the same problem. x86 OSX. |
I'm not to familiar with usage of |
In the cc invocation I found: "-fuse-ld=lld" "--target=x86_64-apple-macosx10.7.0" but > cc --print-target-triple
x86_64-apple-darwin21.6.0
> xcrun --sdk macosx --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk This is OSX 12.5.1. |
I was under the impression that you couldn't target this kind of version without a compatible SDK, I might be wrong though. Another thing that looks odd to me: why explicitly giving the version of macOS you are targeting in the target? The normal way of doing it is with Finally, |
The last "-Zgcc-ld=lld" working normally Rust build was At first, it was All errors happened on the same macos without LLVM installed. |
I filed a new issue to continue the discussion: |
-Zgcc-ld=lld
is currently broken. CI is currently ignoring its tests.cc @Mark-Simulacrum on the
compiletest
change: I'm not sure which ofbootstrap
's test step orcompiletest
is currently incorrect wrt windows'--compile-lib-path
. Sincesysroot/bin
is passed on windows, that means thatcompiletest
can't findrust-lld
on windows and tests are currently ignored: it's looking for something that is insysroot/lib
instead.They are currently ignored on unixes for a different reason: the lld wrapper has a different name than what is checked.
(I've changed
compiletest
in this PR, just because I could make a very targeted change there, whereas completely changing the intentional lib path that is passed seemed it'd have wider reaching implications on all tests.)And in both unix/win cases, I've changed the detection to look for
rust-lld
rather than the wrappers inbin/gcc-ld/
. It seems like the more stable of all these executable names.r? @petrochenkov
I've tested the
lld-wrapper
change on linux and osx, but couldn't test on windows gnu targets (I only have MSVC targets, and these can't userust-lld
via-Zgcc-ld=lld
, nor do they use the lld wrapper IIUC).I'd expect it to work whether or not the wrapper is called with or without an executable suffix. But at least now CI should test it in these targets.
Fixes #101370.