[PATCH] D60470: [DWARF] Prefer larger DW_AT_low_pc when constructing aranges

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D60470#1460670 <https://reviews.llvm.org/D60470#1460670>, @MaskRay wrote:

> > What does addr2line (or any other symbolizers, not that I know of others) do in these cases?
>
> addr2line doesn't use binary search. It iterates all ELF sections, finds the first section containing the address, then iterates DWARF CUs and finds the associated line table. It has the same problem


Any idea why this problem hasn't been much of an issue until now/recently (both your change here, and the LLD change being reviewed)?

> and is computationally more expensive:
> 
>   % addr2line -e res.out 0x2010f0
>   /tmp/c/not_used.cpp:2
> 
> 
> 
> 
>> How's this compare/contrast with the lld change that's being made to achieve similar results? (does this remove the need for the lld change?)
> 
> This patch drops one minor space optimization, and changes the arbitrary `*ValidCUs.begin()` (prefer smallest CUOffset) to `LiveCUs.rbegin()` (prefer largest LowPC). It thus fixes an issue similar to D54747 <https://reviews.llvm.org/D54747>. After this patch, for an address in a non-discarded CU, it will not be incorrectly attributed to the discarded CU if the discarded CU also contains that address.
> 
> The D54747 <https://reviews.llvm.org/D54747> lld change will resolve a relocation to a discarded `DW_AT_low_pc` to -2. It will make the `DW_AT_low_pc=0` scenario not happen for lld-linked executables/DSOs (note, ld.bfd and gold still use zero `DW_AT_low_pc`). It won't affect other overlapping CU scenarios. We still need a rule defensively and I think (largest LowPC) is probably better than (smallest CUOffset).




Repository:
  rL LLVM

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

https://reviews.llvm.org/D60470