Skip to content
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

Mandatory/optional merging of build attributes #352

Closed
MaskRay opened this issue Nov 23, 2022 · 5 comments · Fixed by #360
Closed

Mandatory/optional merging of build attributes #352

MaskRay opened this issue Nov 23, 2022 · 5 comments · Fixed by #360

Comments

@MaskRay
Copy link
Collaborator

MaskRay commented Nov 23, 2022

I am concerned with the use of build attributes but I think it's useful noting down a GNU ld arm behavior regarding to .ARM.attributes merging.

For a tag number tag, if tag%128 < 64, the attribute is considered mandatory and GNU ld unaware of it reports an error (which will change the exit status to 1); otherwise, (tag%128 >= 64), the attribute is considered optional: there is a warning but the link will succeed (--fatal-warnings can upgrade the warning to an error).

% cat a.s
.eabi_attribute 88, 2
.eabi_attribute 128, 2
% arm-linux-gnueabi-gcc -nostdlib a.s a.s
/usr/lib/gcc-cross/arm-linux-gnueabi/12/../../../../arm-linux-gnueabi/bin/ld: warning: a.out: unknown EABI object attribute 88
/usr/lib/gcc-cross/arm-linux-gnueabi/12/../../../../arm-linux-gnueabi/bin/ld: a.out: unknown mandatory EABI object attribute 128
/usr/lib/gcc-cross/arm-linux-gnueabi/12/../../../../arm-linux-gnueabi/bin/ld: failed to merge target specific data of file /tmp/ccfJwSI8.o
/usr/lib/gcc-cross/arm-linux-gnueabi/12/../../../../arm-linux-gnueabi/bin/ld: warning: cannot find entry symbol _start; defaulting to 0000000000000178
collect2: error: ld returned 1 exit status

For RISC-V, it's perhaps useful specifying whether it's necessary to emit a warning/error when all input files with an attribute have the same value. Personally I think that's not useful. When GNU ld sees an unknown attribute only in one input, it doesn't give a warning/error.

@jrtc27
Copy link
Collaborator

jrtc27 commented Nov 23, 2022

64 seems like an arbitrary limit. If we want to do this it should probably follow the odd/even approach and use something like bit 1 to indicate whether it's optional (with special cases for existing attributes that don't fit that).

@MaskRay
Copy link
Collaborator Author

MaskRay commented Nov 23, 2022

.ARM.attributes uses modular arithmetic. I originally used tag < 64 (mod 128). Now I changed it to if tag%128 < 64 to make it clearer. So there is no arbitrary lower limit like 64.

@jrtc27
Copy link
Collaborator

jrtc27 commented Nov 23, 2022

Ah I missed the (mod 128), that works (and effectively is equivalent, just uses bit 6 instead of bit 1)

@kito-cheng
Copy link
Collaborator

I like the general idea, it sounds like more easier to linker to handle unknown attributes :)

@MaskRay MaskRay changed the title Mandatory/optional build attributes Mandatory/optional merging of build attributes Dec 2, 2022
MaskRay added a commit to llvm/llvm-project that referenced this issue Dec 8, 2022
Currently we take the first SHT_RISCV_ATTRIBUTES (.riscv.attributes) as the
output. If we link an object without an extension with an object with the
extension, the output Tag_RISCV_arch may not contain the extension and some
tools like objdump -d will not decode the related instructions.

This patch implements
Tag_RISCV_stack_align/Tag_RISCV_arch/Tag_RISCV_unaligned_access merge as
specified by
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#attributes

For the deprecated Tag_RISCV_priv_spec{,_minor,_revision}, dump the attribute to
the output iff all input agree on the value. This is different from GNU ld but
our simple approach should be ok for deprecated tags.

`RISCVAttributeParser::handler` currently warns about unknown tags. This
behavior is retained. In GNU ld arm, tags >= 64 (mod 128) are ignored with a
warning. If RISC-V ever wants to do something similar
(riscv-non-isa/riscv-elf-psabi-doc#352), consider
documenting it in the psABI and changing RISCVAttributeParser.

Like GNU ld, zero value integer attributes and empty string attributes are not
dumped to the output.

Reviewed By: asb, kito-cheng

Differential Revision: https://reviews.llvm.org/D138550
kito-cheng added a commit that referenced this issue Jan 30, 2023
kito-cheng added a commit that referenced this issue Jan 30, 2023
It's mandatory when (tag number % 128) < 64
and optional when (tag number % 128) >= 64.

Fix #352
@kito-cheng
Copy link
Collaborator

Created PR for this: #360

kito-cheng added a commit that referenced this issue Feb 20, 2023
It's mandatory when (tag number % 128) < 64
and optional when (tag number % 128) >= 64.

Fix #352
ilovepi pushed a commit to ilovepi/riscv-elf-psabi-doc that referenced this issue Mar 21, 2023
It's mandatory when (tag number % 128) < 64
and optional when (tag number % 128) >= 64.

Fix riscv-non-isa#352
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 6, 2024
Currently we take the first SHT_RISCV_ATTRIBUTES (.riscv.attributes) as the
output. If we link an object without an extension with an object with the
extension, the output Tag_RISCV_arch may not contain the extension and some
tools like objdump -d will not decode the related instructions.

This patch implements
Tag_RISCV_stack_align/Tag_RISCV_arch/Tag_RISCV_unaligned_access merge as
specified by
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#attributes

For the deprecated Tag_RISCV_priv_spec{,_minor,_revision}, dump the attribute to
the output iff all input agree on the value. This is different from GNU ld but
our simple approach should be ok for deprecated tags.

`RISCVAttributeParser::handler` currently warns about unknown tags. This
behavior is retained. In GNU ld arm, tags >= 64 (mod 128) are ignored with a
warning. If RISC-V ever wants to do something similar
(riscv-non-isa/riscv-elf-psabi-doc#352), consider
documenting it in the psABI and changing RISCVAttributeParser.

Like GNU ld, zero value integer attributes and empty string attributes are not
dumped to the output.

Reviewed By: asb, kito-cheng

Differential Revision: https://reviews.llvm.org/D138550
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants