[PATCH] D82828: [ELF] Don't resolve a relocation in .debug_line referencing an ICF folded symbol to the tombstone value

Tue Jun 30 09:12:18 PDT 2020

MaskRay marked 2 inline comments as done.
MaskRay added a comment.

In D82828#2121605 <https://reviews.llvm.org/D82828#2121605>, @dblaikie wrote:

> > Doing this for .debug_line has a problem (https://reviews.llvm.org/D81784#2116925 )
>
> I think that describes a different problem from the one you're trying to address here - that one is more (from my understanding) a quality-of-life thing. Where it might be nicer if you could set breakpoints on ICF eliminated functions, right?
>
> >   .debug_line may describe folded lines as having addresses UINT64_MAX or some wraparound small addresses.
>
> ^ that's kind of the goal of the feature, though. So that seems like "working as intended" (& describes the behavior of this feature in other sections too - and for gc'd (rather than ICF'd) code in debug_line still, right?)

As @jhenderson commented, this is more about debugging experience, not a "confused debugger" problem.

>> These entries can confuse debuggers
> 
> Any idea why 0 entries don't confuse debuggers in the same way? Or did lld not use zero previously for ICF functions? (instead did it point all relocations to the ICF to the one copy - rather than making one real and the rest zero (well, zero+addend)?)

Previously, a relocation referencing a folded section symbol (e.g. `.text.1`) resolves to `.text`, not 0. So .debug_line has duplicate address entries but for different lines.
This patch restores the behavior.

> (I'm sort of on the fence about this - seems an unfortunate extra special case (especially applying it only to .debug_line - while the .debug_info, etc, still don't see these duplicates - which isn't necessarily better because overlapping debug_ranges might confuse some consumers, etc... ) where we've already got a few)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82828