[PATCH] D64930: [ELF][AArch64] Allow PT_LOAD to have overlapping p_offset ranges

[Konstantin Belousov via Phabricator via llvm-commits]

kib added a comment.

In D64930#1607709 <https://reviews.llvm.org/D64930#1607709>, @MaskRay wrote:

> > I'm not sure about breaking the (p_vaddr % p_align == 0) invariant. ld.bfd, gold, and (for the most part) lld currently provide this invariant for TLS segments in output files.
>
> I think p_vaddr%p_align = p_offset%p_align is the invariant we must satisfy. p_vaddr%p_align=0 is not. However, p_vaddr%p_align!=0 will expose bugs (runtime address not congruent to p_vaddr modulo p_align) in existing ld.so implementations so **I added a workaround to D64906 <https://reviews.llvm.org/D64906>.**
>
> //Below are discussions about bugs in various ld.so implementations. They should not be affected by these patches but ideally they should be fixed.//
>
> > Supporting n-mod-m alignment isn't free for dynamic TLS -- allocators typically have [posix_]memalign and free APIs, but I'm not aware of APIs that provide n-mod-m alignment.
>
> When allocating TCB+static TLS blocks, ld.so should properly pad the allocated space. See my comment added to InputSection.cpp in D64906 <https://reviews.llvm.org/D64906>:
>
>   // Variant 1. TP will be followed by an optional gap (which is the size of 2
>   // pointers on ARM/AArch64, 0 on other targets), followed by alignment
>   // padding, then the static TLS blocks. The alignment padding is added so that
>   // (TP + gap + padding) is congruent to p_vaddr modulo p_align.
>   //
>   // Variant 2. Static TLS blocks, followed by alignment padding are placed
>   // before TP. The alignment padding is added so that (TP - padding -
>   // p_memsz) is congruent to p_vaddr modulo p_align.
>   
>
>
>
> > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=24606
>
> This is a great test case! CC @kib who reviewed https://reviews.freebsd.org/D13378 @luporl for powerpc64. CC @emaste who is already subscribed...
>
> FreeBSD rtld-elf (FreeBSD 13.0-CURRENT #89 r350361M) doesn't make runtime address of PT_TLS congruent to p_vaddr modulo p_align on i386/amd64/arm64/powerpc64:
>
>   # amd64 case
>   tp          = 0x8002278d0
>   &tlsvar     = 0x8002212c8  # this should be 1-mod-4
>   tlsvar[1]   = 7            # the value is correct
>


I put (an experimental at this stage) review https://reviews.freebsd.org/D21163 to change the behavior of the FreeBSD dynamic linker.
With the patch applied, and with the test program from the sourceware bugzilla, I see on amd64:

  orion% ~/build/bsd/DEV/obj/usr/home/kostik/work/build/bsd/DEV/src/amd64.amd64/libexec/rtld-elf/ld-elf.so.1 ./test1
  tp          = 0x8010448d0
  &tlsvar     = 0x80103d4a9
  tlsvar[1]   = 7

The address is 1 mod 4, as requested.

> glibc (i386 and x86_64) has the same bug. It gets the static TLS block offsets correct.
>  Its aarch64 port is correct (https://sourceware.org/bugzilla/show_bug.cgi?id=24606).
> 
> musl i386 and x86_64 has been correct for many years:
> 
>   % ./test1
>   tp          = 0x7f2697b10cc8
>   &tlsvar     = 0x7f2697b113b9  # 1-mod-4
>   tlsvar[1]   = 7
> 
> 
> musl aarch64 (and likely arm/powerpc/powerpc64) was incorrect before 1.1.23 .
>  (Fixed by nsz's "fix tls offsets when p_vaddr%p_align != 0 on TLS_ABOVE_TP targets")
> 
>> I think there's an interoperability/compatibility issue. Like current Bionic, I don't think the BSDs pay attention to the (p_vaddr % p_align) value, for static or dynamic TLS. glibc uses that value for static TLS segments, but not for dynamic TLS[1]. (i.e. With this change, I believe lld can generate DSOs where some .tbss variables are misaligned when dlopen'ed with glibc.)
> 
> It'd be nice if Bionic can fix the problem :) Though it may hardly happen on Bionic arm/aarch64 because .tdata is overaligned to 64.
> 
> `p_vaddr(PT_TLS) % p_align(PT_TLS) != 0` can only happen if `sh_addralign(.tdata) < sh_addralign(.tbss)`.
>  So you may have to align .tbss to >=128 to verify.
> 
> FreeBSD definitely paid attention to the alignment of PT_TLS:
> 
>   // lib/libc/gen/tls.c
>    * +----------+--------------+--------------+-----------+------------------+
>    * | pre gap  | extended TCB |     TCB      | post gap  |    TLS segment   |
>    * | pre_size |  extra_size  | TLS_TCB_SIZE | post_size | tls_static_space |
>    * +----------+--------------+--------------+-----------+------------------+
>    *
>    * where:
>    *  extra_size is tcbsize - TLS_TCB_SIZE
>    *  post_size is used to adjust TCB to TLS aligment for first version of TLS
>    *            layout and is always 0 for second version.
>    *  pre_size  is used to adjust TCB aligment for first version and to adjust
>    *            TLS alignment for second version.
> 
> 
> Though amd64 is know to have problems as to dynamic TLS blocks. aarch64 may likely have static TLS offset problems (according to the experiments in D62055 <https://reviews.llvm.org/D62055>).

Can you provide the test for static TLS on amd64/i386 ?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D64930