[PATCH] D64930: [ELF][AArch64] Allow PT_LOAD to have overlapping p_offset ranges
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 30 20:40:05 PDT 2019
MaskRay added subscribers: kib, luporl.
MaskRay added a comment.
> 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 amd64:
tp = 0x8002278d0
&tlsvar = 0x8002212c8 # this should be 1-mod-4
tlsvar[1] = 7 # the value is correct
glibc (i386 and x86_64) has the same bug. It gets the static TLS block offsets correct.
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>).
Repository:
rLLD LLVM Linker
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64930/new/
https://reviews.llvm.org/D64930
More information about the llvm-commits
mailing list