[PATCH] D154655: [DebugInfo] Force users of DWARFDebugAbbrev to call parse before iterating

[Alex Langford via Phabricator via llvm-commits]

bulbazord created this revision.
bulbazord added reviewers: aprantl, JDevlieghere, fdeazeve, jhenderson, dblaikie.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

In an attempt to make it easier to catch errors when parsing the
debug_abbrev section, we should force users to call `parse` before
calling `begin`. In a follow-up change, I will change the return type of
`parse` from `void` to `Error`.

I also explored using the fallible_iterator pattern instead of forcing
users to parse everything up front. I think it would be a useful and
interesting pattern to implement, but it would require more extensive
changes to both DWARFDebugAbbrev and its users. Because my top priority
is improving the safety around parsing debug_abbrev, I'm opting to
preserve existing behavior until I or somebody else has time to refactor
to be able to implement a fallible_iterator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154655

Files:
  llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
  llvm/tools/obj2yaml/dwarf2yaml.cpp


Index: llvm/tools/obj2yaml/dwarf2yaml.cpp
===================================================================
--- llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -26,6 +26,7 @@
   auto AbbrevSetPtr = DCtx.getDebugAbbrev();
   if (AbbrevSetPtr) {
     uint64_t AbbrevTableID = 0;
+    AbbrevSetPtr->parse();
     for (auto AbbrvDeclSet : *AbbrevSetPtr) {
       Y.DebugAbbrev.emplace_back();
       Y.DebugAbbrev.back().ID = AbbrevTableID++;
@@ -219,6 +220,7 @@
     if (NewUnit.Version >= 5)
       NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
     const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
+    DebugAbbrev->parse();
     NewUnit.AbbrevTableID = std::distance(
         DebugAbbrev->begin(),
         llvm::find_if(
Index: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
===================================================================
--- llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
+++ llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
@@ -73,7 +73,7 @@
   void parse() const;
 
   DWARFAbbreviationDeclarationSetMap::const_iterator begin() const {
-    parse();
+    assert(!Data && "Must call parse before iterating over DWARFDebugAbbrev");
     return AbbrDeclSets.begin();
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154655.537866.patch
Type: text/x-patch
Size: 1261 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230706/985f452b/attachment.bin>