[PATCH] D117734: [ELF] Fix the branch range computation when reusing a thunk

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: ikudrin, peter.smith.
Herald added subscribers: kristof.beyls, arichardson, emaste.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Notation: dst is `t->getThunkTargetSym()->getVA()`

On AArch64, when `src-0x8000000-r_addend <= dst < src-0x8000000`, the condition
`target->inBranchRange(rel.type, src, rel.sym->getVA(rel.addend))` may
incorrectly consider a thunk reusable.
`rel.addend = -getPCBias(rel.type)` resets the addend to 0 for AArch64/PPC
and the zero addend is used by `rel.sym->getVA(rel.addend)` to check
out-of-range relocations.

See the test for a case this computation is wrong:
`error: a.o:(.text_high+0x4): relocation R_AARCH64_JUMP26 out of range: -134217732 is not in [-134217728, 134217727]`
I have seen a real world case with r_addend=19960.

I'd like to use `t->getThunkTargetSym()->getVA(getPCBias(rel.type))` but two
arm-* tests would fail. I think that is the remaining getPCBias issue after
D97550 <https://reviews.llvm.org/D97550>. Not fixing it is probably fine since the failing scenario seems
extremely corner-case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117734

Files:
  lld/ELF/Relocations.cpp
  lld/test/ELF/aarch64-thunk-reuse.s


Index: lld/test/ELF/aarch64-thunk-reuse.s
===================================================================
--- /dev/null
+++ lld/test/ELF/aarch64-thunk-reuse.s
@@ -0,0 +1,49 @@
+# REQUIRES: aarch64
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/a.s -o %t/a.o
+# RUN: ld.lld -pie -T %t/lds %t/a.o -o %t/a
+# RUN: llvm-objdump -d --no-show-raw-insn %t/a | FileCheck %s
+
+## We create a thunk for dest.
+# CHECK-LABEL: <mid>:
+# CHECK-NEXT:   8010008:       b       0x801000c <__AArch64ADRPThunk_>
+# CHECK-EMPTY:
+# CHECK-NEXT:  <__AArch64ADRPThunk_>:
+# CHECK-NEXT:   801000c:       adrp    x16, 0x10000
+# CHECK-NEXT:                  add     x16, x16, #4
+# CHECK-NEXT:                  br      x16
+
+## The first instruction can reuse the thunk but the second can't.
+## If we reuse the thunk for b, we will get an "out of range" error.
+# CHECK-LABEL: <high>:
+# CHECK-NEXT:  1001000c:       bl      0x801000c <__AArch64ADRPThunk_>
+# CHECK-NEXT:                  b       0x10010014 <__AArch64ADRPThunk_>
+# CHECK-EMPTY:
+# CHECK-NEXT:  <__AArch64ADRPThunk_>:
+# CHECK-NEXT:  10010014:       adrp    x16, 0x10000
+# CHECK-NEXT:                  add     x16, x16, #4
+# CHECK-NEXT:                  br      x16
+
+#--- a.s
+.section .text_low, "ax", %progbits
+.globl _start
+_start:
+  nop
+dest:
+  ret
+
+.section .text_mid, "ax", %progbits
+mid:
+  b dest
+
+.section .text_high, "ax", %progbits
+high:
+  bl dest
+  b dest
+
+#--- lds
+SECTIONS {
+  .text_low 0x10000: { *(.text_low) }
+  .text_mid 0x8010008 : { *(.text_mid) }
+  .text_high 0x1001000c : { *(.text_high) }
+}
Index: lld/ELF/Relocations.cpp
===================================================================
--- lld/ELF/Relocations.cpp
+++ lld/ELF/Relocations.cpp
@@ -2060,8 +2060,7 @@
   for (Thunk *t : *thunkVec)
     if (isThunkSectionCompatible(isec, t->getThunkTargetSym()->section) &&
         t->isCompatibleWith(*isec, rel) &&
-        target->inBranchRange(rel.type, src,
-                              t->getThunkTargetSym()->getVA(rel.addend)))
+        target->inBranchRange(rel.type, src, t->getThunkTargetSym()->getVA()))
       return std::make_pair(t, false);
 
   // No existing compatible Thunk in range, create a new one


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117734.401449.patch
Type: text/x-patch
Size: 2260 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220120/10d2e4ab/attachment.bin>