[dwarfdump] Fix off-by-one bug in accelerator table extractor.

[JDevlieghere]

Summary

This fixes a bug where the verifier was complaining about empty
accelerator tables. When the table is empty, its size is not a valid
offset as it points after the end of the section.

This patch also makes the extractor return llvm:Error instead of bool
for better error reporting in the verifier.

[davide]

LGTM.

include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h

public:
     }
   };
 
 
   DWARFAcceleratorTable(const DWARFDataExtractor &AccelSection,
                         DataExtractor StringSection)
       : AccelSection(AccelSection), StringSection(StringSection) {}
 
-  bool extract();
+  llvm::Error extract();
   uint32_t getNumBuckets();
   uint32_t getNumHashes();
   uint32_t getSizeHdr();
   uint32_t getHeaderDataLength();
   ArrayRef<std::pair<HeaderData::AtomType, HeaderData::Form>> getAtomsDesc();
   bool validateForms();
 
   /// Return information related to the DWARF DIE we're looking for when

lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp

 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstddef>
 #include <cstdint>
 #include <utility>
 
 using namespace llvm;
 
-bool DWARFAcceleratorTable::extract() {
+llvm::Error DWARFAcceleratorTable::extract() {
   uint32_t Offset = 0;
 
   // Check that we can at least read the header.
   if (!AccelSection.isValidOffset(offsetof(Header, HeaderDataLength)+4))
-    return false;
+    return make_error<StringError>("Section too small: cannot read header.",
+                                   inconvertibleErrorCode());
 
   Hdr.Magic = AccelSection.getU32(&Offset);
   Hdr.Version = AccelSection.getU16(&Offset);
   Hdr.HashFunction = AccelSection.getU16(&Offset);
   Hdr.NumBuckets = AccelSection.getU32(&Offset);
   Hdr.NumHashes = AccelSection.getU32(&Offset);
   Hdr.HeaderDataLength = AccelSection.getU32(&Offset);
 
   // Check that we can read all the hashes and offsets from the
   // section (see SourceLevelDebugging.rst for the structure of the index).
+  // We need to substract one because we're checking for an *offset* which is
+  // equal to the size for an empty table and hence pointer after the section.
   if (!AccelSection.isValidOffset(sizeof(Hdr) + Hdr.HeaderDataLength +
-                                  Hdr.NumBuckets*4 + Hdr.NumHashes*8))
-    return false;
+                                  Hdr.NumBuckets * 4 + Hdr.NumHashes * 8 - 1))
+    return make_error<StringError>(
+        "Section too small: cannot read buckets and hashes.",
+        inconvertibleErrorCode());
 
   HdrData.DIEOffsetBase = AccelSection.getU32(&Offset);
   uint32_t NumAtoms = AccelSection.getU32(&Offset);
 
   for (unsigned i = 0; i < NumAtoms; ++i) {
     uint16_t AtomType = AccelSection.getU16(&Offset);
     auto AtomForm = static_cast<dwarf::Form>(AccelSection.getU16(&Offset));
     HdrData.Atoms.push_back(std::make_pair(AtomType, AtomForm));
   }
 
   IsValid = true;
-  return true;
+  return Error::success();
 }
 
 uint32_t DWARFAcceleratorTable::getNumBuckets() { return Hdr.NumBuckets; }
 uint32_t DWARFAcceleratorTable::getNumHashes() { return Hdr.NumHashes; }
 uint32_t DWARFAcceleratorTable::getSizeHdr() { return sizeof(Hdr); }
 uint32_t DWARFAcceleratorTable::getHeaderDataLength() {
   return Hdr.HeaderDataLength;
 }

lib/DebugInfo/DWARF/DWARFContext.cpp

 getAccelTable(std::unique_ptr<DWARFAcceleratorTable> &Cache,
               const DWARFObject &Obj, const DWARFSection &Section,
               StringRef StringSection, bool IsLittleEndian) {
   if (Cache)
     return *Cache;
   DWARFDataExtractor AccelSection(Obj, Section, IsLittleEndian, 0);
   DataExtractor StrData(StringSection, IsLittleEndian, 0);
   Cache.reset(new DWARFAcceleratorTable(AccelSection, StrData));
-  Cache->extract();
+  if (Error E = Cache->extract())
+    llvm::consumeError(std::move(E));
   return *Cache;
 }
 
 const DWARFAcceleratorTable &DWARFContext::getAppleNames() {
   return getAccelTable(AppleNames, *DObj, DObj->getAppleNamesSection(),
                        DObj->getStringSection(), isLittleEndian());
 }
 

lib/DebugInfo/DWARF/DWARFVerifier.cpp

unsigned DWARFVerifier::verifyAccelTable(const DWARFSection *AccelSection,
 
   // Verify that the fixed part of the header is not too short.
   if (!AccelSectionData.isValidOffset(AccelTable.getSizeHdr())) {
     error() << "Section is too small to fit a section header.\n";
     return 1;
   }
 
   // Verify that the section is not too short.
-  if (!AccelTable.extract()) {
-    error() << "Section is smaller than size described in section header.\n";
+  if (Error E = AccelTable.extract()) {
+    error() << toString(std::move(E)) << '\n';
     return 1;
   }
 
   // Verify that all buckets have a valid hash index or are empty.
   uint32_t NumBuckets = AccelTable.getNumBuckets();
   uint32_t NumHashes = AccelTable.getNumHashes();
 
   uint32_t BucketsOffset =

test/DebugInfo/dwarfdump-accel.test

 CHECK:     {Atom[0]: [[READONLY]]}
 CHECK:     {Atom[0]: [[ASSIGN]]}
 CHECK:     {Atom[0]: [[SETASSIGN]]}
 
 Verify the debug info in the apple_names accelerator table.
 VERIFY: Verifying .apple_names...
 VERIFY-NEXT: Verifying .apple_types...
 VERIFY-NEXT: Verifying .apple_namespaces...
-VERIFY-NEXT: error: Section is smaller than size described in section header.
+VERIFY-NEXT: error: Section too small: cannot read buckets and hashes.
 VERIFY-NEXT: Verifying .apple_objc...
 VERIFY-NEXT: Errors detected.