[PATCH] D70637: [ELF][AArch64] Support R_AARCH64_{CALL26,JUMP26} range extension thunks with addends

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Will look at the code in detail this afternoon.

> On REL targets (ARM, MIPS), jumps/calls are not represented as STT_SECTION + non-zero addend (see MCELFObjectTargetWriter::needsRelocateWithSymbol), so they don't need this feature, but we need to make sure this patch does not affect them.

On Arm using a STT_FUNC symbol is required to get an interworking thunk (LLD isn't strictly compliant here, see appendix) so using a branch to STT_SECTION symbol + offset may not work reliably unless the caller knows the destination execution state and can do the interworking at the call site. I've seen MIPS use the bottom bit of the symbol STO_MIPS_MICROMIPS but I don't know the details.

It is possible to write in assembler

  bl sym + 8

Where the +8 will be added to the PC-bias correction (-8 in Arm and -4 in Thumb) to give an immediate of 0 in Arm or +4 in Thumb. So this is at least possible to encounter in Thumb. As far as I know compilers won't generate this though. This could be supported on Arm at least by reading the implicit addend rather than assuming it is either -8 in Arm or -4 in Thumb as we do now.

> On ARM, the compensation of implicit addends is done in ARM::needsThunk. This makes it tricky not to affect ARM. I feel that the code can be generalized (see config->isRela introduced in the patch) if we reorganize the code (ARM::needsThunk).

Can you elaborate? My understanding is that needsThunk() and inBranchRange() assume all Arm branches to have an immediate (which becomes the relocation addend) of -8 and Thumb branches to have -4. This cancels the PC-bias of 8-bytes in Arm and 4-bytes in Thumb.

This is my current thinking. If the addend from the branch immediate, whether read from the instruction, or assumed to be -8 for an Arm branch or -4 for a Thumb branch, is passed in to needsThunk() I think that we could do:

  src = branchAddr +  (Is Thumb branch then + 4, else +8) 
  dst = (expr == R_PLT_PC) ? s.getPltVA(addend) : s.getVA(addend);

The code sequence:

  // PC at Src is 2 instructions ahead, immediate of branch is signed
  if (src > dst)
    range -= 2 * instrSize;
  else
    range += instrSize;
  
  if ((dst & 0x1) == 0)
    // Destination is ARM, if ARM caller then Src is already 4-byte aligned.
    // If Thumb Caller (BLX) the Src address has bottom 2 bits cleared to ensure
    // destination will be 4 byte aligned.
    src &= ~0x3;
  else
    // Bit 0 == 1 denotes Thumb state, it is not part of the range
    dst &= ~0x1;

I think should be unaffected. This is accounting for the implicit PC-bias and the alignment requirements when calculating what immediates are encodable.

Appendix/Aside:
Strictly speaking Arm/Thumb interworking is only done for STT_FUNC symbols. LLD does this for all symbols as we have to choose between a BL or BLX in relocateOne which doesn't have access to Symbol to check the type. There are also some corner case rules to do with how the thumb bit is processed in relocations that we don't follow. Strictly, for a STT_FUNC the value of the symbol should strip out the Thumb bit before adding the addend, the Thumb bit is then orred in. For example some relocations are defined as ((S + A) | T) - P. This can matter when the addend is 1 modulo 2, which in practice never happens outside of assembly when someone is trying to manually interwork. I would like to fix this but it will mean adding Symbol, or at least the result of S.isFunc() as a parameter to relocateOne(). If that doesn't sound too objectionable I'd be happy to work on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70637