[PATCH] D105214: [ARM] RELA relocations for 32bit ARM ignore the addend.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Object/RelocationResolver.cpp:314
 
 static uint64_t resolveARM(uint64_t Type, uint64_t Offset, uint64_t S,
+                           uint64_t LocData, int64_t Addend) {
----------------
wolfgangp wrote:
> peter.smith wrote:
> > MaskRay wrote:
> > > MaskRay wrote:
> > > > The code is written in the assumption that targets preferring RELA always have Addend while targets preferring REL always have LocData.
> > > > 
> > > > Are you trying to add RELA support to ARM? I think it is unpopular but maybe you have specific needs which are not specified.
> > > > 
> > > > No objection, but a comment is needed.
> > > I think adding LocData and Addend is wrong. When RELA is used, LocData (implicit addend) should be ignored.
> > Agreed. The ELF spec says in http://www.sco.com/developers/gabi/latest/ch4.reloc.html
> > "As specified previously, only Elf32_Rela and Elf64_Rela entries contain an explicit addend. Entries of type Elf32_Rel and Elf64_Rel store an implicit addend in the location to be modified. Depending on the processor architecture, one form or the other might be necessary or more convenient. Consequently, an implementation for a particular machine may use one form exclusively or either form depending on context."
> > 
> > "The typical application of an ELF relocation is to determine the referenced symbol value, extract the addend (either from the field to be relocated or from the addend field contained in the relocation record, as appropriate for the type of relocation record), apply the expression implied by the relocation type to the symbol and addend, extract the desired part of the expression result, and place it in the field to be relocated."
> Ah, thanks for pointing that out. It is curious, though, because some RISCV relocations clearly seem to use both addend and location data ([[ https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md | see here ]]). 
> Accordingly, when I zero out LocData for RELA relocations, some RISCV test cases start failing. I may have to restrict zeroing out LocData to ARM platforms, which I don't particularly like.
The special RISC-V relocations are `R_RISCV_SUB*`. They do label differences and the explicit addend is always zero. It can be implemented by "consecutive relocations" but processing the relocation consecutively can also be correct. If anything, it should not be used as examples for other targets because it imposes many restrictions that other targets may not have.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105214/new/

https://reviews.llvm.org/D105214