[PATCH] D38112: [ELF] Fix edge condition in thunk offset calculation

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai created this revision.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.

For ARM thunks, the `movt` half of the relocation was using an incorrect
offset (it was off by 4 bytes). The original intent seems to have been
for the offset to have been relative to the current instruction, in
which case the difference of 4 makes sense. As the code stands, however,
the offset is always calculated relative to the start of the thunk
(`P`), and so the `movw` and `movt` halves should use the same offset.
This requires a very particular offset between the thunk and its target
to be triggered, and it results in the `movt` half of the relocation
being off-by-one.

The tests here use ARM-Thumb interworking thunks, since those are the
only ARM thunks currently implemented. I actually encountered this with
a range extension thunk (having Peter's patches cherry-picked locally),
but the underlying issue is identical.


https://reviews.llvm.org/D38112

Files:
  ELF/Thunks.cpp
  test/ELF/Inputs/armfunc.s
  test/ELF/Inputs/thumbfunc.s
  test/ELF/arm-thunk-edgecase.s


Index: test/ELF/arm-thunk-edgecase.s
===================================================================
--- /dev/null
+++ test/ELF/arm-thunk-edgecase.s
@@ -0,0 +1,23 @@
+// REQUIRES: arm
+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %S/Inputs/armfunc.s -o %tarm.o
+// RUN: llvm-mc -filetype=obj -triple=thumbv7a-none-linux-gnueabi %S/Inputs/thumbfunc.s -o %tthumb.o
+// RUN: echo "SECTIONS { \
+// RUN:           .text_armfunc 0x1000 : { *(.text_armfunc) } \
+// RUN:           .text_thumbfunc 0x11010 : { *(.text_thumbfunc) } \
+// RUN:       }" > %tarm_to_thumb.script
+// RUN: echo "SECTIONS { \
+// RUN:           .text_thumbfunc 0x1000 : { *(.text_thumbfunc) } \
+// RUN:           .text_armfunc 0x1100c : { *(.text_armfunc) } \
+// RUN:       }" > %tthumb_to_arm.script
+// RUN: ld.lld -shared -Bsymbolic -script %tarm_to_thumb.script %tarm.o %tthumb.o -o %tarm_to_thumb.so
+// RUN: ld.lld -shared -Bsymbolic -script %tthumb_to_arm.script %tarm.o %tthumb.o -o %tthumb_to_arm.so
+// RUN: llvm-objdump -triple=armv7a-none-linux-gnueabi -d %tarm_to_thumb.so | FileCheck -check-prefix=ARM-TO-THUMB %s
+// RUN: llvm-objdump -triple=thumbv7a-none-linux-gnueabi -d %tthumb_to_arm.so | FileCheck -check-prefix=THUMB-TO-ARM %s
+
+ARM-TO-THUMB:      __ARMV7PILongThunk_thumbfunc:
+ARM-TO-THUMB-NEXT:     1004:        fd cf 0f e3         movw        r12, #65533
+ARM-TO-THUMB-NEXT:     1008:        00 c0 40 e3         movt        r12, #0
+
+THUMB-TO-ARM:      __ThumbV7PILongThunk_armfunc:
+THUMB-TO-ARM-NEXT:     1004:        4f f6 fc 7c         movw        r12, #65532
+THUMB-TO-ARM-NEXT:     1008:        c0 f2 00 0c         movt        r12, #0
Index: test/ELF/Inputs/thumbfunc.s
===================================================================
--- /dev/null
+++ test/ELF/Inputs/thumbfunc.s
@@ -0,0 +1,8 @@
+.syntax unified
+.text
+.thumb
+.section .text_thumbfunc, "ax", %progbits
+.globl thumbfunc
+.thumb_func
+thumbfunc:
+	b.w	armfunc
Index: test/ELF/Inputs/armfunc.s
===================================================================
--- /dev/null
+++ test/ELF/Inputs/armfunc.s
@@ -0,0 +1,6 @@
+.syntax unified
+.text
+.section .text_armfunc, "ax", %progbits
+.globl armfunc
+armfunc:
+	b	thumbfunc
Index: ELF/Thunks.cpp
===================================================================
--- ELF/Thunks.cpp
+++ ELF/Thunks.cpp
@@ -160,16 +160,17 @@
 
 void ARMV7PILongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
-      0xf0, 0xcf, 0x0f, 0xe3, // P:  movw ip,:lower16:S - (P + (L1-P) +8)
-      0x00, 0xc0, 0x40, 0xe3, //     movt ip,:upper16:S - (P + (L1-P+4) +8)
+      0xf0, 0xcf, 0x0f, 0xe3, // P:  movw ip,:lower16:S - (P + (L1-P) + 8)
+      0x00, 0xc0, 0x40, 0xe3, //     movt ip,:upper16:S - (P + (L1-P) + 8)
       0x0f, 0xc0, 0x8c, 0xe0, // L1: add ip, ip, pc
       0x1c, 0xff, 0x2f, 0xe1, //     bx r12
   };
   uint64_t S = getARMThunkDestVA(Destination);
   uint64_t P = ThunkSym->getVA();
+  uint64_t Offset = S - P - 16;
   memcpy(Buf, Data, sizeof(Data));
-  Target->relocateOne(Buf, R_ARM_MOVW_PREL_NC, S - P - 16);
-  Target->relocateOne(Buf + 4, R_ARM_MOVT_PREL, S - P - 12);
+  Target->relocateOne(Buf, R_ARM_MOVW_PREL_NC, Offset);
+  Target->relocateOne(Buf + 4, R_ARM_MOVT_PREL, Offset);
 }
 
 void ARMV7PILongThunk::addSymbols(ThunkSection &IS) {
@@ -193,9 +194,10 @@
   };
   uint64_t S = getARMThunkDestVA(Destination);
   uint64_t P = ThunkSym->getVA() & ~0x1;
+  uint64_t Offset = S - P - 12;
   memcpy(Buf, Data, sizeof(Data));
-  Target->relocateOne(Buf, R_ARM_THM_MOVW_PREL_NC, S - P - 12);
-  Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_PREL, S - P - 8);
+  Target->relocateOne(Buf, R_ARM_THM_MOVW_PREL_NC, Offset);
+  Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_PREL, Offset);
 }
 
 void ThumbV7PILongThunk::addSymbols(ThunkSection &IS) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38112.116118.patch
Type: text/x-patch
Size: 3838 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170921/84c9bdfb/attachment.bin>