[PATCH] D125779: [llvm-debuginfo-analyzer] 04 - Locations and ranges

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:762
+LVScope *LVScope::outermostParent(LVAddress Address) {
+  LVScope *Parent = this;
+  while (Parent) {
----------------
CarlosAlbertoEnciso wrote:
> probinson wrote:
> > Looks like if Ranges is null, this ends up returning null.  So, cheaper to do
> > ```
> > if (!Ranges)
> >   return nullptr;
> > ```
> > and then remove the `if (Ranges)` from inside the while loop.
> I think you found an issue with the code.
> The `Ranges` is not updated when we move scopes. We must use the `Ranges` for the new scope and check if the given `Address` is contained in the specific set.
> 
> ```
> LVScope *LVScope::outermostParent(LVAddress Address) {
>   LVScope *Parent = this;
>   while (Parent) {
>     const LVLocations *ParentRanges = Parent->getRanges();
>     if (ParentRanges)
>       for (const LVLocation *Location : *ParentRanges)
>         if (Location->getLowerAddress() <= Address)
>           return Parent;
>     Parent = Parent->getParentScope();
>   }
>   return Parent;
> }
> 
> ```
> 
The internal regression suite detected 2 cases where the location coverage changed from:
`0.00 -> 19.05` and `0.00 -> 23.53`.


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

https://reviews.llvm.org/D125779