[PATCH] D110363: [DWARF][NFC] add ParentIdx and SiblingIdx to DWARFDebugInfoEntry for faster navigation.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 28 05:55:24 PDT 2021


avl added a comment.

After some thinking, it looks like this idea "So the DWARFUnit class no longer needs to be involved in the getting the parent, sibling, child stuff" may not be suitable for dies navigation. The reason for this is that DWARFDebugInfoEntry does not know the size of DWARFDebugInfoEntry vector. That makes it impossible to assert and/or check whether new pointers to the DWARFDebugInfoEntry-es are valid. i.e.

a) we could not write assertions for the proper value of DWARFDebugInfoEntry pointer.
b) we could not stop parsing if we passes out of DWARFDebugInfoEntry vector.

f.e. :

1.

    DWARFDie DWARFUnit::getFirstChild(const DWARFDebugInfoEntry *Die) {
    if (!Die->hasChildren())
      return DWARFDie();
  
    // We do not want access out of bounds when parsing corrupted debug data.
    size_t I = getDIEIndex(Die) + 1;
    if (I >= DieArray.size())    <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
      return DWARFDie();
    return DWARFDie(this, &DieArray[I]);
  }

We could not do the same check inside DWARFDebugInfoEntry::getFirstChild()

2.



  DWARFDie DWARFUnit::getLastChild(const DWARFDebugInfoEntry *Die) {
  ...
    uint32_t ChildIdx = DieIdx + 1;
    while (ChildIdx < DieArray.size()) {   <<<<<<<<<<<<<<<<<<<<
      assert(*DieArray[ChildIdx].getParentIdx() == DieIdx && "Bad parent");
  
      if (DieArray[ChildIdx].getTag() == dwarf::DW_TAG_null)
        return DWARFDie(this, &DieArray[ChildIdx]);
  
      if (Optional<uint32_t> Idx = DieArray[ChildIdx].getSiblingIdx())
        ChildIdx = *Idx;
      else
        // Return empty die if DWARF is corrupted.
        return DWARFDie();
    }

We could not do the same check inside DWARFDebugInfoEntry::getLastChild()

3.



  if (Optional<uint32_t> SiblingIdx = Die->getSiblingIdx()) {
    assert(*SiblingIdx < DieArray.size() &&   <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
           "SiblingIdx is out of DieArray boundaries");
    assert(DieArray[*SiblingIdx - 1].getTag() == dwarf::DW_TAG_null &&
           "Bad end of children marker");
    return DWARFDie(this, &DieArray[*SiblingIdx - 1]);
  }

above assertion could not be done inside DWARFDebugInfoEntry.

I propose to use current solution when DWARFUnit does dies navigation and properly validates elements. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110363



More information about the llvm-commits mailing list