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

Sun Aug 4 18:52:06 PDT 2019

MaskRay added a comment.

In D64930#1614131 <https://reviews.llvm.org/D64930#1614131>, @kib wrote:

> 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 ?

The new test aarch64-tls-vaddr-align.s checks p_vaddr%p_align = 0. Once D64906 <https://reviews.llvm.org/D64906> (PPC64) is accepted, I'll create an i386 patch, which involves ~40 test changes.

If you comment out the following part included in D64906 <https://reviews.llvm.org/D64906>:

  // ELF/Writer.cpp
        else if (Out::tlsPhdr && Out::tlsPhdr->firstSec == p->firstSec)
          cmd->addrExpr = [] {
            return alignTo(script->getDot(), config->maxPageSize) +
                   alignTo(script->getDot() % config->maxPageSize,
                           Out::tlsPhdr->p_align);
          };

aarch64-tls-vaddr-align.s will create a PT_TLS where p_vaddr%p_align!=0.

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D64930