-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
ldelf: aarch32: Accept ELFOSABI_ARM as OS ABI #6605
Conversation
Looks like the CI build failure isn't related to this PR. |
ldelf/ta_elf.c
Outdated
} | ||
|
||
if (ehdr->e_ident[EI_OSABI] == ELFOSABI_ARM) { | ||
if ((ehdr->e_flags & EF_ARM_ABIMASK) != EF_ARM_ABI_UNKNOWN) |
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 sucks somewhat. I guess Rust is going to use floating point if it feels like it, but we'll never know.
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.
We should perhaps accept both EF_ARM_ABI_UNKNOWN
and EF_ARM_ABI_V5
here? We do support EABIv5 so let's be prepared in case Rust sets the flag.
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.
Binutils-gdb reference here [1] indicates that there is 1:1 relation among ELFOSABI_ARM
and EF_ARM_ABI_UNKNOWN
in flags field. However, for floating point case I agree corresponding bit EF_ARM_ABI_FLOAT_HARD
should be checked here as well for Rust case. I will make that common instead as earlier.
[1] https://github.com/bminor/binutils-gdb/blob/master/bfd/elf32-arm.c#L17661
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.
BTW, Linux arm32 ELF parser does the similar floating point checking here: https://github.com/torvalds/linux/blob/master/arch/arm/kernel/elf.c#L26
|
|
Thanks, tags applied. |
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.
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Rust TAs built for no-std mode targeting 32-bit Arm architecture use ELFOSABI_ARM as the OS ABI within ELF header. So allow ldelf to load those Rust TAs built for 32-bit Arm. Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (vexpress-qemu_armv8a) Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Addressed comment and tag applied, thanks. |
Rust TAs built for no-std mode targeting 32-bit Arm architecture use ELFOSABI_ARM as the OS ABI within ELF header. So allow ldelf to load those Rust TAs built for 32-bit Arm.