Fix e_flags for 32-bit MIPS targets in generated object file #96930
+23
−15
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
In rust-lang#95604 the compiler started generating a temporary symbols.o which is added to the linker invocation. This object file has an `e_flags` which may be invalid for 32-bit MIPS targets. Even though symbols.o doesn't contain code, linking with [lld fails](https://github.com/llvm/llvm-project/blob/main/lld/ELF/Arch/MipsArchTree.cpp#L79) with ``` rust-lld: error: foo-cgu.0.rcgu.o: ABI 'o32' is incompatible with target ABI 'n64' ``` because it omits the ABI bits (EF_MIPS_ABI_O32) so lld assumes it's using the N64 ABI. This breaks linking on nightly for the out-of-tree [psx target](ayrtonm/psx-sdk-rs#9), the builtin mipsel-sony-psp target (cc @overdrivenpotato) and any other 32-bit MIPS target using lld. This PR sets the ABI in `e_flags` to O32 since that's the only ABI for 32-bit MIPS that LLVM supports. It also sets other `e_flags` bits based on the target. I had to bump the object crate version since some of these constants were [added recently](gimli-rs/object#433). I'm not sure if this PR needs a test, but I can confirm that it fixes the linking issue on both targets I mentioned.
rustbot
added
the
T-compiler
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
petrochenkov
added
S-waiting-on-author
|
Thanks! If this doesn't get into 1.61 beta in time please ping me and we'll backport this. |
|
📌 Commit 3d5b1ee has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (1c80ac0): comparison url. Summary: This benchmark run did not return any relevant results. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
|
@petrochenkov This PR landed in 1.62 -- was there a separate PR that should be landing in 1.61 beta? |
|
@Mark-Simulacrum The important part is to land this PR into the same release as #95604, because it fixes a regression introduced by it. |
xSetech
added a commit
to xSetech/rust
that referenced
this pull request
Jul 9, 2023
PR rust-lang#95604 introduced a "synthetic object file to ensure all exported and used symbols participate in the linking". One constraint on this file is that for MIPS-based targets, its architecture-specific ELF flags must be the same as all other object files passed to the linker. That's enforced by LLD, here: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/lld/ELF/Arch/MipsArchTree.cpp#L77 The current approach to determining e_flags for 32-bit was implemented in PR rust-lang#96930, which links to this issue that summarizes the problem well: ayrtonm/psx-sdk-rs#9 > ... the temporary object file is created with an e_flags which is > invalid for 32-bit MIPS targets. The main issue is that it omits the ABI > bits (EF_MIPS_ABI_O32) which implies it uses the N64 ABI. To enable the N32 MIPS ABI (which succeeded O32), this patch enables setting the synthetic object's ABI based on the target "llvm-abiname" field, if it's given; otherwise, the O32 ABI is assumed for 32-bit MIPS targets. More information about the N32 ABI can be found here: https://web.archive.org/web/20160121005457/http://techpubs.sgi.com/library/manuals/2000/007-2816-005/pdf/007-2816-005.pdf
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jul 11, 2023
Support explicit 32-bit MIPS ABI for the synthetic object PR rust-lang#95604 introduced a "synthetic object file to ensure all exported and used symbols participate in the linking". One constraint on this file is that for MIPS-based targets, its architecture-specific ELF flags must be the same as all other object files passed to the linker. That's enforced by LLD, here: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.6/lld/ELF/Arch/MipsArchTree.cpp#L77 The current approach to determining e_flags for 32-bit was implemented in PR rust-lang#96930, which links to this issue that summarizes the problem well: ayrtonm/psx-sdk-rs#9 > ... the temporary object file is created with an e_flags which is > invalid for 32-bit MIPS targets. The main issue is that it omits the ABI > bits (EF_MIPS_ABI_O32) which implies it uses the N64 ABI. To enable the N32 MIPS ABI (which succeeded O32), this patch enables setting the synthetic object's ABI based on the target "llvm-abiname" field, if it's given; otherwise, the O32 ABI is assumed for 32-bit MIPS targets. More information about the N32 ABI can be found here: https://web.archive.org/web/20160121005457/http://techpubs.sgi.com/library/manuals/2000/007-2816-005/pdf/007-2816-005.pdf
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.
In #95604 the compiler started generating a temporary symbols.o which is added to the linker invocation. This object file has an
e_flagswhich is invalid for 32-bit MIPS targets. Even though symbols.o doesn't contain code, linking these targets with lld fails withbecause it omits the ABI bits (
EF_MIPS_ABI_O32) so lld assumes it's using the N64 ABI. This breaks linking on nightly for the out-of-tree mipsel-sony-psx target, the builtin mipsel-sony-psp target (cc @overdrivenpotato) and probably any other 32-bit MIPS target using lld.This PR sets the ABI in
e_flagsto O32 since that's the only ABI for 32-bit MIPS that LLVM supports. It also sets othere_flagsbits based on the target to avoid similar issues with the object file arch and PIC. I had to bump the object crate version since some of these constants were added recently. I'm not sure if this PR needs a test, but I can confirm that it fixes the linking issue on both targets I mentioned.