[llvm] [DebugInfo] Change return type of DWARFDebugAbbrev::parse (PR #67191)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-objectyaml

<details>
<summary>Changes</summary>

To make DWARFDebugAbbrev more amenable to error-handling, I would like to change the return type of DWARFDebugAbbrev::parse from `void` to `Error`. Users of DWARFDebugAbbrev can consume the error if they want to use all the valid DWARF that was parsed (without worrying about the malformed DWARF) or stop when the parse fails if the use case needs to be strict.

This also will bring the LLVM DWARFDebugAbbrev interface closer to LLDB's which opens up the opportunity for LLDB adopt the LLVM implementation with minimal changes.

---
Full diff: https://github.com/llvm/llvm-project/pull/67191.diff


5 Files Affected:

- (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h (+1-1) 
- (modified) llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp (+8-6) 
- (modified) llvm/tools/obj2yaml/dwarf2yaml.cpp (+10-3) 
- (modified) llvm/tools/obj2yaml/macho2yaml.cpp (+2-4) 
- (modified) llvm/tools/obj2yaml/obj2yaml.h (+1-1) 


``````````diff
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
index 8e4aa3aa61e9ea5..6439827ef70f0f0 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h
@@ -70,7 +70,7 @@ class DWARFDebugAbbrev {
   getAbbreviationDeclarationSet(uint64_t CUAbbrOffset) const;
 
   void dump(raw_ostream &OS) const;
-  void parse() const;
+  Error parse() const;
 
   DWARFAbbreviationDeclarationSetMap::const_iterator begin() const {
     assert(!Data && "Must call parse before iterating over DWARFDebugAbbrev");
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
index 3014e61f566a909..85959ecc5e17f14 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp
@@ -105,9 +105,9 @@ std::string DWARFAbbreviationDeclarationSet::getCodeRange() const {
 DWARFDebugAbbrev::DWARFDebugAbbrev(DataExtractor Data)
     : AbbrDeclSets(), PrevAbbrOffsetPos(AbbrDeclSets.end()), Data(Data) {}
 
-void DWARFDebugAbbrev::parse() const {
+Error DWARFDebugAbbrev::parse() const {
   if (!Data)
-    return;
+    return Error::success();
   uint64_t Offset = 0;
   auto I = AbbrDeclSets.begin();
   while (Data->isValidOffset(Offset)) {
@@ -116,17 +116,19 @@ void DWARFDebugAbbrev::parse() const {
     uint64_t CUAbbrOffset = Offset;
     DWARFAbbreviationDeclarationSet AbbrDecls;
     if (Error Err = AbbrDecls.extract(*Data, &Offset)) {
-      // FIXME: We should propagate the error upwards.
-      consumeError(std::move(Err));
-      break;
+      Data = std::nullopt;
+      return Err;
     }
     AbbrDeclSets.insert(I, std::make_pair(CUAbbrOffset, std::move(AbbrDecls)));
   }
   Data = std::nullopt;
+  return Error::success();
 }
 
 void DWARFDebugAbbrev::dump(raw_ostream &OS) const {
-  parse();
+  if (Error Err = parse())
+    // FIXME: We should propagate this error or otherwise display it.
+    llvm::consumeError(std::move(Err));
 
   if (AbbrDeclSets.empty()) {
     OS << "< EMPTY >\n";
diff --git a/llvm/tools/obj2yaml/dwarf2yaml.cpp b/llvm/tools/obj2yaml/dwarf2yaml.cpp
index 3d3798d5f18d784..f2c96968cc7deeb 100644
--- a/llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ b/llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -22,11 +22,12 @@
 
 using namespace llvm;
 
-void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
+Error dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
   auto AbbrevSetPtr = DCtx.getDebugAbbrev();
   if (AbbrevSetPtr) {
     uint64_t AbbrevTableID = 0;
-    AbbrevSetPtr->parse();
+    if (Error Err = AbbrevSetPtr->parse())
+      return Err;
     for (auto AbbrvDeclSet : *AbbrevSetPtr) {
       Y.DebugAbbrev.emplace_back();
       Y.DebugAbbrev.back().ID = AbbrevTableID++;
@@ -48,6 +49,7 @@ void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
       }
     }
   }
+  return Error::success();
 }
 
 Error dumpDebugAddr(DWARFContext &DCtx, DWARFYAML::Data &Y) {
@@ -220,7 +222,12 @@ void dumpDebugInfo(DWARFContext &DCtx, DWARFYAML::Data &Y) {
     if (NewUnit.Version >= 5)
       NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
     const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
-    DebugAbbrev->parse();
+    // FIXME: Ideally we would propagate this error upwards, but that would
+    // prevent us from displaying any debug info at all. For now we just consume
+    // the error and display everything that was parsed successfully.
+    if (Error Err = DebugAbbrev->parse())
+      llvm::consumeError(std::move(Err));
+
     NewUnit.AbbrevTableID = std::distance(
         DebugAbbrev->begin(),
         llvm::find_if(
diff --git a/llvm/tools/obj2yaml/macho2yaml.cpp b/llvm/tools/obj2yaml/macho2yaml.cpp
index ed678ea28a71735..efd43a5f1285f74 100644
--- a/llvm/tools/obj2yaml/macho2yaml.cpp
+++ b/llvm/tools/obj2yaml/macho2yaml.cpp
@@ -139,10 +139,8 @@ MachODumper::constructSection(MachO::section_64 Sec, size_t SecIndex) {
 
 static Error dumpDebugSection(StringRef SecName, DWARFContext &DCtx,
                               DWARFYAML::Data &DWARF) {
-  if (SecName == "__debug_abbrev") {
-    dumpDebugAbbrev(DCtx, DWARF);
-    return Error::success();
-  }
+  if (SecName == "__debug_abbrev")
+    return dumpDebugAbbrev(DCtx, DWARF);
   if (SecName == "__debug_aranges")
     return dumpDebugARanges(DCtx, DWARF);
   if (SecName == "__debug_info") {
diff --git a/llvm/tools/obj2yaml/obj2yaml.h b/llvm/tools/obj2yaml/obj2yaml.h
index e0204c6c608f4b1..03d7db5317acde1 100644
--- a/llvm/tools/obj2yaml/obj2yaml.h
+++ b/llvm/tools/obj2yaml/obj2yaml.h
@@ -46,7 +46,7 @@ struct Data;
 }
 } // namespace llvm
 
-void dumpDebugAbbrev(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
+llvm::Error dumpDebugAbbrev(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 llvm::Error dumpDebugAddr(llvm::DWARFContext &DCtx, llvm::DWARFYAML::Data &Y);
 llvm::Error dumpDebugARanges(llvm::DWARFContext &DCtx,
                              llvm::DWARFYAML::Data &Y);

``````````

</details>


https://github.com/llvm/llvm-project/pull/67191