[PATCH] D53906: [ELF] Allow configuring the TLS layout for an Android executable

[Ryan Prichard via Phabricator via llvm-commits]

rprichard added a comment.

In https://reviews.llvm.org/D53906#1284788, @ruiu wrote:

> Setting a large alignment to the TLS segment to make room from TP and the TLS segment is an interesting idea. It is tempting, but at the same time it feels a bit too subtle and perhaps fragile. Looks like it is a very indirect way to do what we actually want to do.
>
> If I understand correctly, what we want to do is boiled down to this:
>
>    // A TLS symbol's virtual address is relative to the TLS segment. Add a
>    // target-specific adjustment to produce a thread-pointer-relative offset.
>    static int64_t getTlsTpOffset() {
>      switch (Config->EMachine) {
>      case EM_ARM:
>      case EM_AARCH64:
>        // 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.
>   -    return alignTo(Config->Wordsize * 2, Out::TlsPhdr->p_align);
>   +    if (Android)
>   +      return alignTo(Config->Wordsize * 6, Out::TlsPhdr->p_align);
>   +    else
>   +      return alignTo(Config->Wordsize * 2, Out::TlsPhdr->p_align);
>      case EM_386:
>      case EM_X86_64:
>        // Variant 2. The TLS segment is located just before the thread pointer.
>        return -Out::TlsPhdr->p_memsz;
>   
>
> Is this understanding correct? If so, one radical but simple approach would be to always skip the first 6 words from TP on ARM and AArch64 whether it is Android or not. Obviously that wastes 4 words per a thread, but since thread is not a lightweight data structure, it might be negligible.


We wouldn't be able to do that on non-Android targets, because libc is responsible for initializing the thread pointer to point at a 2-word TCB followed by the TLS segment. Other libc's would need to use a 6-word TCB instead. (e.g. musl would need its `GAP_ABOVE_TP` value changed -- https://git.musl-libc.org/cgit/musl/tree/arch/aarch64/pthread_arch.h?h=v1.1.20#n9)

The Android libc could use a 6-word TCB, so I believe the patch above would work. I have a few reservations:

- If we reserve 6 words, maybe it makes sense to reserve a bit more to avoid moving the TSAN slot?
- We probably still need a `PT_ANDROID_TLS_TPOFF` or equivalent note, because without it, it's too easy to create executables with memory corruption.
- It simplifies Bionic if x86 and ARM use the same TLS layout. One justification for variant 2 in Drepper's TLS document was backwards-compatibility with existing thread pointer usage, and since then, new architectures have used variant 1 (or a fixed-offset) instead. If x86 used a variant 1 layout instead, then Bionic likely wouldn't have to implement variant 2. I'm a little hesitant to diverge unnecessarily from the x86 ABI, but I can't find a specific reason not to. I wouldn't object if lld continuing conforming to the standard x86 TLS ABI, though.

Aside: I don't think lld currently knows when it's targeting Android?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53906