[PATCH] D82899: [ELF] Resolve R_DTPREL in .debug_* referencing discarded symbols to -1

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D82899#2126519 <https://reviews.llvm.org/D82899#2126519>, @MaskRay wrote:

> In D82899#2124984 <https://reviews.llvm.org/D82899#2124984>, @jhenderson wrote:
>
> > In D82899#2124699 <https://reviews.llvm.org/D82899#2124699>, @jhenderson wrote:
> >
> > > Sorry, I don't follow why we can't just use this approach for all relocations referencing dead allocatable code/data? We should probably only get certain relocations, or b) not fixing up things when a new relocation gets introduced in the future.
> >
>
>
> If we want to support R_X86_64_32, we'll need to truncate UINT64_MAX (R_X86_64_32 calls checkUInt(..., 32), UINT64_MAX will cause a relocation error), i.e. we'll need some code somewhere. I'd rather do it with existing `type == target->symbolicRel`.


I see your point, but isn't the truncation going to be required for R_X86_64_DTPOFF32 anyway? Related aside: could you make sure that code path is tested too, please, since it's a real use-case, and highlighted a bug in my local attempt at fixing what this patch fixes. More generally, if you add the truncation code, you could drop the relocation check entirely, so you're not really changing the code complexity, whilst improving the code robustness in my opinion.

>> but I could see us either a) easily missing another relevant one
> 
> No, there is not a need for another R_ABS relocation. For new relocation types, we should evaluate them, rather than being loose and using -1 for every new relocation type.
> 
>> To add to this, reading up on the TLS documentation, at least for X86_64, it is possible to sometimes use R_X86_64_32 instead of R_X86_64_DTPOFF32, although I don't know if that situation would ever occur in debug data.
> 
> R_X86_64_{32,64} referencing a STT_TLS is incorrect.

That statement is not the same as my understanding of the ps-ABI doc though. See page 76 of https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-1.0.pdf:

> R_X86_64_DTPOFF64 and R_X86_64_DTPOFF32 compute the offset from the pointer in that entry to the referenced symbol. The linker generates such relocations in adjacent entries in the GOT, in response to R_X86_64_TLSGD and R_X86_64_TLSLD relocations. If the linker can compute the offset itself, because the referenced symbol binds locally, the relocations R_X86_64_64 and R_X86_64_32 may be used instead.

My reading of this says if a compiler knows that the symbol would be locally bound (presumably because the symbol is `static`/in an anonymous namespace?), it can emit an R_X86_64_32 instead of R_X86_64_DTPOFF32. That being said, although I could get gcc to emit a DTPOFF32 relocation, I wasn't able to persuade it to emit an ABS 32 relocation for the same case.

> 
> 
> - R_X86_64_64: load_base + st_value + r_addend
> - R_X86_64_DTPOFFSET{32,64}: st_value + r_addend - DTP_OFFSET
> 
>   DTP_OFFSET is musl term. glibc calls it TLS_DTV_OFFSET. On x86-64, DTP_OFFSET is 0 so mixing is correct if

This sentence is unfinished?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82899