[PATCH] D81184: [WIP][ELF][AArch64] Implement the logic for handling R_AARCH64_PLT32

Thu Jun 4 15:30:23 PDT 2020

psmith added a comment.

In D81184#2074619 <https://reviews.llvm.org/D81184#2074619>, @MaskRay wrote:

> I'm not sure unconditionally changing `.word foo - .` to use R_AARCH64_PLT32 is the appropriate change. Note that x86 `R_X86_64_PC32` doesn't do this.
>  `.word foo at plt - .` would probably make more sense. For one reason, R_AARCH64_PLT32 is not supported by binutils, unconditionally using it can break -fno-integrated-as.
>  There is also no need to special case on visiblity: `(ElfSym.getVisibility() == llvm::ELF::STV_HIDDEN) ? R_CLS(PREL32) : R_CLS(PLT32);`, you can entirely defer the handling to the linker
>
> Additionally, the lld changes should probably be moved to a separate patch. It is not tightly coupled with MC. The tests are also inadequate - you'd need a range extension thunk test. I can do this part if you don't mind.

I agree with MaskRay that we should not unconditionally change .word foo - . to use R_AARCH64_PLT32. I think the suggestion to use foo at plt - . is a good one as I think this is how the X86 version is used. I noticed your LLVM-DEV message on assertion failures. I've not got a suggestion at the moment it is late at night right now; I'd expect the assertion is there as we wouldn't expect @plt to be used without a relocation like R_AARCH64_PLT32.

The range thunks for R_AARCH64_PLT32 will need a large distance to the symbol destination as the range for the relocation is +/- 2^31, I suggest using a linker script to avoid creating a gigantic file. Will also be useful to test that R_AARCH64_PLT32 can generate a PLT entry.

================
Comment at: lld/ELF/Arch/AArch64.cpp:263
 bool AArch64::inBranchRange(RelType type, uint64_t src, uint64_t dst) const {
   if (type != R_AARCH64_CALL26 && type != R_AARCH64_JUMP26)
     return true;
----------------
This will need updating for R_AARCH64_PLT32, the range is larger as well, from memory it is a signed 32-bit value.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81184