[PATCH] D111436: [ELF][test] Add testing for dynamic TLS relocations in .debug_info

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

In D111436#3051813 <https://reviews.llvm.org/D111436#3051813>, @MaskRay wrote:

> Please place this in one of the `x86-64-tls-ld*.s` files. For ppc64 we use `ppc64-dtprel.s`

This test is testing the patching behaviour of DTPREL in non-alloc sections, specifically `.debug_info` because that's the usual use case, so I'm not sure it really fits with the `x86-64-tls-ld*.s` tests. Do you want me to rename this test to `x86-64-dtprel.s` or do you have another suggestion?



================
Comment at: lld/test/ELF/relocation-dyn-tls-debug-info.s:10
+
+# RUN: ld.lld --gc-sections -shared %t.o -o %t-gc
+# RUN: llvm-objdump -s --section=.debug_info %t-gc | FileCheck --check-prefix=GC %s
----------------
MaskRay wrote:
> Drop `--gc-sections` from this test. It doesn't do anything special.
> 
> The property is tested in `debug-dead-reloc*` files.
The focus of this testing is the following [[https://github.com/llvm/llvm-project/blob/943b3048484b7e3cf04f4d51c23c82fcece2185d/lld/ELF/InputSection.cpp#L940-L941|code]] from `InputSection::relocateNonAlloc` in `InputSection.cpp`:
```
if (tombstone ||
    (isDebug && (type == target->symbolicRel || expr == R_DTPREL))) {
```
This is why I added `--gc-sections` because although the actual behaviour now is not special there is this specific code path. However, thanks for spotting `debug-dead-reloc-tls.s` which already provides coverage. Not sure how I managed to miss this in my search for existing coverage. I will remove this `--gc-sections` test case.


================
Comment at: lld/test/ELF/relocation-dyn-tls-debug-info.s:31
+.section .debug_info,"", at progbits
+.quad t2 at DTPOFF
+.quad t0 at DTPOFF
----------------
MaskRay wrote:
> Two values should be sufficient.
> 
> You can use `.quad 0` or `.space` to add a gap if you think 0 as a value is too special.
Yes, two values are sufficient, the third value was really for the `--gc-sections` test case which is no longer required.


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

https://reviews.llvm.org/D111436