[PATCH] D107554: [DWARF][Verifier] Do not add child DieRangeInfo with empty address range to the parent.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D107554#2929474 <https://reviews.llvm.org/D107554#2929474>, @avl wrote:

> In D107554#2929390 <https://reviews.llvm.org/D107554#2929390>, @dblaikie wrote:
>
>> Is this only for DIEs that have ranges (I guess DIEs that have no ranges/high/low they'll go through the error path at line 379? or do DIEs without those attributes silently create an empty range?)?
>
> DIEs without those attributes create an empty range. DWARFDie::getAddressRanges() function returns DWARFAddressRangesVector() for all dies that have no ranges/high/low. Thus, it would not go through the error path at line 379. Instead it would create an empty DieRangeInfo RI(RI.Ranges.empty());

Ah, yeah, if this produces empty ranges for every DIE that has nothing to do with addresses... then yeah, that's going to be a problem/this is a good thing to do for sure, it seems to me.

>> If it's only for DIEs with ranges (high/low/ranges) - then I guess they only end up empty if using the new tombstone addresses or using the now default lld/bfd relocations and ranges (or address encoding (rather than length encoding) for DW_AT_high_pc), so the address ranges end up empty? (rather than gold's tombstoning which would produce zero-based but non-empty ranges)
>
> That would be another case. RI.Ranges would be non empty in this case. It would contain an empty address range.

Ah *nod* wouldn't be covered here. Perhaps a more general fix could be made to the way DieRangeInfo does its work? Specifically DieRangeInfo::intersects(DieRangeInfo) - perhaps it could favor whichever has the smaller list? (currently it iterates all the LHS and checks them against the RHS - but usually, especially for this use in the verifier, the LHS will be much larger (containing the aggregate of all the ranges seen so far) than the RHS - I'd hazard that even just always flipping it (always iterating RHS to check for presence in LHS) would be faster, but to be general/symmetric, perhaps checking which is smaller and iterating that would be best). That would let the empty case fall out naturally, and hopefully make the several zero-length ranges (like linker-gc'd functions) better too, hopefully?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107554