[PATCH] D59553: [WIP][LLD][ELF][DebugInfo] Use predefined value to mark debug address ranges pointing to deleted code.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D59553#2083604 <https://reviews.llvm.org/D59553#2083604>, @probinson wrote:

> In D59553#2083362 <https://reviews.llvm.org/D59553#2083362>, @dblaikie wrote:
>
> > Ah @MaskRay posted the link to the DWARF issue ( http://www.dwarfstd.org/ShowIssue.php?issue=200609.1 ) in another thread.
>
>
> I'm not aware of that other thread.


https://sourceware.org/pipermail/binutils/2020-June/111640.html I will share results here or our llvm-dev thread if there is useful feedback.

>> In lieu of somewhere better to discuss it, just a couple of points to make, @probinson :
>> 
>> Currently if two inline functions are /identical/, then at least with the unix linkers I know of (bfd, gold, lld), to the best of my knowledge - the linker resolves relocations to either original copy to the final copy - so both CUs point to the same function. (with the complications as you pointed out of overlapping aranges, CU ranges, etc)
>>  If the inline functions aren't identical, then the preserved copy is only pointed to by the DWARF that originally described it - that's important because different optimizations on the function could invalidate the DWARF (eg: one file claims the variable A is in register B, the other claims it is in register C - if both subprogram descriptions point to the final copy, one of them will be incorrect)
>>  As for Identical Code Folding - I think it might actually be valuable to preserve that information and have both subprograms point to the same code (even with overlapping aranges/CU ranges/etc). But I could understand if that's a problem for some consumers - probably the sort of thing I'd be inclined to/hope DWARF allows, even if it suggests some implementations might choose not to take advantage of it.
>> 
>> (summary-ish:
>>  comdat functions that are optimized differently: subprograms must not refer to the differently optimized versio
>>  comdat functions that are optimized identically: multiple subprograms could point to the same code, but there's probably little benefit/reason to do so
>>  identical unrelated functions: there's value in multiple subprograms pointing to the same code)
> 
> No dispute with any of the above.  In some respects, yeah I glossed over some things or got details wrong, but the point is:  There used to be more copies of a thing, now there are fewer, what is appropriate for DWARF to say about that?  Allowing a more efficient way to say "not here boss" is what matters.



  // clang -g -c a.c
  // ld.lld --icf=all a.o
  void foo() {} // .text.foo
  void bar() {} // .text.bar , folded by ICF

Resolving a relocation referencing `.text.bar` to the tombstone value will make .debug_* diverge more from other non-SHF_ALLOC sections. It is not difficult to implement, though:
`if (Defined *ds = dyn_cast<Defined>(&sym)) ds->section->repl == ds->section` => (not folded by ICF)

>> Wording-wise: I'd have thought DWARF would be a bit more generic rather than talking about how producers might produce code, or how linkers might be involved in that process - just saying formally "-1 based addresses should be treated the same way as if the attribute were not present" - if a producer, for whatever reason, wants to produce a -1 address up-front rather than only by the linker, doesn't seem like DWARF shuold care/have an opinion on that.
> 
> DWARF already acknowledges the existence of COMDAT and linkers, for purposes of motivating type sections.  A similar motivation seems necessary here.  But as you say, it's probably not necessary to intrude that into the formal specification, and just have it say there are two ways to say it's not there (omit the attributes, or use the tombstone).  I thought about moving that part out into nonnormative and probably should have followed that impulse.
> 
> If you think there would be real value to it (and knowing that the DWARF committee is not actively meeting at the moment, so it will be a while before this gets discussed), I can resubmit the issue with a cleaner writeup and mark the current one as superseded.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59553