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] Fix equation for R_AARCH64_GOTPCREL32 #247

Conversation

PiJoules
Copy link
Contributor

The current wording for this reloc is G(GDAT(S+A))-P which means the addend is applied to the symbol we're taking the GOT entry for. What we actually want (and what's implemented in lld) is something similar to R_X86_64_GOTPCREL where the addend is applied to the location of the reloc rather than the symbol.

The current wording for this reloc is `G(GDAT(S+A))-P` which means the
addend is applied to the symbol we're taking the GOT entry for. What we
actually want (and what's implemented in lld) is something similar to
`R_X86_64_GOTPCREL` where the addend is applied to the location of the
reloc rather than the symbol.
@smithp35
Copy link
Contributor

As mentioned in #223 there is a generic ABI issue with how GDAT(S+A) is written vs how it is implemented #217 . No linker implements it as written, and the implementations are not consistent. The practical conclusion is that the only useful addend value for most of these relocations is 0 as that is the one value that all agree on.

For this specific relocation, it is only implemented by LLD so we have some scope to make the proposed change to match an implementation.

As an open question, can you foresee any occasion when the compiler would use a non zero addend using this relocation? If the answer is yes then we definitely need to use this change. If the answer is no, then we may be better off forbidding non zero addends as proposed as a resolution for #217

@MaskRay
Copy link
Contributor

MaskRay commented Feb 20, 2024

I agree that GDAT(S+A) is certainly not ideal. Other architectures don't do this, and typical linker implementations associate a single GOT entry to a symbol. Support multiple GOT entries for a symbol or supporting a GOT entry for an address requires more code and typically not good for performance.

As an open question, can you foresee any occasion when the compiler would use a non zero addend using this relocation? If the answer is yes then we definitely need to use this change. If the answer is no, then we may be better off forbidding non zero addends as proposed as a resolution for #217

There is a curious R_X86_64_GOTPCREL use with a non-zero addend: https://sourceware.org/bugzilla/show_bug.cgi?id=26939

   void x();
   int foo() { return (long)x >> 32; }

If ABI arbiters' stance is that this does not justify allowing a non-zero addend, I think it will be fairly reasonable to disallow addend!=0.

If the answer is no, then we may be better off forbidding non zero addends as proposed as a resolution for #217

I support this stance.

smithp35 added a commit to smithp35/abi-aa that referenced this pull request Jul 2, 2024
The GDAT(S + A) relocation operation requires a static linker to
create a GOT entry for (S + A). Requiring at least one GOT entry
for each unique tuple (S, A). Unfortunately no known static linker
has implemented this correctly, with one of two forms being
implemented instead:
* GDAT(S) with the addend ignored.
* GDAT(S) + A with a single GOT entry per S, and A added to the
  value of GDAT(S).
These implementations are correct and consistent only for an
addend (A) of zero.

No known compiler uses non-zero addends in relocations that use
the GDAT(S+A) operation, although it is possible to generate
them using assembly language.

This change synchronizes the ABI with the behavior of existing
static linker implementations. The benefit of permitting code
generators [*] to use a non zero addend in GDAT(S + A) is judged
to be lower than implementing GDAT(S + A) correctly in existing
static linkers, many of which assume that there is a single
GOT entry per unique symbol S.

It is QoI whether a static linker gives an error if a non zero
addend is used for a relocation that uses the GDAT(S) operation.

Fixes ARM-software#217
Also resolves ARM-software#247

[*] The most common use case for a non-zero addend is in
constructing a C++ object with a vtable. The first two entries
in the vtable are the offset to top and a pointer to RTTI, the
vtable pointer in the object starts at offset 0x10. This offset
can be encoded in the relocation addend. We would save an add
instruction for each construction of a C++ object with a vtable
if addends were permitted.
@smithp35
Copy link
Contributor

smithp35 commented Jul 2, 2024

I've submitted #272 which removes A from the GDAT(S + A) relocation operation.

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.

3 participants