-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
base: master
Are you sure you want to change the base?
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. |
I have opened a proposal to clarify we do not need that much process for this sort of additive change. But I think that as long as this PR incorporates the current user-facing elements, it would unfortunately still be stuck on the process of agreeing we want to change the process to require less process, even if that process may be lighter than the process that currently seems to be prescribed. :^) Specifically, 10 days. |
First of all, thank you @workingjubilee for taking care of all the processes required to incorporate this change. If I understand correctly the "user-facing change" is only the |
@ostylk Correct. |
I split this PR in two. #142598 contains the pure code generation changes. When the other PR is merged, I'll rebase this to only include the last commit. |
Going through the list of 64-bit powerpc targets, the only missing one is Afaict 32-bit targets don't have ELF ABI v1/v2 stuff. |
…workingjubilee Set elf e_flags on ppc64 targets according to abi (This PR contains the non user-facing changes of rust-lang#142321) Fixes rust-lang#85589 by making sure that ld.lld errors out instead of generating a broken binary. 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``): ```rust #![feature(no_core, lang_items, repr_simd)] #![crate_type = "bin"] #![no_core] #![no_main] #[lang = "sized"] trait Sized {} #[lang = "copy"] trait Copy {} #[lang = "panic_cannot_unwind"] pub fn panic() -> ! { loop {} } pub fn my_rad_unmangled_function() { loop {} } pub fn my_rad_function() { loop {} } #[no_mangle] pub fn _start() { my_rad_unmangled_function(); my_rad_function(); } ``` Compile with ``rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs`` Before change: ``` $ llvm-objdump -d min Disassembly of section .text: 000000001001030c <.text>: ... 10010334: 7c 08 02 a6 mflr 0 10010338: f8 21 ff 91 stdu 1, -112(1) 1001033c: f8 01 00 80 std 0, 128(1) 10010340: 48 02 00 39 bl 0x10030378 <_ZN3min25my_rad_unmangled_function17h7471c49af58039f5E> 10010344: 60 00 00 00 nop 10010348: 48 02 00 49 bl 0x10030390 <_ZN3min15my_rad_function17h37112b8fd1008c9bE> 1001034c: 60 00 00 00 nop ... ``` The branch instructions ``bl 0x10030378`` and ``bl 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: ``` error: linking with `ld.lld` failed: exit status: 1 | = note: "ld.lld" "/tmp/rustcNYKZCS/symbols.o" "<1 object files omitted>" "--as-needed" "-L" "/tmp/rustcNYKZCS/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "<sysroot>/lib/rustlib/powerpc64-unknown-linux-gnu/lib" "-o" "min" "--gc-sections" "-z" "relro" "-z" "now" = note: some arguments are omitted. use `--verbose` to show all linker arguments = note: ld.lld: error: /tmp/rustcNYKZCS/symbols.o: ABI version 1 is not supported ``` Which is correct because ld.lld doesn't support ELFv1 ABI.
…workingjubilee Set elf e_flags on ppc64 targets according to abi (This PR contains the non user-facing changes of rust-lang#142321) Fixes rust-lang#85589 by making sure that ld.lld errors out instead of generating a broken binary. 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``): ```rust #![feature(no_core, lang_items, repr_simd)] #![crate_type = "bin"] #![no_core] #![no_main] #[lang = "sized"] trait Sized {} #[lang = "copy"] trait Copy {} #[lang = "panic_cannot_unwind"] pub fn panic() -> ! { loop {} } pub fn my_rad_unmangled_function() { loop {} } pub fn my_rad_function() { loop {} } #[no_mangle] pub fn _start() { my_rad_unmangled_function(); my_rad_function(); } ``` Compile with ``rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs`` Before change: ``` $ llvm-objdump -d min Disassembly of section .text: 000000001001030c <.text>: ... 10010334: 7c 08 02 a6 mflr 0 10010338: f8 21 ff 91 stdu 1, -112(1) 1001033c: f8 01 00 80 std 0, 128(1) 10010340: 48 02 00 39 bl 0x10030378 <_ZN3min25my_rad_unmangled_function17h7471c49af58039f5E> 10010344: 60 00 00 00 nop 10010348: 48 02 00 49 bl 0x10030390 <_ZN3min15my_rad_function17h37112b8fd1008c9bE> 1001034c: 60 00 00 00 nop ... ``` The branch instructions ``bl 0x10030378`` and ``bl 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: ``` error: linking with `ld.lld` failed: exit status: 1 | = note: "ld.lld" "/tmp/rustcNYKZCS/symbols.o" "<1 object files omitted>" "--as-needed" "-L" "/tmp/rustcNYKZCS/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "<sysroot>/lib/rustlib/powerpc64-unknown-linux-gnu/lib" "-o" "min" "--gc-sections" "-z" "relro" "-z" "now" = note: some arguments are omitted. use `--verbose` to show all linker arguments = note: ld.lld: error: /tmp/rustcNYKZCS/symbols.o: ABI version 1 is not supported ``` Which is correct because ld.lld doesn't support ELFv1 ABI.
…workingjubilee Set elf e_flags on ppc64 targets according to abi (This PR contains the non user-facing changes of rust-lang#142321) Fixes rust-lang#85589 by making sure that ld.lld errors out instead of generating a broken binary. 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``): ```rust #![feature(no_core, lang_items, repr_simd)] #![crate_type = "bin"] #![no_core] #![no_main] #[lang = "sized"] trait Sized {} #[lang = "copy"] trait Copy {} #[lang = "panic_cannot_unwind"] pub fn panic() -> ! { loop {} } pub fn my_rad_unmangled_function() { loop {} } pub fn my_rad_function() { loop {} } #[no_mangle] pub fn _start() { my_rad_unmangled_function(); my_rad_function(); } ``` Compile with ``rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs`` Before change: ``` $ llvm-objdump -d min Disassembly of section .text: 000000001001030c <.text>: ... 10010334: 7c 08 02 a6 mflr 0 10010338: f8 21 ff 91 stdu 1, -112(1) 1001033c: f8 01 00 80 std 0, 128(1) 10010340: 48 02 00 39 bl 0x10030378 <_ZN3min25my_rad_unmangled_function17h7471c49af58039f5E> 10010344: 60 00 00 00 nop 10010348: 48 02 00 49 bl 0x10030390 <_ZN3min15my_rad_function17h37112b8fd1008c9bE> 1001034c: 60 00 00 00 nop ... ``` The branch instructions ``bl 0x10030378`` and ``bl 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: ``` error: linking with `ld.lld` failed: exit status: 1 | = note: "ld.lld" "/tmp/rustcNYKZCS/symbols.o" "<1 object files omitted>" "--as-needed" "-L" "/tmp/rustcNYKZCS/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "<sysroot>/lib/rustlib/powerpc64-unknown-linux-gnu/lib" "-o" "min" "--gc-sections" "-z" "relro" "-z" "now" = note: some arguments are omitted. use `--verbose` to show all linker arguments = note: ld.lld: error: /tmp/rustcNYKZCS/symbols.o: ABI version 1 is not supported ``` Which is correct because ld.lld doesn't support ELFv1 ABI.
…workingjubilee Set elf e_flags on ppc64 targets according to abi (This PR contains the non user-facing changes of rust-lang#142321) Fixes rust-lang#85589 by making sure that ld.lld errors out instead of generating a broken binary. 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``): ```rust #![feature(no_core, lang_items, repr_simd)] #![crate_type = "bin"] #![no_core] #![no_main] #[lang = "sized"] trait Sized {} #[lang = "copy"] trait Copy {} #[lang = "panic_cannot_unwind"] pub fn panic() -> ! { loop {} } pub fn my_rad_unmangled_function() { loop {} } pub fn my_rad_function() { loop {} } #[no_mangle] pub fn _start() { my_rad_unmangled_function(); my_rad_function(); } ``` Compile with ``rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs`` Before change: ``` $ llvm-objdump -d min Disassembly of section .text: 000000001001030c <.text>: ... 10010334: 7c 08 02 a6 mflr 0 10010338: f8 21 ff 91 stdu 1, -112(1) 1001033c: f8 01 00 80 std 0, 128(1) 10010340: 48 02 00 39 bl 0x10030378 <_ZN3min25my_rad_unmangled_function17h7471c49af58039f5E> 10010344: 60 00 00 00 nop 10010348: 48 02 00 49 bl 0x10030390 <_ZN3min15my_rad_function17h37112b8fd1008c9bE> 1001034c: 60 00 00 00 nop ... ``` The branch instructions ``bl 0x10030378`` and ``bl 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: ``` error: linking with `ld.lld` failed: exit status: 1 | = note: "ld.lld" "/tmp/rustcNYKZCS/symbols.o" "<1 object files omitted>" "--as-needed" "-L" "/tmp/rustcNYKZCS/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "<sysroot>/lib/rustlib/powerpc64-unknown-linux-gnu/lib" "-o" "min" "--gc-sections" "-z" "relro" "-z" "now" = note: some arguments are omitted. use `--verbose` to show all linker arguments = note: ld.lld: error: /tmp/rustcNYKZCS/symbols.o: ABI version 1 is not supported ``` Which is correct because ld.lld doesn't support ELFv1 ABI.
Rollup merge of #142598 - ostylk:fix/ppc64_llvmabi, r=nikic,workingjubilee Set elf e_flags on ppc64 targets according to abi (This PR contains the non user-facing changes of #142321) Fixes #85589 by making sure that ld.lld errors out instead of generating a broken binary. 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``): ```rust #![feature(no_core, lang_items, repr_simd)] #![crate_type = "bin"] #![no_core] #![no_main] #[lang = "sized"] trait Sized {} #[lang = "copy"] trait Copy {} #[lang = "panic_cannot_unwind"] pub fn panic() -> ! { loop {} } pub fn my_rad_unmangled_function() { loop {} } pub fn my_rad_function() { loop {} } #[no_mangle] pub fn _start() { my_rad_unmangled_function(); my_rad_function(); } ``` Compile with ``rustc --target=powerpc64-unknown-linux-gnu -C linker=ld.lld -C relocation-model=static min.rs`` Before change: ``` $ llvm-objdump -d min Disassembly of section .text: 000000001001030c <.text>: ... 10010334: 7c 08 02 a6 mflr 0 10010338: f8 21 ff 91 stdu 1, -112(1) 1001033c: f8 01 00 80 std 0, 128(1) 10010340: 48 02 00 39 bl 0x10030378 <_ZN3min25my_rad_unmangled_function17h7471c49af58039f5E> 10010344: 60 00 00 00 nop 10010348: 48 02 00 49 bl 0x10030390 <_ZN3min15my_rad_function17h37112b8fd1008c9bE> 1001034c: 60 00 00 00 nop ... ``` The branch instructions ``bl 0x10030378`` and ``bl 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: ``` error: linking with `ld.lld` failed: exit status: 1 | = note: "ld.lld" "/tmp/rustcNYKZCS/symbols.o" "<1 object files omitted>" "--as-needed" "-L" "/tmp/rustcNYKZCS/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "<sysroot>/lib/rustlib/powerpc64-unknown-linux-gnu/lib" "-o" "min" "--gc-sections" "-z" "relro" "-z" "now" = note: some arguments are omitted. use `--verbose` to show all linker arguments = note: ld.lld: error: /tmp/rustcNYKZCS/symbols.o: ABI version 1 is not supported ``` Which is correct because ld.lld doesn't support ELFv1 ABI.
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.This PR will be rebased after #142598 has been accepted as the only difference between these PRs is that this one contains user-facing changes which requires a sign-off.
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.