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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Will be worth mentioning that this is dependent on D64906 <https://reviews.llvm.org/D64906> and D64903 <https://reviews.llvm.org/D64903>, I could guess at D64903 <https://reviews.llvm.org/D64903> but basic-aarch64.s didn't apply cleanly without D64903 <https://reviews.llvm.org/D64903>. I successfully did a 2 stage bootstrap of clang with ninja check-all on a native aarch64 machine and made a quick test to check the TP relative relocation. I struggled a bit to understand the description and the comments though. In particular the use of GAP_ABOVE_TP instead of TCB_SIZE, it also wasn't clear where the alignment calculation had come from. I've made a few suggestions.



================
Comment at: ELF/InputSection.cpp:617
     // followed by a variable amount of alignment padding, followed by the TLS
     // segment.
+    // The start of PT_TLS (p_vaddr) has TP offset (2*wordize +
----------------
The comment below is just an abbreviation of the code. Given that we have a more complex calculation to do than ld.bfd or ld.gold (IIUC they force the alignment of the first TLS section to p_align which means that the p_offset will be 0 mod p_align), I think we need a bit more explanation. Something like:
```
At run time TP will be aligned to p_align, followed by the TCB which is the size of 2 pointers, followed by alignment padding, then the TLS blocks starting with the local block. In the ELF file the p_vaddr will be congruent to p_offset modulo p_align we therefore add additional padding so that (TP + TCB_SIZE + padding) is congruent to p_vaddr modulo p_align. 
```
I also needed to reach for my operator precedence table more than once when looking at the expression. It may be worth splitting it up a bit, for example:
```
offsetIntoBlock = s.getVA(0)
tcbSize = config->wordsize * 2 
padSize = (tls->p_vaddr - tcbSize) & (tls->p_align - 1)
return tcbSize + padSize + offsetIntoBlock;
```


================
Comment at: test/ELF/aarch64-tls-vaddr-align.s:15
+
+## TP offset computation is a bit tricky if p_vaddr%p_align != 0.
+## GAP_ABOVE_TP = 16
----------------
Personally I find GAP_ABOVE_TP confusing. I think TCB_SIZE would be more useful here for example:

```
## At run time TP will point to a structure aligned on a p_align boundary:
## | TCB_SIZE | padding | TLS block 0 | ... |
##
## a at tprel = TCB_SIZE + padding + st_value(a)
## TCB_SIZE = 16 (2 pointers)
## padding is (p_vaddr - TCB_SIZE) & (p_align - 1)
## st_value(a) is the offset into TLS block 0
## 0x220200-0x2201cc + 16 + (0x2201cc-16 & 0x100-1) = 0x100
```



Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D64930