[LLD][ELF][ARM] Fix case of patched unrelocated BLX

[peter.smith]

Summary

There are a couple of problems with the code to patch unrelocated BLX instructions:

• The calculation of the PC needs to take into account the alignment of the instruction. The Thumb BLX uses alignDown(PC, 4) for the source address.
• The calculation of the PC bias is hard-coded to 4 which works for Thumb, but when there is a BLX the branch will be in Arm state so it needs an 8 byte PC bias.

No asssembler generates an unrelocated BLX instruction so these problems do not affect real world programs. However we should still fix them.

Depends on D104701 for objdump output of BLX

[ikudrin]

LGTM

lld/ELF/ARMErrataFix.cpp

static uint64_t getThumbDestAddr(uint64_t sourceAddr, uint32_t instr) {
   write16le(buf + 2, instr & 0x0000ffff);
   int64_t offset;
   if (isBcc(instr))
     offset = target->getImplicitAddend(buf, R_ARM_THM_JUMP19);
   else if (isB(instr))
     offset = target->getImplicitAddend(buf, R_ARM_THM_JUMP24);
   else
     offset = target->getImplicitAddend(buf, R_ARM_THM_CALL);
+  // A BLX instruction from Thumb to Arm may have an address that is
+  // not 4-byte aligned. As Arm instructions are always 4-byte aligned
+  // the instruction is calculated (from Arm ARM):
+  // targetAddress = Align(PC, 4) + imm32
+  // where
+  //   Align(x, y) = y * (x Div y)
+  // which corresponds to alignDown.
+  if (isBLX(instr))
+    sourceAddr = alignDown(sourceAddr, 4);
   return sourceAddr + offset + 4;
 }
 
 void Patch657417Section::writeTo(uint8_t *buf) {
   // The base instruction of the patch is always a 32-bit unconditional branch.
   if (isARM)
     write32le(buf, 0xea000000);
   else
     write32le(buf, 0x9000f000);
   // If we have a relocation then apply it.
   if (!relocations.empty()) {
     relocateAlloc(buf, buf + getSize());
     return;
   }
 
   // If we don't have a relocation then we must calculate and write the offset
   // ourselves.
   // Get the destination offset from the addend in the branch instruction.
   // We cannot use the instruction in the patchee section as this will have
   // been altered to point to us!
   uint64_t s = getThumbDestAddr(getBranchAddr(), instr);
-  uint64_t p = getVA(4);
+  // A BLX changes the state of the branch in the patch to Arm state, which
+  // has a PC Bias of 8, whereas in all other cases the branch is in Thumb
+  // state with a PC Bias of 4.
+  uint64_t pcBias = isBLX(instr) ? 8 : 4;
+  uint64_t p = getVA(pcBias);
   target->relocateNoSym(buf, isARM ? R_ARM_JUMP24 : R_ARM_THM_JUMP24, s - p);
 }
 
 // Given a branch instruction spanning two 4KiB regions, at offset off from the
 // start of isec, return true if the destination of the branch is within the
 // first of the two 4Kib regions.
 static bool branchDestInFirstRegion(const InputSection *isec, uint64_t off,
                                     uint32_t instr, const Relocation *r) {

lld/test/ELF/arm-fix-cortex-a8-blx.s

  nop.w
 /// Encoding for blx _start. Use .inst.n directives to avoid a relocation.
  .inst.n 0xf7ff
  .inst.n 0xe800
 
 // CHECK-PATCH:         21ffa:          nop.w
 // CHECK-PATCH-NEXT:    21ffe:          blx     0x22004 <__CortexA8657417_21FFE>
 // CHECK-PATCH:      00022004 <__CortexA8657417_21FFE>:
-// CHECK-PATCH-NEXT:    22004:          b       0x21004 <{{.+}}> @ imm = #-4104
+// CHECK-PATCH-NEXT:    22004:          b       0x21000 <_start>