[ARM][LLD] ARM TLS shouldn't use relaxations

[peter.smith]

Summary

Disable all relaxations for ARM TLS

The ARM TLS relocations are placed on literal data and not the code-sequence, it is therefore not possible to implement the relaxTls* functions. The Mips Target also does not do TLS relaxation and has a separate handleMipsTlsRelocation() function to keep handleTlsRelocation() clean from target specific hooks. I've chosen to rename handleMipsTlsRelocation() to handleNoRelaxTlsRelocation() so that it can be used for both ARM and Mips.

Unfortunately there are some differences in ARM and Mips target handling as there are some cases where Mips can omit a dynamic relocation where ARM cannot. I tried some experiments to see if I could get away without these changes, but they caused segfaults at runtime. The Mips code should be logically identical to what it was before.

There is still some work to be done for ARM TLS and static linking, I'll address this separately as it is a smaller use case.

The 4 test cases will fail assertions without these changes.

This should address PR30218 (https://llvm.org/bugs/show_bug.cgi?id=30218)

[emaste]

ELF/Relocations.cpp

88 ↗

(On Diff #72160)

typo "intothe"

[ruiu]

ELF/Relocations.cpp

89 ↗

(On Diff #72160)

Please also briefly mention that the difference between ARM and MIPS in this comment.

[ed]

Not that it helps much, but with this patch I can at least get all of the packages for CloudABI to build on ARMv6. I can't test whether the resulting binaries work yet, as I'm at a conference. I will be able to test this for you on Monday.

[peter.smith]

Updated diff to fix typo and explain the differences between ARM and Mips.

I've put the comment as a FIXME as some more work is needed to support static linking. In effect we need to hard-code the module index (1 for applications) in the GOT rather than getting the dynamic linker to do it for us. I think that this should be done in a separate patch as part of supporting and testing static linking.

Ed, I think ARMv6 for LLD won't work very well as there are some assumptions that ARMv7 instructions are available in interworking Thunks and in the encoding of Thumb2 branches. If you can I would recommend ARMv7 as a baseline if using LLD. I would like to add this support at some point, but it will need some thought as to the best way of doing it.

I'll be at a conference myself next week, but I should have some spare time to work on stuff as well.

[peter.smith]

Committed as r282250, so I've got a chance of fixing problems before end of work day. Happy to make any further updates later.

[ruiu]

LGTM

lld/trunk/ELF/Relocations.cpp

static bool isPreemptible(const SymbolBody &Body, uint32_t Type) {
   // relocation types occupy eight bit. In case of N64 ABI we extract first
   // relocation from 3-in-1 packet because only the first relocation can
   // be against a real symbol.
   if (Config->EMachine == EM_MIPS && (Type & 0xff) == R_MIPS_GPREL16)
     return false;
   return Body.isPreemptible();
 }
 
-// This function is similar to the `handleTlsRelocation`. MIPS does not support
-// any relaxations for TLS relocations so by factoring out MIPS handling into
-// the separate function we can simplify the code and does not pollute
-// `handleTlsRelocation` by MIPS `ifs` statements.
+// This function is similar to the `handleTlsRelocation`. ARM and MIPS do not
+// support any relaxations for TLS relocations so by factoring out ARM and MIPS
+// handling in to the separate function we can simplify the code and do not
+// pollute `handleTlsRelocation` by ARM and MIPS `ifs` statements.
+// FIXME: The ARM implementation always adds the module index dynamic
+// relocation even for non-preemptible symbols in applications. For static
+// linking support we must either resolve the module index relocation at static
+// link time, or hard code the module index (1) for the application in the GOT.
 template <class ELFT>
-static unsigned
-handleMipsTlsRelocation(uint32_t Type, SymbolBody &Body,
-                        InputSectionBase<ELFT> &C, typename ELFT::uint Offset,
-                        typename ELFT::uint Addend, RelExpr Expr) {
-  if (Expr == R_MIPS_TLSLD) {
-    if (Out<ELFT>::Got->addTlsIndex() && Config->Pic)
+static unsigned handleNoRelaxTlsRelocation(uint32_t Type, SymbolBody &Body,
+                                           InputSectionBase<ELFT> &C,
+                                           typename ELFT::uint Offset,
+                                           typename ELFT::uint Addend,
+                                           RelExpr Expr) {
+  if (Expr == R_MIPS_TLSLD || Expr == R_TLSLD_PC) {
+    if (Out<ELFT>::Got->addTlsIndex() &&
+        (Config->Pic || Config->EMachine == EM_ARM))
       Out<ELFT>::RelaDyn->addReloc({Target->TlsModuleIndexRel, Out<ELFT>::Got,
                                     Out<ELFT>::Got->getTlsIndexOff(), false,
                                     nullptr, 0});
     C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
     return 1;
   }
-  if (Target->isTlsGlobalDynamicRel(Type)) {
-    if (Out<ELFT>::Got->addDynTlsEntry(Body) && Body.isPreemptible()) {
-      typedef typename ELFT::uint uintX_t;
+  if (Target->isTlsGlobalDynamicRel(Type)) {
+    if (Out<ELFT>::Got->addDynTlsEntry(Body) &&
+        (Body.isPreemptible() || Config->EMachine == EM_ARM)) {
       uintX_t Off = Out<ELFT>::Got->getGlobalDynOffset(Body);
       Out<ELFT>::RelaDyn->addReloc(
           {Target->TlsModuleIndexRel, Out<ELFT>::Got, Off, false, &Body, 0});
+      if (Body.isPreemptible())
         Out<ELFT>::RelaDyn->addReloc({Target->TlsOffsetRel, Out<ELFT>::Got,
                                       Off + (uintX_t)sizeof(uintX_t), false,
                                       &Body, 0});
     }
     C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
     return 1;
   }
   return 0;
 }
 
 // Returns the number of relocations processed.
 template <class ELFT>
 static unsigned handleTlsRelocation(uint32_t Type, SymbolBody &Body,
                                     InputSectionBase<ELFT> &C,
                                     typename ELFT::uint Offset,
                                     typename ELFT::uint Addend, RelExpr Expr) {
   if (!(C.getSectionHdr()->sh_flags & SHF_ALLOC))
     return 0;
 
   if (!Body.isTls())
     return 0;
 
   typedef typename ELFT::uint uintX_t;
 
-  if (Config->EMachine == EM_MIPS)
-    return handleMipsTlsRelocation<ELFT>(Type, Body, C, Offset, Addend, Expr);
+  if (Config->EMachine == EM_MIPS || Config->EMachine == EM_ARM)
+    return handleNoRelaxTlsRelocation<ELFT>(Type, Body, C, Offset, Addend,
+                                            Expr);
 
   if ((Expr == R_TLSDESC || Expr == R_TLSDESC_PAGE || Expr == R_HINT) &&
       Config->Shared) {
     if (Out<ELFT>::Got->addDynTlsEntry(Body)) {
       uintX_t Off = Out<ELFT>::Got->getGlobalDynOffset(Body);
       Out<ELFT>::RelaDyn->addReloc(
           {Target->TlsDescRel, Out<ELFT>::Got, Off, false, &Body, 0});
     }

lld/trunk/test/ELF/Inputs/arm-tls-get-addr.s

+ .syntax unified
+ .text
+ .globl __tls_get_addr
+ .type __tls_get_addr,%function
+__tls_get_addr:
+ bx lr
+
+.section       .tbss,"awT",%nobits
+ .p2align  2
+y:
+ .space 4
+ .globl y
+ .type  y, %object

lld/trunk/test/ELF/arm-tls-norelax-gd-ie.s

+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %p/Inputs/arm-tls-get-addr.s -o %t1
+// RUN: ld.lld %t1 --shared -o %t1.so
+// RUN: llvm-mc %s -o %t.o -filetype=obj -triple=armv7a-linux-gnueabi
+// RUN: ld.lld %t1.so %t.o -o %t
+// RUN: llvm-readobj -s -dyn-relocations %t | FileCheck %s
+// REQUIRES: arm
+
+// This tls global-dynamic sequence is with respect to a preemptible symbol but
+// is in an application so a relaxation to Initial Exec would normally be
+// possible. This would result in an assertion failure on ARM as the
+// relaxation functions can't be implemented on ARM. Check that the sequence
+// is handled as global dynamic
+
+ .text
+ .syntax unified
+ .globl  func
+ .p2align        2
+ .type   func,%function
+func:
+.L0:
+ .globl __tls_get_addr
+ bl __tls_get_addr
+ bx lr
+ .p2align 2
+ .Lt0: .word   y(TLSGD) + (. - .L0 - 8)
+
+// CHECK: Dynamic Relocations {
+// CHECK-NEXT:   0x12078 R_ARM_TLS_DTPMOD32 y
+// CHECK-NEXT:   0x1207C R_ARM_TLS_DTPOFF32 y
+// CHECK-NEXT:   0x1300C R_ARM_JUMP_SLOT __tls_get_addr

lld/trunk/test/ELF/arm-tls-norelax-gd-le.s

+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %p/Inputs/arm-tls-get-addr.s -o %t1
+// RUN: ld.lld %t1 --shared -o %t1.so
+// RUN: llvm-mc %s -o %t.o -filetype=obj -triple=armv7a-linux-gnueabi
+// RUN: ld.lld %t1.so %t.o -o %t
+// RUN: llvm-readobj -s -dyn-relocations %t | FileCheck %s
+// REQUIRES: arm
+
+// This tls global-dynamic sequence is with respect to a non-preemptible
+// symbol in an application so a relaxation to Local Exec would normally be
+// possible. This would result in an assertion failure on ARM as the
+// relaxation functions can't be implemented on ARM. Check that the sequence
+// is handled as global dynamic
+
+ .text
+ .syntax unified
+ .globl  func
+ .p2align        2
+ .type   func,%function
+func:
+.L0:
+ .globl __tls_get_addr
+ bl __tls_get_addr
+ bx lr
+ .p2align 2
+ .Lt0: .word   x(TLSGD) + (. - .L0 - 8)
+
+ .globl  x
+.section       .tbss,"awT",%nobits
+ .p2align  2
+x:
+ .space 4
+ .type  x, %object
+
+// CHECK: Dynamic Relocations {
+// CHECK-NEXT:   0x12078 R_ARM_TLS_DTPMOD32
+// CHECK-NEXT:   0x1300C R_ARM_JUMP_SLOT __tls_get_addr
+

lld/trunk/test/ELF/arm-tls-norelax-ie-le.s

+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %p/Inputs/arm-tls-get-addr.s -o %t1
+// RUN: ld.lld %t1 --shared -o %t1.so
+// RUN: llvm-mc %s -o %t.o -filetype=obj -triple=armv7a-linux-gnueabi
+// RUN: ld.lld %t1.so %t.o -o %t
+// RUN: llvm-objdump -s -triple=armv7a-linux-gnueabi %t
+// REQUIRES: arm
+
+// This tls Initial Exec sequence is with respect to a non-preemptible symbol
+// so a relaxation would normally be possible. This would result in an assertion
+// failure on ARM as the relaxation functions can't be implemented on ARM.
+// Check that the sequence is handled as initial exec
+ .text
+ .syntax unified
+ .globl  func
+ .p2align        2
+ .type   func,%function
+func:
+.L0:
+ .globl __tls_get_addr
+ bl __tls_get_addr
+.L1:
+ bx lr
+ .p2align 2
+ .Lt0: .word  x1(gottpoff) + (. - .L0 - 8)
+ .Lt1: .word  x2(gottpoff) + (. - .L1 - 8)
+
+ .globl  x1
+ .section       .trw,"awT",%progbits
+ .p2align  2
+x1:
+ .word 0x1
+ .globl x2
+ .section       .tbss,"awT",%nobits
+ .type  x1, %object
+x2:
+ .space 4
+ .type x2, %object
+
+// CHECK: Contents of section .got
+// x1 at offset 0 from TP, x2 at offset 4 from TP
+// 12064 00000000 04000000

lld/trunk/test/ELF/arm-tls-norelax-ld-le.s

+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %p/Inputs/arm-tls-get-addr.s -o %t1
+// RUN: ld.lld %t1 --shared -o %t1.so
+// RUN: llvm-mc %s -o %t.o -filetype=obj -triple=armv7a-linux-gnueabi
+// RUN: ld.lld %t1.so %t.o -o %t
+// RUN: llvm-readobj -s -dyn-relocations %t | FileCheck %s
+// REQUIRES: arm
+
+ .global __tls_get_addr
+ .text
+ .p2align  2
+ .global _start
+ .syntax unified
+ .arm
+ .type   _start, %function
+_start:
+.L0:
+ bl __tls_get_addr
+
+ .word   x(tlsldm) + (. - .L0 - 8)
+ .word   x(tlsldo)
+ .word   y(tlsldo)
+
+ .section        .tbss,"awT",%nobits
+ .p2align  2
+ .type   y, %object
+y:
+ .space  4
+ .section        .tdata,"awT",%progbits
+ .p2align  2
+ .type   x, %object
+x:
+ .word   10
+
+// CHECK: Dynamic Relocations {
+// CHECK-NEXT:   0x1207C R_ARM_TLS_DTPMOD32 - 0x0
+// CHECK-NEXT:   0x1300C R_ARM_JUMP_SLOT __tls_get_addr 0x0