[PATCH] D104905: [LLD][ELF][ARM] Fix case of patched unrelocated BLX

[Peter Smith via Phabricator via llvm-commits]

peter.smith created this revision.
peter.smith added reviewers: ikudrin, MaskRay, grimar.
Herald added subscribers: danielkiss, kristof.beyls, arichardson, emaste.
peter.smith requested review of this revision.

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 <https://reviews.llvm.org/D104701> for objdump output of BLX


https://reviews.llvm.org/D104905

Files:
  lld/ELF/ARMErrataFix.cpp
  lld/test/ELF/arm-fix-cortex-a8-blx.s


Index: lld/test/ELF/arm-fix-cortex-a8-blx.s
===================================================================
--- lld/test/ELF/arm-fix-cortex-a8-blx.s
+++ lld/test/ELF/arm-fix-cortex-a8-blx.s
@@ -30,4 +30,4 @@
 // 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>
Index: lld/ELF/ARMErrataFix.cpp
===================================================================
--- lld/ELF/ARMErrataFix.cpp
+++ lld/ELF/ARMErrataFix.cpp
@@ -164,6 +164,15 @@
     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;
 }
 
@@ -185,7 +194,11 @@
   // 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);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104905.354458.patch
Type: text/x-patch
Size: 1847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210625/8325db41/attachment.bin>