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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D61824#1499796 <https://reviews.llvm.org/D61824#1499796>, @peter.smith wrote:

> I think that this will definitely fix the problem, but I'd like to understand a bit more about what glibc is doing and what assumptions it is making, there does seem to be some code that attempts to deal with a PT_TLS with p_vaddr != 0 modulo p_align. My understanding from looking at dt-tls.c _dl_determine_tlsoffset() which sets the offset of each tls block (with the executable being in slot 0). Does something like this:
>
> - offset = TLS_TCB_SIZE (hard-coded to 2 * (void*) pointers.
> - calculates firstbyte, which is derived from l_tls_firstbyte_offset which is PT_TLS p_vaddr % p_align. For the example in pr41527 this is 32 bytes
> - sets the l_tls_offset of slot 0 to alignto(offset, p_align) - firstbyte.
>
>   With an unpatched LLD we are expecting l_tls_offset to be alignto(offset, p_align).
>
>   Have I got that right, and can you point out what assumption glibc is making? I've not yet managed to convince myself what it is doing wrong? I think that this may be important in choosing between this and D61825 <https://reviews.llvm.org/D61825>. It may be that what we do now just happens to work for non-glibc but we may still not be entirely correct. If that is the case then this would favour this fix rather than D61825 <https://reviews.llvm.org/D61825>.
>
>   The other problem with D61825 <https://reviews.llvm.org/D61825> is that it forces a change to the clang driver to Android to always pass the flag through to LLD. That is something the Android toolchain folk will want to comment on.


Some background:

  GAP_ABOVE_TP = 2 * sizeof(void*)
  the offset of the main module = alignUp(GAP_ABOVE_TP, main_tls_align)
  the linker and the dynamic loader must agree upon this
  
  Android bionic: the offset of the main module must be >= 8 * sizeof(void*)
  freebsd rtld: good
  musl: if main_tls_align > GAP_ABOVE_TP, the offset of the main module was wrong before 1.1.20
  glibc: p_vaddr%p_align != 0 => wrong offset of the first module



> Have I got that right, and can you point out what assumption glibc is making?

Below is my understanding about how glibc is wrong. glibc/elf/dl-tls.c:250 (the `#elif TLS_DTV_AT_TP` part)

  // I don't know what this piece of code intended to do with non-zero firstbyte...
  off = roundup (offset, slotinfo[cnt].map->l_tls_align); // roundup(16, 64) = 64
  if (off - offset < firstbyte) // 64 - 16 ; 32
    off += slotinfo[cnt].map->l_tls_align;
  slotinfo[cnt].map->l_tls_offset = off - firstbyte; // 64 - 32 = 32, wrong, should be 64
  
  // If firstbyte is 0 (i.e. p_vaddr % l_tls_align == 0), this is simply:
  off = roundup (offset, slotinfo[cnt].map->l_tls_align);
  slotinfo[cnt].map->l_tls_offset = off;
  // and is correct

I hope Szabolcs can chime in as he is the maintainer of glibc's arm port..


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D61824