[PATCH] D61824: [ARM][AArch64] Overalign SHF_TLS instead of PT_TLS (Android Bionic hack) to make glibc happy

Sat May 11 06:41:37 PDT 2019

MaskRay created this revision.
MaskRay added reviewers: peter.smith, rprichard, ruiu.
Herald added subscribers: llvm-commits, kristof.beyls, arichardson, javed.absar, emaste, srhines.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

Fixes PR41527

D53906 <https://reviews.llvm.org/D53906> increases p_align of PT_TLS on ARM/AArch64 to 32/64 to make the
TLS layout compatible with Android/Bionic. However, this causes issues
on glibc ARM/AArch64 platforms using initial-exec TLS model (lld's and
glibc's understanding of TPREL is different).

The problem is that we increase the p_align of the PT_TLS after the
OutputSection's addresses are fixed (assignAddress()). This may create a
PT_TLS that p_vaddr%p_align!=0. It turns out glibc elf/dl-tls.c doesn't
handle such PT_TLS correctly (the executable's TLS offset is
2*sizeof(void*) instead of alignUp(2*sizeof(void*), exe.p_align)).

This patches move the "overalign" hack to OutputSections.cpp where we
set SHF_TLS sections' alignment, thus ensures p_vaddr%p_align==0.

As a side note, musl and Bionic handle p_vaddr%p_align!=0 correctly.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D61824

Files:
  ELF/OutputSections.cpp
  ELF/Writer.cpp
  test/ELF/arm-tls-gd-nonpreemptible.s
  test/ELF/arm-tls-le32.s
  test/ELF/arm-tls-norelax-ie-le.s


Index: test/ELF/arm-tls-norelax-ie-le.s
===================================================================

--- test/ELF/arm-tls-norelax-ie-le.s
+++ test/ELF/arm-tls-norelax-ie-le.s
@@ -38,4 +38,4 @@
 
 // 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
+// CHECK-NEXT: 12064 20000000 40000000
Index: test/ELF/arm-tls-le32.s
===================================================================
--- test/ELF/arm-tls-le32.s
+++ test/ELF/arm-tls-le32.s
@@ -61,7 +61,7 @@
 // SEC-NEXT:   SHF_TLS
 // SEC-NEXT:   SHF_WRITE
 // SEC-NEXT: ]
-// SEC-NEXT: Address: 0x12004
+// SEC-NEXT: Address: 0x12020
 // SEC:      Size: 8
 
 // SEC: Dynamic Relocations {
@@ -73,6 +73,6 @@
 // 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
+// CHECK-NEXT:   11004:         44 00 00 00
+// offset of y from Thread pointer = (TcbSize + 0x24 = 0x44)
+// CHECK-NEXT:   11008:         40 00 00 00
Index: test/ELF/arm-tls-gd-nonpreemptible.s
===================================================================
--- test/ELF/arm-tls-gd-nonpreemptible.s
+++ test/ELF/arm-tls-gd-nonpreemptible.s
@@ -65,7 +65,7 @@
 
 // CHECK: Contents of section .got:
 // CHECK-NEXT:  12008 01000000 00000000 01000000 04000000
-// CHECK-NEXT:  12018 01000000 08000000 01000000 0c000000
+// CHECK-NEXT:  12018 01000000 20000000 01000000 24000000
 
 // CHECK-SHARED: Contents of section .got:
 // CHECK-SHARED-NEXT:  2050 00000000 00000000 00000000 04000000
Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -2188,17 +2188,6 @@
     }
 
     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);
-      }
-
       // 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.
Index: ELF/OutputSections.cpp
===================================================================
--- ELF/OutputSections.cpp
+++ ELF/OutputSections.cpp
@@ -120,6 +120,17 @@
 
   Alignment = std::max(Alignment, IS->Alignment);
 
+  // On ARM/AArch64, overalign the SHF_TLS section to 8 words. This ensures the
+  // alignment of the created PT_TLS is at least 8 words. The TLS offset of the
+  // main module is thus alignUp(2*WordSize, main_tls_align) >= 8*WordSize. The
+  // gap can hold 8 slots used by Android Bionic's TCB, which is needed for
+  // backwards compatibility (it 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.
+  if ((Flags & ELF::SHF_TLS) && !Config->Shared &&
+      (Config->EMachine == EM_ARM || Config->EMachine == EM_AARCH64))
+    Alignment = std::max<uint32_t>(Alignment, Config->Wordsize * 8);
+
   // If this section contains a table of fixed-size entries, sh_entsize
   // holds the element size. If it contains elements of different size we
   // set sh_entsize to 0.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61824.199135.patch
Type: text/x-patch
Size: 3935 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190511/4879912c/attachment.bin>
