-
Notifications
You must be signed in to change notification settings - Fork 196
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
[aaelf64] Fix equation for R_AARCH64_GOTPCREL32 #247
Conversation
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.
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 |
I agree that
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.
I support this stance. |
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.
I've submitted #272 which removes A from the GDAT(S + A) relocation operation. |
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 toR_X86_64_GOTPCREL
where the addend is applied to the location of the reloc rather than the symbol.