[PATCH] D62059: [ELF] Don't align PT_TLS's p_memsz

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D62059#1507017 <https://reviews.llvm.org/D62059#1507017>, @rprichard wrote:

> I had suggested doing something like this at https://reviews.llvm.org/D53906#1326683. I like the idea, but I think we need to fix up the variant 2 case in getTlsTpOffset:
>
>   case EM_386:
>   case EM_X86_64:
>     // Variant 2. The TLS segment is located just before the thread pointer.
>     return -Out::TlsPhdr->p_memsz;
>   
>
> e.g. If the TLS segment is 4-bytes in size with 8-byte alignment (and assuming p_vaddr % p_align == 0), then the segment will be located at TP-8 on variant 2 targets, not TP-4.
>
> I haven't tested this, but something like this would probably work:
>
>   return -alignTo(Out::TlsPhdr->p_memsz, Out::TlsPhdr->p_align);
>   
>
> Otherwise, lld and the loader will disagree about the location of TLS symbols.


Thanks for pointing this out! I pushed a quick fix. To be more precise, it should be:

  diff --git c/ELF/InputSection.cpp w/ELF/InputSection.cpp
  index 72a2c298d..2eb9a0452 100644
  --- c/ELF/InputSection.cpp
  +++ w/ELF/InputSection.cpp
  @@ -594,11 +594,13 @@ static int64_t getTlsTpOffset() {
       // 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);
  +    return alignTo(Config->Wordsize * 2, Out::TlsPhdr->p_align, Out::TlsPhdr->FirstSec->Addr);
     case EM_386:
     case EM_X86_64:
       // Variant 2. The TLS segment is located just before the thread pointer.
  -    return -alignTo(Out::TlsPhdr->p_memsz, Out::TlsPhdr->p_align);
  +    return -(Out::TlsPhdr->p_memsz +
  +             (-Out::TlsPhdr->p_memsz - Out::TlsPhdr->FirstSec->Addr &
  +              Out::TlsPhdr->p_align - 1));
     case EM_PPC64:
       // The thread pointer points to a fixed offset from the start of the
       // executable's TLS segment. An offset of 0x7000 allows a signed 16-bit

The formulae are complex because they take `p_vaddr%p_align!=0` into account... I didn't do that in the commit because I cannot think of a way to make `p_vaddr%p_align!=0` (after we remove the ARM/AArch64 overalignment hack).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62059/new/

https://reviews.llvm.org/D62059