[ELF] Fix edge condition in thunk offset calculation

[smeenai]

Summary

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.

[smeenai]

Fold inputs into test case

[smeenai]

Correct comment

[peter.smith]

Thanks for spotting that, and my apologies for the mistake, I agree the offset needs to be from the add ip, ip, pc, and shouldn't vary between the movw and movt that construct the offset. Looks good to me, but will be worth waiting to see if Rui or Rafael have any comments.

[smeenai]

Gah, I added the wrong account as a reviewer. Thanks for taking a look though, @peter.smith!

[smeenai]

I'm gonna go ahead and commit this, since it's quite small and self-contained, and since Peter wrote the original code and is okay with the change. I can address any post-commit comments in a follow-up.

[peter.smith]

In D38112#877912, @smeenai wrote:

Gah, I added the wrong account as a reviewer. Thanks for taking a look though, @peter.smith!

No problem, they are both my accounts, I'm assigned to Linaro so all my OSS work needs to go through the Linaro account.

lld/trunk/ELF/Thunks.cpp

 bool ThumbV7ABSLongThunk::isCompatibleWith(uint32_t RelocType) const {
   // ARM branch relocations can't use BLX
   return RelocType != R_ARM_JUMP24 && RelocType != R_ARM_PC24 &&
          RelocType != R_ARM_PLT32;
 }
 
 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) {
   ThunkSym = addSyntheticLocal(
       Saver.save("__ARMV7PILongThunk_" + Destination.getName()), STT_FUNC,
       Offset, size(), &IS);
   addSyntheticLocal("$a", STT_NOTYPE, Offset, 0, &IS);
 }
 
 bool ARMV7PILongThunk::isCompatibleWith(uint32_t RelocType) const {
   // Thumb branch relocations can't use BLX
   return RelocType != R_ARM_THM_JUMP19 && RelocType != R_ARM_THM_JUMP24;
 }
 
 void ThumbV7PILongThunk::writeTo(uint8_t *Buf, ThunkSection &IS) const {
   const uint8_t Data[] = {
       0x4f, 0xf6, 0xf4, 0x7c, // P:  movw ip,:lower16:S - (P + (L1-P) + 4)
-      0xc0, 0xf2, 0x00, 0x0c, //     movt ip,:upper16:S - (P + (L1-P+4) + 4)
+      0xc0, 0xf2, 0x00, 0x0c, //     movt ip,:upper16:S - (P + (L1-P) + 4)
       0xfc, 0x44,             // L1: add  r12, pc
       0x60, 0x47,             //     bx   r12
   };
   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) {
   ThunkSym = addSyntheticLocal(
       Saver.save("__ThumbV7PILongThunk_" + Destination.getName()), STT_FUNC,
       Offset | 0x1, size(), &IS);
   addSyntheticLocal("$t", STT_NOTYPE, Offset, 0, &IS);
 }

lld/trunk/test/ELF/arm-thunk-edgecase.s

+// REQUIRES: arm
+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.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 %t.o -o %tarm_to_thumb.so
+// RUN: ld.lld -shared -Bsymbolic -script %tthumb_to_arm.script %t.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
+
+.syntax unified
+
+.arm
+.section .text_armfunc, "ax", %progbits
+.globl armfunc
+armfunc:
+  b thumbfunc
+
+.thumb
+.section .text_thumbfunc, "ax", %progbits
+.globl thumbfunc
+.thumb_func
+thumbfunc:
+  b.w armfunc
+
+// 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