[ARM][AArch64] Put Android Bionic PT_TLS overaligning hack under --android-tls

[MaskRay]

Summary

D53906 increased p_align of PT_TLS on ARM/AArch64 to 32/64 to make the
TLS layout compatible with Android Bionic. However, this can make glibc
ARM/AArch64 programs using {initial,local}-exec TLS models crash (see
PR41527).

The faulty PT_TLS satisfies p_vaddr%p_align != 0. The remainder is normally
0 but may be non-zero with the D53906 hack in place. The problem is that
we increase the p_align of the PT_TLS after the OutputSection's
addresses are fixed (assignAddress()). It is possible that
p_vaddr%old_p_align = 0 while p_vaddr%new_p_align != 0.

When this happens, lld and different ld.so implementations have
different opinions on the offset of the first module. glibc elf/dl-tls.c
computes an offset that satisfies: offset%p_align == p_vaddr%p_align.
This is correct as a basic ELF requirement.

lld, musl (1.1.20 ~ latest 1.1.22), FreeBSD rtld, and Bionic place the
main TLS block at a wrong offset: `alignUp(2*sizeof(void*),
main_tls_align)`. This choice doesn't take p_vaddr%p_align into account.

An approach to make both Bionic and glibc happy is to overalign SHF_TLS
instead of PT_TLS. It has several disadvantages:

• Moving the hack to the OutputSection layer makes the hack more perceivable as it has interaction with the linker script ALIGNUP command.
• It may waste up to 8*WordSize * 2 bytes of each thread (when both .tbss and .tdata exist and are overaligned). With more code, we can apply the overalignment to the first TLS output section (either .tbss or .tdata) and decrease the wastage to 8*WordSize, but that is still unsatisfactory on other platforms.

This patch keeps the hack under an option. This option may be deleted
when a better fix arises, e.g. a solution that works across
ld.bfd/gold/lld is to add a dummy .tdata (1-sized 64-aligned if Bionic
aligns the end of the TLS block; 48-sized 1-aligned if Bionic aligns the
start of the TLS block) in Android's crtbegin_{dynamic,static}.o . A
R_{ARM,AARCH64}_NONE relocation reference from _start is required to
make the .tdata survive --gc-sections.

Currently, however, Bionic has an alignment check
__bionic_check_tls_alignment that collides with this fix and it may be
too late due to deployment issues. Ideally the vaddr%p_align != 0 case
should also be fixed.

[MaskRay]

.

[evgeny777]

If we always align first PT_TLS section on 64 bytes wouldn't this fix the problem?

[MaskRay]

In D61825#1499782, @evgeny777 wrote:

If we always align first PT_TLS section on 64 bytes wouldn't this fix the problem?

The alternative solution D61824 does that.

[MaskRay]

Update description

[MaskRay]

Update description

[MaskRay]

@rprichard @srhines I know the workaround is important for Android ELF TLS, but according to our analysis this is bad to other platforms... The 8.0.1 release is coming, I hope we can fix this issue before that... I just created a clang-side change D61931.

See my updated description for a better long-term solution (".tdata (1-sized 64-aligned if Bionic aligns the end of the TLS block; 48-sized 1-aligned if Bionic aligns the start of the TLS block")

[MaskRay]

Abandon in favor of a revert: D62055. Ryan told me reverting is preferable.

ELF/Config.h

struct Configuration {
   std::vector<llvm::StringRef> Undefined;
   std::vector<SymbolVersion> DynamicList;
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<SymbolVersion> VersionScriptLocals;
   std::vector<uint8_t> BuildIdVector;
   llvm::MapVector<std::pair<const InputSectionBase *, const InputSectionBase *>,
                   uint64_t>
       CallGraphProfile;
+  bool AndroidTls;
   bool AllowMultipleDefinition;
   bool AllowShlibUndefined;
   bool AndroidPackDynRelocs;
   bool ARMHasBlx = false;
   bool ARMHasMovtMovw = false;
   bool ARMJ1J2BranchEncoding = false;
   bool AsNeeded = false;
   bool Bsymbolic;

ELF/Driver.cpp

static void readConfigs(opt::InputArgList &Args) {
 
   Config->AllowMultipleDefinition =
       Args.hasFlag(OPT_allow_multiple_definition,
                    OPT_no_allow_multiple_definition, false) ||
       hasZOption(Args, "muldefs");
   Config->AllowShlibUndefined =
       Args.hasFlag(OPT_allow_shlib_undefined, OPT_no_allow_shlib_undefined,
                    Args.hasArg(OPT_shared));
+  Config->AndroidTls = Args.hasArg(OPT_android_tls);
   Config->AuxiliaryList = args::getStrings(Args, OPT_auxiliary);
   Config->Bsymbolic = Args.hasArg(OPT_Bsymbolic);
   Config->BsymbolicFunctions = Args.hasArg(OPT_Bsymbolic_functions);
   Config->CheckSections =
       Args.hasFlag(OPT_check_sections, OPT_no_check_sections, true);
   Config->Chroot = Args.getLastArgValue(OPT_chroot);
   Config->CompressDebugSections = getCompressDebugSections(Args);
   Config->Cref = Args.hasFlag(OPT_cref, OPT_no_cref, false);

ELF/Options.td

def NAME # _eq: Joined<["--", "-"], name # "=">, Alias<!cast<Separate>(NAME)>,
     HelpText<help>;
 }
 
 multiclass B<string name, string help1, string help2> {
   def NAME: Flag<["--", "-"], name>, HelpText<help1>;
   def no_ # NAME: Flag<["--", "-"], "no-" # name>, HelpText<help2>;
 }
 
+def android_tls
+    : Flag<["--"], "android-tls">,
+      HelpText<"Use Android Bionic compatible TLS layout on ARM/AArch64">;
+
 defm auxiliary: Eq<"auxiliary", "Set DT_AUXILIARY field to the specified name">;
 
 def Bsymbolic: F<"Bsymbolic">, HelpText<"Bind defined symbols locally">;
 
 def Bsymbolic_functions: F<"Bsymbolic-functions">,
   HelpText<"Bind defined function symbols locally">;
 
 def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries (default)">;

ELF/Writer.cpp

if (P->p_type == PT_LOAD) {
       P->p_align = 1;
       // The glibc dynamic loader rounds the size down, so we need to round up
       // to protect the last page. This is a no-op on FreeBSD which always
       // rounds up.
       P->p_memsz = alignTo(P->p_memsz, Config->CommonPageSize);
     }
 
     if (P->p_type == PT_TLS && P->p_memsz) {
-      if (!Config->Shared &&
-          (Config->EMachine == EM_ARM || Config->EMachine == EM_AARCH64)) {
-        // On ARM/AArch64, reserve extra space (8 words) between the thread
-        // pointer and an executable's TLS segment by overaligning the segment.
-        // This reservation is needed for backwards compatibility with Android's
-        // TCB, which allocates several slots after the thread pointer (e.g.
-        // TLS_SLOT_STACK_GUARD==5). For simplicity, this overalignment is also
-        // done on other operating systems.
-        P->p_align = std::max<uint64_t>(P->p_align, Config->Wordsize * 8);
-      }
+      // On Android ARM/AArch64, reserve extra space (8 words) between the
+      // thread pointer and an executable's TLS segment by overaligning the
+      // segment. This reservation is needed for backwards compatibility with
+      // Android's TCB, which allocates several slots after the thread pointer
+      // (e.g. TLS_SLOT_STACK_GUARD==5).
+      if (Config->AndroidTls && !Config->Shared &&
+          (Config->EMachine == EM_ARM || Config->EMachine == EM_AARCH64))
+        P->p_align = std::max<uint32_t>(P->p_align, Config->Wordsize * 8);
 
       // The TLS pointer goes after PT_TLS for variant 2 targets. At least glibc
       // will align it, so round up the size to make sure the offsets are
       // correct.
       P->p_memsz = alignTo(P->p_memsz, P->p_align);
     }
   }
 }

docs/ld.lld.1

 .Sh OPTIONS
 .Bl -tag -width indent
 .It Fl -allow-multiple-definition
 Do not error if a symbol is defined multiple times.
 The first definition will be used.
 .It Fl -allow-shlib-undefined
 Allow unresolved references in shared libraries.
 This option is enabled by default when linking a shared library.
+.It Fl -android-tls
+Use Android Bionic compatible TLS layout on ARM/AArch64.
 .It Fl -apply-dynamic-relocs
 Apply link-time values for dynamic relocations.
 .It Fl -as-needed
 Only set
 .Dv DT_NEEDED
 for shared libraries if used.
 .It Fl -auxiliary Ns = Ns Ar value
 Set the

test/ELF/aarch64-cortex-a53-843419-tlsrelax.s

  ldr    x1, [x0, #:gottprel_lo12:v]
  adrp   x0, :gottprel:v
  ldr    x1, [x0, #:gottprel_lo12:v]
  ret
 
 // CHECK: _start:
 // CHECK-NEXT:   210ff8:        41 d0 3b d5     mrs     x1, TPIDR_EL0
 // CHECK-NEXT:   210ffc:        00 00 a0 d2     movz    x0, #0, lsl #16
-// CHECK-NEXT:   211000:        01 08 80 f2     movk    x1, #64
+// CHECK-NEXT:   211000:        01 02 80 f2     movk    x1, #16
 // CHECK-NEXT:   211004:        00 00 a0 d2     movz    x0, #0, lsl #16
-// CHECK-NEXT:   211008:        01 08 80 f2     movk    x1, #64
+// CHECK-NEXT:   211008:        01 02 80 f2     movk    x1, #16
 // CHECK-NEXT:   21100c:        c0 03 5f d6     ret
 
  .type  v,@object
  .section       .tbss,"awT",@nobits
  .globl v
 v:
  .word 0

test/ELF/aarch64-tls-gdle.s

 # REQUIRES: aarch64
 # RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-linux %p/Inputs/aarch64-tls-ie.s -o %ttlsie.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-linux %s -o %tmain.o
 # RUN: ld.lld %tmain.o %ttlsie.o -o %tout
 # RUN: llvm-objdump -d %tout | FileCheck %s
 # RUN: llvm-readobj -S -r %tout | FileCheck -check-prefix=RELOC %s
 
 #Local-Dynamic to Local-Exec relax creates no
 #RELOC:      Relocations [
 #RELOC-NEXT: ]
 
-# TCB size = 64 and foo is first element from TLS register.
+# TCB size = 0x16 and foo is first element from TLS register.
 # CHECK: Disassembly of section .text:
 # CHECK-EMPTY:
 # CHECK: _start:
 # CHECK:  210000: 00 00 a0 d2 movz  x0, #0, lsl #16
-# CHECK:  210004: 00 08 80 f2   movk  x0, #64
+# CHECK:  210004: 00 02 80 f2   movk  x0, #16
 # CHECK:  210008: 1f 20 03 d5   nop
 # CHECK:  21000c: 1f 20 03 d5   nop
 
 .globl _start
 _start:
  adrp    x0, :tlsdesc:foo
  ldr     x1, [x0, :tlsdesc_lo12:foo]
  add     x0, x0, :tlsdesc_lo12:foo
  .tlsdesccall foo
  blr     x1

test/ELF/aarch64-tls-iele.s

 # REQUIRES: aarch64
 # RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-linux %p/Inputs/aarch64-tls-ie.s -o %ttlsie.o
 # RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-linux %s -o %tmain.o
 # RUN: ld.lld %tmain.o %ttlsie.o -o %tout
 # RUN: llvm-objdump -d %tout | FileCheck %s
 # RUN: llvm-readobj -S -r %tout | FileCheck -check-prefix=RELOC %s
 
 # Initial-Exec to Local-Exec relax creates no dynamic relocations.
 # RELOC:      Relocations [
 # RELOC-NEXT: ]
 
-# TCB size = 64 and foo is first element from TLS register.
+# TCB size = 0x16 and foo is first element from TLS register.
 # CHECK: Disassembly of section .text:
 # CHECK-EMPTY:
 # CHECK: _start:
 # CHECK-NEXT: 210000:  00 00 a0 d2   movz   x0, #0, lsl #16
-# CHECK-NEXT: 210004:  80 08 80 f2   movk   x0, #68
+# CHECK-NEXT: 210004:  80 02 80 f2   movk   x0, #20
 # CHECK-NEXT: 210008:  00 00 a0 d2   movz   x0, #0, lsl #16
-# CHECK-NEXT: 21000c:  00 08 80 f2   movk   x0, #64
+# CHECK-NEXT: 21000c:  00 02 80 f2   movk   x0, #16
 
 .section .tdata
 .align 2
 .type foo_local, %object
 .size foo_local, 4
 foo_local:
 .word 5
 .text
 
 .globl _start
 _start:
  adrp    x0, :gottprel:foo
  ldr     x0, [x0, :gottprel_lo12:foo]
  adrp    x0, :gottprel:foo_local
  ldr     x0, [x0, :gottprel_lo12:foo_local]

test/ELF/aarch64-tls-le.s

 _start:
  mrs x0, TPIDR_EL0
  add x0, x0, :tprel_hi12:v1
  add x0, x0, :tprel_lo12_nc:v1
  mrs x0, TPIDR_EL0
  add x0, x0, :tprel_hi12:v2
  add x0, x0, :tprel_lo12_nc:v2
 
-# TCB size = 64 and foo is first element from TLS register.
+# TCB size = 0x16 and foo is first element from TLS register.
 #CHECK: Disassembly of section .text:
 #CHECK: _start:
 #CHECK:  210000: 40 d0 3b d5     mrs     x0, TPIDR_EL0
 #CHECK:  210004: 00 00 40 91     add     x0, x0, #0, lsl #12
-#CHECK:  210008: 00 00 01 91     add     x0, x0, #64
+#CHECK:  210008: 00 40 00 91     add     x0, x0, #16
 #CHECK:  21000c: 40 d0 3b d5     mrs     x0, TPIDR_EL0
 #CHECK:  210010: 00 fc 7f 91     add     x0, x0, #4095, lsl #12
 #CHECK:  210014: 00 e0 3f 91     add     x0, x0, #4088
 
 .section        .tbss,"awT",@nobits
 
 .type   v1,@object
 .globl  v1
 .p2align 2
 v1:
 .word  0
 .size  v1, 4
 
-# The current offset from the thread pointer is 68. Raise it to just below the
+# The current offset from the thread pointer is 20. Raise it to just below the
 # 24-bit limit.
-.space (0xfffff8 - 68)
+.space (0xfffff8 - 20)
 
 .type   v2,@object
 .globl  v2
 .p2align 2
 v2:
 .word  0
 .size  v2, 4

test/ELF/aarch64-tlsld-ldst.s

_start:  mrs x8, TPIDR_EL0
         add x8, x8, :tprel_hi12:var3
         ldrh w0, [x8, :tprel_lo12_nc:var3]
 
         add x8, x8, :tprel_hi12:var4
         ldrb w0, [x8, :tprel_lo12_nc:var4]
 
 // CHECK: _start:
 // CHECK-NEXT:    210000:       48 d0 3b d5     mrs     x8, TPIDR_EL0
-// 0x0 + c40 = 0xc40       = tcb (64-bytes) + var0
-// CHECK-NEXT:    210004:       08 01 40 91     add x8, x8, #0, lsl #12
-// CHECK-NEXT:    210008:       14 11 c3 3d     ldr q20, [x8, #3136]
-// 0x1000 + 0x850 = 0x1850 = tcb + var1
-// CHECK-NEXT:    21000c:       08 05 40 91     add x8, x8, #1, lsl #12
-// CHECK-NEXT:    210010:       00 29 44 f9     ldr x0, [x8, #2128]
-// 0x2000 + 0x458 = 0x2458 = tcb + var2
-// CHECK-NEXT:    210014:       08 09 40 91     add x8, x8, #2, lsl #12
-// CHECK-NEXT:    210018:       00 59 44 b9     ldr w0, [x8, #1112]
-// 0x3000 + 0x5c  = 0x305c = tcb + var3
-// CHECK-NEXT:    21001c:       08 0d 40 91     add x8, x8, #3, lsl #12
-// CHECK-NEXT:    210020:       00 b9 40 79     ldrh  w0, [x8, #92]
-// 0x3000 + 0xc5e = 0x3c5e = tcb + var4
-// CHECK-NEXT:    210024:       08 0d 40 91     add x8, x8, #3, lsl #12
-// CHECK-NEXT:    210028:       00 79 71 39     ldrb  w0, [x8, #3166]
+// 0x0 + c10 = 0xc10       = tcb (16-bytes) + var0
+// CHECK-NEXT:    210004:       08 01 40 91     add     x8, x8, #0, lsl #12
+// CHECK-NEXT:    210008:       14 05 c3 3d     ldr     q20, [x8, #3088]
+// 0x1000 + 0x820 = 0x1820 = tcb + var1
+// CHECK-NEXT:    21000c:       08 05 40 91     add     x8, x8, #1, lsl #12
+// CHECK-NEXT:    210010:       00 11 44 f9     ldr     x0, [x8, #2080]
+// 0x2000 + 0x428 = 0x2428 = tcb + var2
+// CHECK-NEXT:    210014:       08 09 40 91     add     x8, x8, #2, lsl #12
+// CHECK-NEXT:    210018:       00 29 44 b9     ldr     w0, [x8, #1064]
+// 0x3000 + 0x2c  = 0x302c = tcb + var3
+// CHECK-NEXT:    21001c:       08 0d 40 91     add     x8, x8, #3, lsl #12
+// CHECK-NEXT:    210020:       00 59 40 79     ldrh    w0, [x8, #44]
+// 0x3000 + 0xc2e = 0x32ce = tcb + var4
+// CHECK-NEXT:    210024:       08 0d 40 91     add     x8, x8, #3, lsl #12
+// CHECK-NEXT:    210028:       00 b9 70 39     ldrb    w0, [x8, #3118]
 
-// CHECK-SYMS:      0000000000000c00    16 TLS     GLOBAL DEFAULT    2 var0
-// CHECK-SYMS-NEXT: 0000000000001810     8 TLS     GLOBAL DEFAULT    2 var1
-// CHECK-SYMS-NEXT: 0000000000002418     4 TLS     GLOBAL DEFAULT    2 var2
-// CHECK-SYMS-NEXT: 000000000000301c     2 TLS     GLOBAL DEFAULT    2 var3
-// CHECK-SYMS-NEXT: 0000000000003c1e     1 TLS     GLOBAL DEFAULT    2 var4
+// CHECK-SYMS:      0000000000000c00     0 TLS     GLOBAL DEFAULT    2 var0
+// CHECK-SYMS-NEXT: 0000000000001810     4 TLS     GLOBAL DEFAULT    2 var1
+// CHECK-SYMS-NEXT: 0000000000002418     2 TLS     GLOBAL DEFAULT    2 var2
+// CHECK-SYMS-NEXT: 000000000000301c     1 TLS     GLOBAL DEFAULT    2 var3
+// CHECK-SYMS-NEXT: 0000000000003c1e     0 TLS     GLOBAL DEFAULT    2 var4
 
         .globl var0
         .globl var1
         .globl var2
         .globl var3
         .globl var4
         .type var0,@object
         .type var1,@object
         .type var2,@object
         .type var3,@object
 
 .section .tbss,"awT",@nobits
-        .balign 64
+        .balign 16
         .space 1024 * 3
 var0:
         .quad 0
         .quad 0
-        .size var0, 16
+        .size var1, 16
         .space 1024 * 3
 var1:
         .quad 0
         .size var1, 8
         .space 1024 * 3
 var2:
         .word 0
-        .size var2, 4
+        .size var1, 4
 
         .space 1024 * 3
 var3:
         .hword 0
-        .size var3, 2
+        .size var2, 2
         .space 1024 * 3
 var4:
         .byte 0
-        .size var4, 1
+        .size var3, 1
         .space 1024 * 3

test/ELF/android-tls.s

+# REQUIRES: arm
+# RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-android %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-readelf -l %t | FileCheck %s
+# RUN: ld.lld %t.o --android-tls -o %t
+# RUN: llvm-readelf -l %t | FileCheck --check-prefix=A32 %s
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux-android %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-readelf -l %t | FileCheck %s
+# RUN: ld.lld %t.o --android-tls -o %t
+# RUN: llvm-readelf -l %t | FileCheck --check-prefix=A64 %s
+
+# CHECK: TLS {{.*}} 0x1
+
+## Android Bionic reserves several slots after the thread pointer. Check we
+## overalign the PT_TLS segment to adapt to its TLS layout.
+
+# A32: TLS {{.*}} 0x20
+# A64: TLS {{.*}} 0x40
+
+.section .tbss,"awT",%nobits
+.byte 0

test/ELF/arm-tls-le32.s

 // SEC:      Size: 8
 
 // SEC: Dynamic Relocations {
 // SEC-NEXT: }
 
 // CHECK: Disassembly of section .text:
 // CHECK-EMPTY:
 // CHECK-NEXT: _start:
-// offset of x from Thread pointer = (TcbSize + 0x0 = 0x20)
-// CHECK-NEXT:   11000:         20 00 00 00
-// offset of z from Thread pointer = (TcbSize + 0x8 = 0x28)
-// CHECK-NEXT:   11004:         28 00 00 00
-// offset of y from Thread pointer = (TcbSize + 0x4 = 0x24)
-// CHECK-NEXT:   11008:         24 00 00 00
+// offset of x from Thread pointer = (TcbSize + 0x0 = 0x8)
+// CHECK-NEXT:   11000:         08 00 00 00
+// offset of z from Thread pointer = (TcbSize + 0x8 = 0x10)
+// CHECK-NEXT:   11004:         10 00 00 00
+// offset of y from Thread pointer = (TcbSize + 0x4 = 0xc)
+// CHECK-NEXT:   11008:         0c 00 00 00

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

  .globl x2
  .section       .tbss,"awT",%nobits
  .type  x1, %object
 x2:
  .space 4
  .type x2, %object
 
 // CHECK: Contents of section .got:
-// x1 at offset 0x20 from TP, x2 at offset 0x24 from TP. Offsets include TCB size of 0x20
-// CHECK-NEXT: 12064 20000000 24000000
+// x1 at offset 8 from TP, x2 at offset 0xc from TP. Offsets include TCB size of 8
+// CHECK-NEXT: 12064 08000000 0c000000