Commit ad5bcd4e authored by Tom Stellard's avatar Tom Stellard
Browse files

Merging r361090: 

------------------------------------------------------------------------
r361090 | maskray | 2019-05-17 20:16:00 -0700 (Fri, 17 May 2019) | 34 lines

[ARM][AArch64] Revert Android Bionic PT_TLS overaligning hack

This reverts D53906.

D53906 increased p_align of PT_TLS on ARM/AArch64 to 32/64 to make the
static TLS layout compatible with Android Bionic's ELF TLS. However,
this may cause glibc ARM/AArch64 programs to crash (see PR41527).

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

For a thread local variable defined in the executable, lld computed TLS
offset (local exec) is different from glibc computed TLS offset from
another module (initial exec/generic dynamic). Note: PR41527 said the
bug affects initial exec but actually generic dynamic is affected as
well.

(glibc is correct in that it compute offsets that satisfy
`offset%p_align == p_vaddr%p_align`, which is a basic ELF requirement.
This hack appears to work on FreeBSD rtld, musl<=1.1.22, and Bionic, but
that is just because they (and lld) incorrectly compute offsets that
satisfy `offset%p_align = 0` instead.)

Android developers are fine to revert this patch, carry this patch in
their tree before figuring out a long-term solution (e.g. a dummy .tdata
with sh_addralign=64 sh_size={0,1} in crtbegin*.o files. The overhead is
now insignificant after D62059).

Reviewed By: rprichard, srhines

Differential Revision: https://reviews.llvm.org/D62055
------------------------------------------------------------------------

llvm-svn: 362858
parent 0e657d45

lld/ELF/InputSection.cpp

+0 −4
Original line number Diff line number Diff line
@@ -580,10 +580,6 @@ static int64_t getTlsTpOffset() { 
    // Variant 1. The thread pointer points to a TCB with a fixed 2-word size,

    // followed by a variable amount of alignment padding, followed by the TLS

    // segment.

    //

    // NB: While the ARM/AArch64 ABI formally has a 2-word TCB size, lld

    // effectively increases the TCB size to 8 words for Android compatibility.

    // It accomplishes this by increasing the segment's alignment.

    return alignTo(Config->Wordsize * 2, Out::TlsPhdr->p_align);

  case EM_386:

  case EM_X86_64:

lld/ELF/Writer.cpp

+0 −11
Original line number Diff line number Diff line
@@ -2197,17 +2197,6 @@ template <class ELFT> void Writer<ELFT>::setPhdrs() { 
    }



    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.

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

+2 −2
Original line number Diff line number Diff line
@@ -26,9 +26,9 @@ _start: 
// 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

lld/test/ELF/aarch64-tls-gdle.s

+2 −2
Original line number Diff line number Diff line
@@ -9,11 +9,11 @@ 
#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: _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

lld/test/ELF/aarch64-tls-iele.s

+3 −3
Original line number Diff line number Diff line
@@ -9,13 +9,13 @@ 
# 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: _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