-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Expose elf abi on ppc64 targets #142321
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
Expose elf abi on ppc64 targets #142321
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
Some changes occurred in compiler/rustc_codegen_ssa |
I don't think we have any business with Power's ELFv1 format. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if we do have anything to do with it, we shouldn't be guessing.
I marked this as a draft because there's now a problem that I really dislike (and ppc64 targets are currently broken):
Compiling the above mentioned program with this target file again produces the broken binary because the abi field is not passed to llvm (powerpc64-unknown-none-elf uses ELFv1 in llvm). |
This comment has been minimized.
This comment has been minimized.
@ostylk This is determined by llvm_abiname, not target.abi. |
Yeah, there are cases where they match but in general target.abi is a cfg option for Rust userspace and target.llvm_abiname is the codegen control. |
Oh thanks, I somehow overlooked llvm_abiname. I changed it to use llvm_abiname, because it really matters what the llvm codegen does. Also I changed the existing ppc64 targets to set llvm_abiname and abi (in case an app needs that information) accordingly. |
These commits modify compiler targets. |
This comment has been minimized.
This comment has been minimized.
@rustbot ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me. I also found #60617 which asks for exposing the ELF ABI version, so I guess this fixes that issue as well.
Does exposing something new to target_abi require any special sign-off?
const EF_PPC64_ABI_ELF_V2: u32 = 2; | ||
|
||
if sess.target.options.llvm_abiname.as_ref().is_empty() | ||
&& sess.target.options.binary_format.to_object() == BinaryFormat::Elf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this code can't even be reached for non-ELF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering this if branch https://github.com/rust-lang/rust/blob/master/compiler%2Frustc_codegen_ssa%2Fsrc%2Fback%2Fmetadata.rs#L224 (the call to elf_e_flags is after the block), I assumed that it could be called for COFF files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding of https://forge.rust-lang.org/compiler/proposals-and-stabilization.html#targets is that it requires an FCP now to add something like a field that will be visible via target_abi
, since it's user-facing. Or at least MCP.
That is why I suggested splitting up codegen and user-facing parts (I mean people can poke at binaries and so in a strained sense that's "user-facing" but you know what I mean) so that the former is not stuck on process. They'll probably ship in different versions as a result but that seems completely fine.
Though thinking about it, that seems patently silly, as we could remove tier 3 targets with just PR approval. |
Cool. @bors r+ |
??? misclick. @bors r+ |
…ilee Expose elf abi on ppc64 targets Fixes rust-lang#60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR rust-lang#142598
Rollup of 12 pull requests Successful merges: - #141829 (Specialize sleep_until implementation for unix (except mac)) - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142568 (Use the .drectve section for exporting symbols from dlls on Windows) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…ilee Expose elf abi on ppc64 targets Fixes rust-lang#60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR rust-lang#142598
Rollup of 11 pull requests Successful merges: - #141829 (Specialize sleep_until implementation for unix (except mac)) - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…ilee Expose elf abi on ppc64 targets Fixes rust-lang#60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR rust-lang#142598
Rollup of 12 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…ilee Expose elf abi on ppc64 targets Fixes rust-lang#60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR rust-lang#142598
Rollup of 11 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…ilee Expose elf abi on ppc64 targets Fixes rust-lang#60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR rust-lang#142598
Rollup of 10 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
Oh yeah, this is purely additive so |
Rollup merge of #142321 - ostylk:fix/ppc64_abi, r=workingjubilee Expose elf abi on ppc64 targets Fixes #60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets. Conditional compilation can now use `cfg(target_abi = "elfv1")` or `cfg(target_abi = "elfv2")` to determine the abi in use. Technical details are included in the other PR #142598
Fixes #60617 (after MCP rust-lang/compiler-team#885 is accepted) by exposing the abi information on ppc64 targets.
Conditional compilation can now use
cfg(target_abi = "elfv1")
orcfg(target_abi = "elfv2")
to determine the abi in use.Technical details are included in the other PR #142598
original pull request text
Attempts to fix https://github.com//issues/85589 (also more information is there).Basically the problem is that ld.lld assumes that all ppc64 object files with e_flags=0 are object files which use the ELFv2 ABI (this here is the check https://github.com/llvm/llvm-project/blob/main/lld/ELF/Arch/PPC64.cpp#L639).
This pull request sets the correct e_flags to indicate the used ABI so ld.lld errors out when encountering ELFv1 ABI files instead of generating a broken binary.
For example compare code generation for this program (file name
min.rs
):Compile with
rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs
Before change:
The branch instructions
bl 0x10030378
andbl 0x10030390
are jumping into the.opd
section which is data. That is a broken binary (because fixing those branches is the task of the linker).After change:
Which is correct because ld.lld doesn't support ELFv1 ABI.