rust: Update to 1.58.1 #10555
Merged
rust: Update to 1.58.1 #10555
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I did neither compile nor test this new version on my machine... |
mati865
approved these changes
Jan 16, 2022
|
I don't think we should block on clang* error: |
I looked into that at one point. Each entry in .pdata is 12 bytes long, so lld assumes the whole section should be a multiple of 12 bytes long. Perhaps rust is 0-padding it to another alignment, like 8 bytes. |
|
🚀 ? |
|
You might want to wait for 1.58.1 that will be released tomorrow assuming Rust's CI gets in better shape. |
hm, autobuild can't handle ignoring an update for specific types right now... I'll have a look |
|
CLANG64 fails with: and yet the build is flagged as successful. |
|
Spurious network failure.
CI is configured to gate only on MINGW* and UCRT64 subsystems. |
|
🚀 ? |
|
Is anything holding up a merge ? |
|
I got the clang build to work with
|
|
It's that error again: |
|
so much for my theory of it padding to an 8- or 16-byte alignment, 1717324 is not a multiple of 8 either. |
|
I pasted a totally unhelpful error message... |
|
While hacking around to force the use of cc, I saw warnings about an unsupported "--no-pie" argument. |
|
Here is the full error : Does it always happen at this step ? PS: Yes, it does. |
|
The check on .pdata size was added by : llvm/llvm-project@e73d0b5 |
|
So 1717324 is the sum of all the pdata section sizes. Need to find which one(s) are throwing off the 12 bytes alignment. |
You might be right after all. The size is the difference between the pointer to the last entry and the first entry plus the size of the last entry. The last entry size is 12. And we have 1717324 - 12 = 1717312 = 2^6 x 26833. Anyways, I am looking into the issue. But building clang is long... |
|
Or like in the comment from commit you have linked codegen goofed and 4 bytes were added since (1717324 - 4) % 12 = 0. |
|
And the fact that the clang32 is not having this issue is probably worth looking into, searching for differences. The relevant PE/COFF spec section is here : https://docs.microsoft.com/en-us/windows/win32/debug/pe-format?redirectedfrom=MSDN#the-pdata-section The trigger function : https://github.com/llvm/llvm-project/blob/main/lld/COFF/Writer.cpp#L1930 That code is brittle at best. I understand that it sorts the entries in place (in the destination buffer) for some reason. |
|
|
|
Made some progress. The The two entries are :
They are loaded from The rlib is linked like this: Many entries in that rlib trigger this message in verbose mode: |
|
I commented out this call : https://github.com/llvm/llvm-project/blob/main/lld/COFF/InputFiles.cpp#L424 Good news is that The thing I commented out was introduced by llvm/llvm-project@21858a9 The related discussion mentions that clang+lld and gcc objects does not work. Are we in this situation ? |
|
I did a test build of rust on CLANG32 with the twice-built clang update, but still get the panic failure (which I previously found to be missing unwind information in @filnet can you share how you were able to get the argument for generating the repro.tar into the rust link command? That was the next thing I wanted to try on CLANG32 but was unable to figure it out. Perhaps one complicating factor in this case is that the link doesn't fail. |
|
@jeremyd2019 AFAICT it won't work since linking gives no error, otherwise it should be |
|
Do we still need the |
Yes. I tested with Edit : I meant |
So if stripping the llvm libraries, the built rustc is broken, but if using an unstripped build, it's not broken (or less broken) - is that right? Was that stripped with the minimally patched llvm-strip, or with the full version of the patches (that got merged into upstream a couple days ago, and also was backported to the 14.x release branch)? If stripping still breaks something, with the fully patched llvm-strip, then there's another bug in the strip tool, that I'd like to fix, but I'd need some assistance in pinpointing the individual library+object file where stripping breaks something (and how it's broken). |
|
Sorry got it wrong, we currently have |
|
So:
I started to look into why stripping rust fails.
I noticed this: |
|
And this was done with the minimal patch. |
|
Will try again with the full patch. But I'll be afk for a week starting tonight. |
|
Rebuild looks good. Thanks everyone! |
|
Testing without |
|
Far from being small but enough to reproduce the problem @mstorsjo: Tarball with binaries is available at: Our package manager calls |
Awesome, thanks! I’ll download it and try to look into it later. So if you do the same with GNU binutils strip, it doesn’t break? |
… because the issue can either be that objcopy/strip breaks something, or be that the DLL has a layout that gets broken by stripping out debug info, regardless of which tool does the stripping. |
Sorry for the late reply, stripping debug info with Binutils 2.38 yelds similarly (the same way?) broken compiler. |
Oh, ok. But a rustc built with a regular gnu toolchain, does that one cope with being stripped? (I might be able to pinpoint the issue by looking at the binaries you provided anyway; haven’t gotten there yet.) |
Tested only the one built with LLVM toolchain, won't be able to check thr one built with GNU until tomorrow. |
|
Interesting, I have ran Binutils |
Awesome, can you share a copy of the corresponding files from such a build too (before stripping)? I’ve started looking at the issue, and I’m pretty sure about what it is. Even better, if you’d be able to provide a |
|
FWIW, the issue is that the DLL has got a Therefore, it'd be nice to have linkrepro inputs when linking such a DLL with LLD, so that I could see what I can do to make it place the sections in an order where it doesn't break when stripped. (It could also be interesting to see the corresponding inputs (or at least the command line) to GNU binutils ld, when it links the corresponding DLL, to see what influences it to place the sections in a different order.) |
https://1drv.ms/u/s!AgMYIlqTF8b9guhs_uZkHsJMlZoJOw?e=CBpjXN GNU one even seems to survive (tested only if version shows) running
https://1drv.ms/u/s!AgMYIlqTF8b9guhr-rTehRBVGKs2yQ?e=ZkmsyC It was a bit tricky because Rust's build system automatically handles dependencies like
I think it should be very similar, the differences should reduce to linking winpthreads and lack of |
Thanks! Looking at the linker inputs; the rlib files contains one object file named Thus I don't think there's any specific behaviour in GNU ld that LLD should try to match wrt this. Instead, maybe the best way ahead would be to teach LLD to sort all |
|
Sounds sensible, thank you for sorting out all those issues. |
|
A potential fix for LLD, to make the rust binaries strippable, is up for review at https://reviews.llvm.org/D120805 now. |
mstorsjo
added a commit
to llvm/llvm-project
that referenced
this pull request
Mar 3, 2022
…s if stripped So far, we sort all discardable sections at the end, with only some extra logic to make sure that the .reloc section is at the start of that group of sections. But if there are other discardable sections, other than .reloc, they must also be ordered before .debug_* sections, to avoid leaving gaps if the executable is stripped. (Stripping executables doesn't remove all discardable sections, only the ones named .debug_*). Rust binaries seem to include a .rmeta section, which is marked discardable. This fixes stripping such binaries if built with dwarf debug info included. This fixes issues observed in MSYS2 in msys2/MINGW-packages#10555. Differential Revision: https://reviews.llvm.org/D120805
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Mar 3, 2022
…s if stripped So far, we sort all discardable sections at the end, with only some extra logic to make sure that the .reloc section is at the start of that group of sections. But if there are other discardable sections, other than .reloc, they must also be ordered before .debug_* sections, to avoid leaving gaps if the executable is stripped. (Stripping executables doesn't remove all discardable sections, only the ones named .debug_*). Rust binaries seem to include a .rmeta section, which is marked discardable. This fixes stripping such binaries if built with dwarf debug info included. This fixes issues observed in MSYS2 in msys2/MINGW-packages#10555. Differential Revision: https://reviews.llvm.org/D120805 (cherry picked from commit 4c3b74b)
tstellar
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Mar 7, 2022
…s if stripped So far, we sort all discardable sections at the end, with only some extra logic to make sure that the .reloc section is at the start of that group of sections. But if there are other discardable sections, other than .reloc, they must also be ordered before .debug_* sections, to avoid leaving gaps if the executable is stripped. (Stripping executables doesn't remove all discardable sections, only the ones named .debug_*). Rust binaries seem to include a .rmeta section, which is marked discardable. This fixes stripping such binaries if built with dwarf debug info included. This fixes issues observed in MSYS2 in msys2/MINGW-packages#10555. Differential Revision: https://reviews.llvm.org/D120805 (cherry picked from commit 4c3b74b)
mem-frob
pushed a commit
to draperlaboratory/hope-llvm-project
that referenced
this pull request
Oct 7, 2022
…s if stripped So far, we sort all discardable sections at the end, with only some extra logic to make sure that the .reloc section is at the start of that group of sections. But if there are other discardable sections, other than .reloc, they must also be ordered before .debug_* sections, to avoid leaving gaps if the executable is stripped. (Stripping executables doesn't remove all discardable sections, only the ones named .debug_*). Rust binaries seem to include a .rmeta section, which is marked discardable. This fixes stripping such binaries if built with dwarf debug info included. This fixes issues observed in MSYS2 in msys2/MINGW-packages#10555. Differential Revision: https://reviews.llvm.org/D120805
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Had to recreate
0001-add-missing-libs.patchto match rust-lang/rust@a076f2b