[DebugInfo] Add error handling to DWARFDebugAbbrev::getAbbreviationDeclarationSet

This gives us more meaningful information when
`getAbbreviationDeclarationSet` fails. Right now only
`verifyAbbrevSection` actually uses the error that it returns, but the
other call sites could be rewritten to take advantage of the returned error.

Differential Revision: https://reviews.llvm.org/D153459

Author: bulbazord

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h

@@ -66,7 +66,7 @@ class DWARFDebugAbbrev {
 public:
   DWARFDebugAbbrev(DataExtractor Data);
 
-  const DWARFAbbreviationDeclarationSet *
+  Expected<const DWARFAbbreviationDeclarationSet *>
   getAbbreviationDeclarationSet(uint64_t CUAbbrOffset) const;
 
   void dump(raw_ostream &OS) const;

llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp

@@ -139,32 +139,30 @@ void DWARFDebugAbbrev::dump(raw_ostream &OS) const {
   }
 }
 
-const DWARFAbbreviationDeclarationSet*
+Expected<const DWARFAbbreviationDeclarationSet *>
 DWARFDebugAbbrev::getAbbreviationDeclarationSet(uint64_t CUAbbrOffset) const {
   const auto End = AbbrDeclSets.end();
   if (PrevAbbrOffsetPos != End && PrevAbbrOffsetPos->first == CUAbbrOffset) {
-    return &(PrevAbbrOffsetPos->second);
+    return &PrevAbbrOffsetPos->second;
   }
 
   const auto Pos = AbbrDeclSets.find(CUAbbrOffset);
   if (Pos != End) {
     PrevAbbrOffsetPos = Pos;
-    return &(Pos->second);
+    return &Pos->second;
   }
 
-  if (Data && CUAbbrOffset < Data->getData().size()) {
-    uint64_t Offset = CUAbbrOffset;
-    DWARFAbbreviationDeclarationSet AbbrDecls;
-    if (Error Err = AbbrDecls.extract(*Data, &Offset)) {
-      // FIXME: We should propagate the error upwards.
-      consumeError(std::move(Err));
-      return nullptr;
-    }
-    PrevAbbrOffsetPos =
-        AbbrDeclSets.insert(std::make_pair(CUAbbrOffset, std::move(AbbrDecls)))
-            .first;
-    return &PrevAbbrOffsetPos->second;
-  }
+  if (!Data || CUAbbrOffset >= Data->getData().size())
+    return make_error<llvm::object::GenericBinaryError>(
+        "the abbreviation offset into the .debug_abbrev section is not valid");
+
+  uint64_t Offset = CUAbbrOffset;
+  DWARFAbbreviationDeclarationSet AbbrDecls;
+  if (Error Err = AbbrDecls.extract(*Data, &Offset))
+    return std::move(Err);
 
-  return nullptr;
+  PrevAbbrOffsetPos =
+      AbbrDeclSets.insert(std::make_pair(CUAbbrOffset, std::move(AbbrDecls)))
+          .first;
+  return &PrevAbbrOffsetPos->second;
 }

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

@@ -1040,8 +1040,16 @@ DWARFUnit::getLastChildEntry(const DWARFDebugInfoEntry *Die) const {
 }
 
 const DWARFAbbreviationDeclarationSet *DWARFUnit::getAbbreviations() const {
-  if (!Abbrevs)
-    Abbrevs = Abbrev->getAbbreviationDeclarationSet(getAbbreviationsOffset());
+  if (!Abbrevs) {
+    Expected<const DWARFAbbreviationDeclarationSet *> AbbrevsOrError =
+        Abbrev->getAbbreviationDeclarationSet(getAbbreviationsOffset());
+    if (!AbbrevsOrError) {
+      // FIXME: We should propagate this error upwards.
+      consumeError(AbbrevsOrError.takeError());
+      return nullptr;
+    }
+    Abbrevs = *AbbrevsOrError;
+  }
   return Abbrevs;
 }
 

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

@@ -150,8 +150,15 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData,
     AddrSize = DebugInfoData.getU8(Offset);
   }
 
-  if (!DCtx.getDebugAbbrev()->getAbbreviationDeclarationSet(AbbrOffset))
+  Expected<const DWARFAbbreviationDeclarationSet *> AbbrevSetOrErr =
+      DCtx.getDebugAbbrev()->getAbbreviationDeclarationSet(AbbrOffset);
+  if (!AbbrevSetOrErr) {
     ValidAbbrevOffset = false;
+    // FIXME: A problematic debug_abbrev section is reported below in the form
+    // of a `note:`. We should propagate this error there (or elsewhere) to
+    // avoid losing the specific problem with the debug_abbrev section.
+    consumeError(AbbrevSetOrErr.takeError());
+  }
 
   ValidLength = DebugInfoData.isValidOffset(OffsetStart + Length + 3);
   ValidVersion = DWARFContext::isSupportedVersion(Version);
@@ -302,14 +309,14 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) {
   if (!Abbrev)
     return 0;
 
-  const DWARFAbbreviationDeclarationSet *AbbrDecls =
+  Expected<const DWARFAbbreviationDeclarationSet *> AbbrDeclsOrErr =
       Abbrev->getAbbreviationDeclarationSet(0);
-  // FIXME: If we failed to get a DWARFAbbreviationDeclarationSet, it's possible
-  // that there are errors. We need to propagate the error from
-  // getAbbreviationDeclarationSet.
-  if (!AbbrDecls)
-    return 0;
+  if (!AbbrDeclsOrErr) {
+    error() << toString(AbbrDeclsOrErr.takeError()) << "\n";
+    return 1;
+  }
 
+  const auto *AbbrDecls = *AbbrDeclsOrErr;
   unsigned NumErrors = 0;
   for (auto AbbrDecl : *AbbrDecls) {
     SmallDenseSet<uint16_t> AttributeSet;

llvm/test/tools/llvm-dwarfdump/X86/empty-CU.s

@@ -1,6 +1,7 @@
 # RUN: llvm-mc %s -filetype obj -triple x86_64-apple-darwin -o - \
 # RUN: | not llvm-dwarfdump --verify --debug-info - \
 # RUN: | FileCheck %s
+# CHECK: error: unable to decode LEB128 at offset 0x00000005: malformed uleb128, extends past end
 # CHECK: error: Compilation unit without DIE.
 
         .section        __DWARF,__debug_info,regular,debug
@@ -17,5 +18,6 @@
 .byte 1    # Abbrev code
 .byte 0x11 # TAG_compile_unit
 .byte 0    # no children
-.byte 0    # no attributes
-.byte 0
+.byte 0    # EOM(1)
+.byte 0    # EOM(2)
+           # Intentionally missing EOM(3)