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

Thu Jul 2 09:42:37 PDT 2020

MaskRay added a comment.

In D82899#2127303 <https://reviews.llvm.org/D82899#2127303>, @jhenderson wrote:

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

An R_ABS relocation is conceptually unsigned (representing an address) while R_DTPREL can be considered signed (an offset added to dtpmod). R_X86_64_DTPOFF32 does not need truncation because it does not call checkUInt32.

It is tested. See `.long global at dtpoff`

Note that GNU ld does not allow `.long foo - 1` (R_X86_64_32) if foo is 0. LLD's checkUInt32 is compatible with GNU ld.

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

This paragraph describes traditional Generic Dynamic/Local Dynamic behavior. `__tls_get_addr({dtpmod, st_value-DTP_OFFSET}) = __tls_get_addr({dtpmod, 0}) + st_value-DTP_OFFSET`

If `__tls_get_addr({dtpmod, 0})` has been precomputed, you can add `st_value - DTP_OFFSET` to the value to get the address of the TLS variable. To get st_value, you can use either R_X86_64_32 or R_X86_64_DTPOFF32.
R_X86_64_DTPOFF32 can be used because on x86, DTP_OFFSET is zero.

.debug_info DW_OP_GNU_push_tls_address/DW_OP_form_tls_address is a different context. I think it is conceptually incorrectly to say that R_X86_64_32 can be used for its operand. GCC/clang do not emit R_X86_64_32.

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

On x86-64, DTP_OFFSET is 0 so mixing can still produce correct results.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82899