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

[aaelf64] Define GOT-Relative data relocation #223

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

PiJoules
Copy link
Contributor

This introduces a new relocation R\_<CLS>\_GOTPCREL32 which follows the existing wording to “R__GOTREL32”, but instead evaluates to the 32-bit offset between a GOT entry for a given symbol and the current location where the relocation is applied, so its equation would be “G(GDAT(S+A))- P”.

Copy link
Contributor

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change text looks good to me, the CI is complaining about the table formatting. I've made a suggestion for how to correct that.

aaelf64/aaelf64.rst Outdated Show resolved Hide resolved
This introduces a new relocation `R\_<CLS>\_GOTPCREL32` which follows the
existing wording to “R\_<CLS>\_GOTREL32”, but instead evaluates to the
32-bit offset between a GOT entry for a given symbol and the current location
where the relocation is applied, so its equation would be “G(GDAT(S+A))- P”.
Copy link
Contributor

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, it passes CI now.

I've marked this as approved as we have got spare relocation codes and it does help make relative vtables be more efficient on AArch64.

Will wait for a colleague to double check and merge.

@mmalcomson mmalcomson merged commit 72df8b1 into ARM-software:main Oct 10, 2023
1 check passed
@mmalcomson
Copy link
Contributor

LGTM as well -- merging as I'm the second to look at it.

@MaskRay
Copy link
Contributor

MaskRay commented Nov 29, 2023

Thanks for the addition. I think another advantage introducing the data relocation is that: -shared -fpic -fexperimental-relative-c++-abi-vtables builds will not create PLT entries for functions in the vtables, as long as they are not directly called.

PiJoules added a commit to PiJoules/llvm-project that referenced this pull request Jan 10, 2024
This is the follopw implementation to
ARM-software/abi-aa#223 that supports this
relocation in llvm and lld.
PiJoules added a commit to llvm/llvm-project that referenced this pull request Jan 10, 2024
This is the follopw implementation to
ARM-software/abi-aa#223 that supports this
relocation in llvm and lld.
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This is the follopw implementation to
ARM-software/abi-aa#223 that supports this
relocation in llvm and lld.
@smeenai
Copy link
Contributor

smeenai commented Feb 15, 2024

I don't understand the equation for this relocation G(GDAT(S+A)) - P. Section 5.7.33 Relocation operations says:

GDAT(S+A) represents a pointer-sized entry in the GOT for address S+A. The entry will be relocated at run time
with relocation R_<CLS>_GLOB_DAT(S+A)

Based on the LLD implementation of this relocation though (llvm/llvm-project#72584), it seems the addend is applied to the place of the relocation, not the GOT entry. See the added test case for what I mean. The relocations in the object file are:

$ bin/llvm-readelf -rW tools/lld/test/ELF/Output/aarch64-reloc-gotpcrel32.s.tmp.o

Relocation section '.rela.data' at offset 0xd0 contains 5 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000000  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar + 0
0000000000000004  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar + 4
0000000000000008  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar - 4
000000000000000c  000000040000013b R_AARCH64_GOTPCREL32   0000000000000000 baz + ffffffff
0000000000000010  000000040000013b R_AARCH64_GOTPCREL32   0000000000000000 baz - ffffffff

The relocations in the output SO are:

$ bin/llvm-readelf -rW tools/lld/test/ELF/Output/aarch64-reloc-gotpcrel32.s.tmp.so

Relocation section '.rela.dyn' at offset 0x2c0 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000020398  0000000100000401 R_AARCH64_GLOB_DAT     0000000000000000 baz + 0
0000000000020390  0000000200000401 R_AARCH64_GLOB_DAT     00000000000303a0 bar + 0

So the addend isn't propagated to the dynamic relocations, which GDAT(S+A) would imply in my reading. Instead, I believe the equation should be:

G(GDAT(S)) + A - P

This also better matches R_X86_64_GOTPCREL, which is defined as G + GOT + A - P, where G is the GOT offset for the symbol and GOT is the GOT address. I belive R_ARM_GOT_PREL is the AArch32 equivalent, and that's also defined as GOT(S) + A - P, where GOT(S) is the address of the GOT entry for S. Am I missing something?

@PiJoules
Copy link
Contributor Author

I don't understand the equation for this relocation G(GDAT(S+A)) - P. Section 5.7.33 Relocation operations says:

GDAT(S+A) represents a pointer-sized entry in the GOT for address S+A. The entry will be relocated at run time
with relocation R_<CLS>_GLOB_DAT(S+A)

Based on the LLD implementation of this relocation though (llvm/llvm-project#72584), it seems the addend is applied to the place of the relocation, not the GOT entry. See the added test case for what I mean. The relocations in the object file are:

$ bin/llvm-readelf -rW tools/lld/test/ELF/Output/aarch64-reloc-gotpcrel32.s.tmp.o

Relocation section '.rela.data' at offset 0xd0 contains 5 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000000  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar + 0
0000000000000004  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar + 4
0000000000000008  000000020000013b R_AARCH64_GOTPCREL32   0000000000000000 bar - 4
000000000000000c  000000040000013b R_AARCH64_GOTPCREL32   0000000000000000 baz + ffffffff
0000000000000010  000000040000013b R_AARCH64_GOTPCREL32   0000000000000000 baz - ffffffff

The relocations in the output SO are:

$ bin/llvm-readelf -rW tools/lld/test/ELF/Output/aarch64-reloc-gotpcrel32.s.tmp.so

Relocation section '.rela.dyn' at offset 0x2c0 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000020398  0000000100000401 R_AARCH64_GLOB_DAT     0000000000000000 baz + 0
0000000000020390  0000000200000401 R_AARCH64_GLOB_DAT     00000000000303a0 bar + 0

So the addend isn't propagated to the dynamic relocations, which GDAT(S+A) would imply in my reading. Instead, I believe the equation should be:

G(GDAT(S)) + A - P

This also better matches R_X86_64_GOTPCREL, which is defined as G + GOT + A - P, where G is the GOT offset for the symbol and GOT is the GOT address. I belive R_ARM_GOT_PREL is the AArch32 equivalent, and that's also defined as GOT(S) + A - P, where GOT(S) is the address of the GOT entry for S. Am I missing something?

No you're correct. The addend should be applied to where the relocation is located, not to symbol we're taking the GOT entry for. The relocation is intended to be similar to R_X86_64_GOTPCREL as you said. I'll send out a followup fix. Sorry for the confusion.

@PiJoules PiJoules deleted the GOTPCREL32 branch February 15, 2024 19:59
@PiJoules
Copy link
Contributor Author

#247

@smeenai
Copy link
Contributor

smeenai commented Feb 15, 2024

Thanks for the confirmation and follow-up!

@smithp35
Copy link
Contributor

Apologies for the delayed response, just come back from vacation.

It is worth mentioning that there is a generic ABI issue with how GOT relative addends are represented in the AArch64 ABI #217 it looks like this instance may fall into that category. To summarise it looks like neither LLD, Mold, Gold or GNU ld implement GDAT(S+A) as written in the spec. Moreover they don't implement it in a consistent way either, with lld doing GDAT(S) + A and GNU ld just ignoring the A.

So far there hasn't been a satisfactory resolution as it's not clear what all the linker's can support. Probably best leave any further comments on #217

I'll leave a comment in #247 too

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 this pull request may close these issues.

None yet

5 participants