Update llvm-readobj to re-use parsing code

[zturner]

Summary

llvm-readobj has a lot of hand rolled CodeView parsing code. Recently I introduced codeview::StringTableRef and codeview::StringTable which can read and write codeview stringtables from and to its serialized format. This patch updates llvm-readobj to use StringTableRef for parsing the string table, and since I'm in the code anyway, updates the file checksum parsing code to use VarStreamArray<FileChecksumEntry> which simplifies some error handling and parsing code.

[rnk]

llvm/include/llvm/DebugInfo/CodeView/StringTable.h

39 ↗

(On Diff #97522)

I went back and forth on this change, but I think it's a good idea. It's consistent with the way we did consumeObject etc.

llvm/trunk/include/llvm/DebugInfo/CodeView/StringTable.h

 /// Represents a read-only view of a CodeView string table.  This is a very
 /// simple flat buffer consisting of null-terminated strings, where strings
 /// are retrieved by their offset in the buffer.  StringTableRef does not own
 /// the underlying storage for the buffer.
 class StringTableRef {
 public:
   StringTableRef();
 
-  Error initialize(BinaryStreamReader &Stream);
+  Error initialize(BinaryStreamRef Contents);
 
-  StringRef getString(uint32_t Offset) const;
+  Expected<StringRef> getString(uint32_t Offset) const;
+
+  bool valid() const { return Stream.valid(); }
 
 private:
   BinaryStreamRef Stream;
 };
 
 /// Represents a read-write view of a CodeView string table.  StringTable owns
 /// the underlying storage for the table, and is capable of serializing the
 /// string table into a format understood by StringTableRef.

llvm/trunk/include/llvm/DebugInfo/CodeView/SymbolVisitorDelegate.h

 #include <cstdint>
 
 namespace llvm {
 
 class BinaryStreamReader;
 
 namespace codeview {
 
+class StringTableRef;
+
 class SymbolVisitorDelegate {
 public:
   virtual ~SymbolVisitorDelegate() = default;
 
   virtual uint32_t getRecordOffset(BinaryStreamReader Reader) = 0;
   virtual StringRef getFileNameForFileOffset(uint32_t FileOffset) = 0;
-  virtual StringRef getStringTable() = 0;
+  virtual StringTableRef getStringTable() = 0;
 };
 
 } // end namespace codeview
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_SYMBOLVISITORDELEGATE_H

llvm/trunk/include/llvm/DebugInfo/PDB/Native/PDBStringTable.h

-//===- PDBStringTable.h - PDB String Table -------------------------*- C++
-//-*-===//
+//===- PDBStringTable.h - PDB String Table -----------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 

 public:
   Error reload(BinaryStreamReader &Reader);
 
   uint32_t getByteSize() const;
   uint32_t getNameCount() const;
   uint32_t getHashVersion() const;
   uint32_t getSignature() const;
 
-  StringRef getStringForID(uint32_t ID) const;
-  uint32_t getIDForString(StringRef Str) const;
+  Expected<StringRef> getStringForID(uint32_t ID) const;
+  Expected<uint32_t> getIDForString(StringRef Str) const;
 
   FixedStreamArray<support::ulittle32_t> name_ids() const;
 
 private:
   Error readHeader(BinaryStreamReader &Reader);
   Error readStrings(BinaryStreamReader &Reader);
   Error readHashTable(BinaryStreamReader &Reader);
   Error readEpilogue(BinaryStreamReader &Reader);

llvm/trunk/include/llvm/Support/BinaryStreamRef.h

BinaryStreamRef(BinaryStream &Stream)
       : BinaryStreamRefBase(Stream, 0, Stream.getLength()) {}
   BinaryStreamRef(BinaryStream &Stream, uint32_t Offset, uint32_t Length)
       : BinaryStreamRefBase(Stream, Offset, Length) {}
 
   // Use BinaryStreamRef.slice() instead.
   BinaryStreamRef(BinaryStreamRef &S, uint32_t Offset,
                   uint32_t Length) = delete;
 
+  /// Check if a Stream is valid.
+  bool valid() const { return Stream != nullptr; }
+
   /// Given an Offset into this StreamRef and a Size, return a reference to a
   /// buffer owned by the stream.
   ///
   /// \returns a success error code if the entire range of data is within the
   /// bounds of this BinaryStreamRef's view and the implementation could read
   /// the data, and an appropriate error code otherwise.
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const {

llvm/trunk/lib/DebugInfo/CodeView/StringTable.cpp

 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/BinaryStreamWriter.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
 StringTableRef::StringTableRef() {}
 
-Error StringTableRef::initialize(BinaryStreamReader &Reader) {
-  return Reader.readStreamRef(Stream, Reader.bytesRemaining());
+Error StringTableRef::initialize(BinaryStreamRef Contents) {
+  Stream = Contents;
+  return Error::success();
 }
 
-StringRef StringTableRef::getString(uint32_t Offset) const {
+Expected<StringRef> StringTableRef::getString(uint32_t Offset) const {
   BinaryStreamReader Reader(Stream);
   Reader.setOffset(Offset);
   StringRef Result;
-  Error EC = Reader.readCString(Result);
-  assert(!EC);
-  consumeError(std::move(EC));
+  if (auto EC = Reader.readCString(Result))
+    return std::move(EC);
   return Result;
 }
 
 uint32_t StringTable::insert(StringRef S) {
   auto P = Strings.insert({S, StringSize});
 
   // If a given string didn't exist in the string table, we want to increment
   // the string table size.

llvm/trunk/lib/DebugInfo/CodeView/SymbolDumper.cpp

 //===-- SymbolDumper.cpp - CodeView symbol info dumper ----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/CodeView/SymbolDumper.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/DebugInfo/CodeView/CVSymbolVisitor.h"
 #include "llvm/DebugInfo/CodeView/CVTypeDumper.h"
 #include "llvm/DebugInfo/CodeView/EnumTables.h"
+#include "llvm/DebugInfo/CodeView/StringTable.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolDumpDelegate.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/SymbolVisitorCallbackPipeline.h"
 #include "llvm/DebugInfo/CodeView/SymbolVisitorCallbacks.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
Error CVSymbolDumperImpl::visitKnownRecord(
   return Error::success();
 }
 
 Error CVSymbolDumperImpl::visitKnownRecord(
     CVSymbol &CVR, DefRangeSubfieldSym &DefRangeSubfield) {
   DictScope S(W, "DefRangeSubfield");
 
   if (ObjDelegate) {
-    StringRef StringTable = ObjDelegate->getStringTable();
-    auto ProgramStringTableOffset = DefRangeSubfield.Program;
-    if (ProgramStringTableOffset >= StringTable.size())
+    StringTableRef Strings = ObjDelegate->getStringTable();
+    auto ExpectedProgram = Strings.getString(DefRangeSubfield.Program);
+    if (!ExpectedProgram) {
+      consumeError(ExpectedProgram.takeError());
       return llvm::make_error<CodeViewError>(
           "String table offset outside of bounds of String Table!");
-    StringRef Program =
-        StringTable.drop_front(ProgramStringTableOffset).split('\0').first;
-    W.printString("Program", Program);
+    }
+    W.printString("Program", *ExpectedProgram);
   }
   W.printNumber("OffsetInParent", DefRangeSubfield.OffsetInParent);
   printLocalVariableAddrRange(DefRangeSubfield.Range,
                               DefRangeSubfield.getRelocationOffset());
   printLocalVariableAddrGap(DefRangeSubfield.Gaps);
   return Error::success();
 }
 
 Error CVSymbolDumperImpl::visitKnownRecord(CVSymbol &CVR,
                                            DefRangeSym &DefRange) {
   DictScope S(W, "DefRange");
 
   if (ObjDelegate) {
-    StringRef StringTable = ObjDelegate->getStringTable();
-    auto ProgramStringTableOffset = DefRange.Program;
-    if (ProgramStringTableOffset >= StringTable.size())
+    StringTableRef Strings = ObjDelegate->getStringTable();
+    auto ExpectedProgram = Strings.getString(DefRange.Program);
+    if (!ExpectedProgram) {
+      consumeError(ExpectedProgram.takeError());
       return llvm::make_error<CodeViewError>(
           "String table offset outside of bounds of String Table!");
-    StringRef Program =
-        StringTable.drop_front(ProgramStringTableOffset).split('\0').first;
-    W.printString("Program", Program);
+    }
+    W.printString("Program", *ExpectedProgram);
   }
   printLocalVariableAddrRange(DefRange.Range, DefRange.getRelocationOffset());
   printLocalVariableAddrGap(DefRange.Gaps);
   return Error::success();
 }
 
 Error CVSymbolDumperImpl::visitKnownRecord(CVSymbol &CVR,
                                            FrameCookieSym &FrameCookie) {

llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTable.cpp

if (Header->HashVersion != 1 && Header->HashVersion != 2)
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Unsupported hash version");
 
   assert(Reader.bytesRemaining() == 0);
   return Error::success();
 }
 
 Error PDBStringTable::readStrings(BinaryStreamReader &Reader) {
-  if (auto EC = Strings.initialize(Reader)) {
+  BinaryStreamRef Stream;
+  if (auto EC = Reader.readStreamRef(Stream))
+    return EC;
+
+  if (auto EC = Strings.initialize(Stream)) {
     return joinErrors(std::move(EC),
                       make_error<RawError>(raw_error_code::corrupt_file,
                                            "Invalid hash table byte length"));
   }
 
   assert(Reader.bytesRemaining() == 0);
   return Error::success();
 }
Error PDBStringTable::reload(BinaryStreamReader &Reader) {
   std::tie(SectionReader, Reader) = Reader.split(sizeof(uint32_t));
   if (auto EC = readEpilogue(SectionReader))
     return EC;
 
   assert(Reader.bytesRemaining() == 0);
   return Error::success();
 }
 
-StringRef PDBStringTable::getStringForID(uint32_t ID) const {
+Expected<StringRef> PDBStringTable::getStringForID(uint32_t ID) const {
   return Strings.getString(ID);
 }
 
-uint32_t PDBStringTable::getIDForString(StringRef Str) const {
+Expected<uint32_t> PDBStringTable::getIDForString(StringRef Str) const {
   uint32_t Hash =
       (Header->HashVersion == 1) ? hashStringV1(Str) : hashStringV2(Str);
   size_t Count = IDs.size();
   uint32_t Start = Hash % Count;
   for (size_t I = 0; I < Count; ++I) {
     // The hash is just a starting point for the search, but if it
     // doesn't work we should find the string no matter what, because
     // we iterate the entire array.
     uint32_t Index = (Start + I) % Count;
 
     uint32_t ID = IDs[Index];
-    StringRef S = getStringForID(ID);
-    if (S == Str)
+    auto ExpectedStr = getStringForID(ID);
+    if (!ExpectedStr)
+      return ExpectedStr.takeError();
+
+    if (*ExpectedStr == Str)
       return ID;
   }
-  // IDs[0] contains the ID of the "invalid" entry.
-  return IDs[0];
+  return make_error<RawError>(raw_error_code::no_entry);
 }
 
 FixedStreamArray<support::ulittle32_t> PDBStringTable::name_ids() const {
   return IDs;
 }

llvm/trunk/tools/llvm-pdbdump/Diff.cpp

if (opts::diff::Pedantic) {
     // same order
     // in both tables.
     uint32_t Max = std::max(IdList1.size(), IdList2.size());
     for (uint32_t I = 0; I < Max; ++I) {
       Optional<uint32_t> Id1, Id2;
       StringRef S1, S2;
       if (I < IdList1.size()) {
         Id1 = IdList1[I];
-        S1 = ST1.getStringForID(*Id1);
+        if (auto Result = ST1.getStringForID(*Id1))
+          S1 = *Result;
+        else
+          return Result.takeError();
       }
       if (I < IdList2.size()) {
         Id2 = IdList2[I];
-        S2 = ST2.getStringForID(*Id2);
+        if (auto Result = ST2.getStringForID(*Id2))
+          S2 = *Result;
+        else
+          return Result.takeError();
       }
       if (Id1 == Id2 && S1 == S2)
         continue;
 
       std::string OutId1 =
           Id1 ? formatv("{0}", *Id1).str() : "(index not present)";
       std::string OutId2 =
           Id2 ? formatv("{0}", *Id2).str() : "(index not present)";
       outs() << formatv("  String {0}\n", I);
       outs() << formatv("    {0}: Hash - {1}, Value - {2}\n",
                         File1.getFilePath(), OutId1, S1);
       outs() << formatv("    {0}: Hash - {1}, Value - {2}\n",
                         File2.getFilePath(), OutId2, S2);
       HasDiff = true;
     }
   } else {
     std::vector<StringRef> Strings1, Strings2;
     Strings1.reserve(IdList1.size());
     Strings2.reserve(IdList2.size());
-    for (auto ID : IdList1)
-      Strings1.push_back(ST1.getStringForID(ID));
-    for (auto ID : IdList2)
-      Strings2.push_back(ST2.getStringForID(ID));
+    for (auto ID : IdList1) {
+      auto S = ST1.getStringForID(ID);
+      if (!S)
+        return S.takeError();
+      Strings1.push_back(*S);
+    }
+    for (auto ID : IdList2) {
+      auto S = ST2.getStringForID(ID);
+      if (!S)
+        return S.takeError();
+      Strings2.push_back(*S);
+    }
 
     SmallVector<StringRef, 64> OnlyP;
     SmallVector<StringRef, 64> OnlyQ;
     auto End1 = std::remove(Strings1.begin(), Strings1.end(), "");
     auto End2 = std::remove(Strings2.begin(), Strings2.end(), "");
     uint32_t Empty1 = std::distance(End1, Strings1.end());
     uint32_t Empty2 = std::distance(End2, Strings2.end());
     Strings1.erase(End1, Strings1.end());

llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp

if (!opts::raw::DumpStringTable)
     return Error::success();
 
   auto IS = File.getStringTable();
   if (!IS)
     return IS.takeError();
 
   DictScope D(P, "String Table");
   for (uint32_t I : IS->name_ids()) {
-    StringRef S = IS->getStringForID(I);
-    if (!S.empty()) {
-      llvm::SmallString<32> Str;
-      Str.append("'");
-      Str.append(S);
-      Str.append("'");
-      P.printString(Str);
-    }
-  }
+    auto ES = IS->getStringForID(I);
+    if (!ES)
+      return ES.takeError();
+
+    if (ES->empty())
+      continue;
+    Str.append(*ES);
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpInfoStream() {
   if (!opts::raw::DumpHeaders)
     return Error::success();
   if (!File.hasPDBInfoStream()) {
     P.printString("PDB Stream not present");
if (DumpTpiHash) {
 
     ListScope LHA(P, "Adjusters");
     auto ExpectedST = File.getStringTable();
     if (!ExpectedST)
       return ExpectedST.takeError();
     const auto &ST = *ExpectedST;
     for (const auto &E : Tpi->getHashAdjusters()) {
       DictScope DHA(P);
-      StringRef Name = ST.getStringForID(E.first);
-      P.printString("Type", Name);
+      auto Name = ST.getStringForID(E.first);
+      if (!Name)
+        return Name.takeError();
+
+      P.printString("Type", *Name);
       P.printHex("TI", E.second);
     }
   }
 
   if (!IsSilentDatabaseBuild) {
     ListScope L(P, "TypeIndexOffsets");
     for (const auto &IO : Tpi->getTypeIndexOffsets()) {
       P.printString(formatv("Index: {0:x}, Offset: {1:N}", IO.Type.getIndex(),

llvm/trunk/tools/llvm-pdbdump/YAMLOutputStyle.cpp

Error YAMLOutputStyle::dumpStringTable() {
 
   Obj.StringTable.emplace();
   auto ExpectedST = File.getStringTable();
   if (!ExpectedST)
     return ExpectedST.takeError();
 
   const auto &ST = ExpectedST.get();
   for (auto ID : ST.name_ids()) {
-    StringRef S = ST.getStringForID(ID);
-    if (!S.empty())
-      Obj.StringTable->push_back(S);
+    auto S = ST.getStringForID(ID);
+    if (!S)
+      return S.takeError();
+    if (S->empty())
+      continue;
+    Obj.StringTable->push_back(*S);
   }
   return Error::success();
 }
 
 Error YAMLOutputStyle::dumpStreamMetadata() {
   if (!opts::pdb2yaml::StreamMetadata)
     return Error::success();
 

llvm/trunk/tools/llvm-readobj/COFFDumper.cpp

 #include "llvm/ADT/StringExtras.h"
 #include "llvm/DebugInfo/CodeView/CVTypeDumper.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/CodeView/Line.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugFileChecksumFragment.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugInlineeLinesFragment.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugLineFragment.h"
 #include "llvm/DebugInfo/CodeView/RecordSerialization.h"
+#include "llvm/DebugInfo/CodeView/StringTable.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolDumpDelegate.h"
 #include "llvm/DebugInfo/CodeView/SymbolDumper.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeDumpVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
private:
   void printRelocatedField(StringRef Label, const coff_section *Sec,
                            uint32_t RelocOffset, uint32_t Offset,
                            StringRef *RelocSym = nullptr);
 
   void printBinaryBlockWithRelocs(StringRef Label, const SectionRef &Sec,
                                   StringRef SectionContents, StringRef Block);
 
   /// Given a .debug$S section, find the string table and file checksum table.
-  void initializeFileAndStringTables(StringRef Data);
+  void initializeFileAndStringTables(BinaryStreamReader &Reader);
 
   void cacheRelocations();
 
   std::error_code resolveSymbol(const coff_section *Section, uint64_t Offset,
                                 SymbolRef &Sym);
   std::error_code resolveSymbolName(const coff_section *Section,
                                     uint64_t Offset, StringRef &Name);
   std::error_code resolveSymbolName(const coff_section *Section,
                                     StringRef SectionContents,
                                     const void *RelocPtr, StringRef &Name);
   void printImportedSymbols(iterator_range<imported_symbol_iterator> Range);
   void printDelayImportedSymbols(
       const DelayImportDirectoryEntryRef &I,
       iterator_range<imported_symbol_iterator> Range);
 
   typedef DenseMap<const coff_section*, std::vector<RelocationRef> > RelocMapTy;
 
   const llvm::object::COFFObjectFile *Obj;
   bool RelocCached = false;
   RelocMapTy RelocMap;
-  StringRef CVFileChecksumTable;
-  StringRef CVStringTable;
+
+  BinaryByteStream ChecksumContents;
+  VarStreamArray<FileChecksumEntry> CVFileChecksumTable;
+
+  BinaryByteStream StringTableContents;
+  StringTableRef CVStringTable;
 
   ScopedPrinter &Writer;
   TypeDatabase TypeDB;
 };
 
 class COFFObjectDumpDelegate : public SymbolDumpDelegate {
 public:
   COFFObjectDumpDelegate(COFFDumper &CD, const SectionRef &SR,
void printBinaryBlockWithRelocs(StringRef Label,
     if (opts::CodeViewSubsectionBytes)
       CD.printBinaryBlockWithRelocs(Label, SR, SectionContents, SBlock);
   }
 
   StringRef getFileNameForFileOffset(uint32_t FileOffset) override {
     return CD.getFileNameForFileOffset(FileOffset);
   }
 
-  StringRef getStringTable() override { return CD.CVStringTable; }
+  StringTableRef getStringTable() override { return CD.CVStringTable; }
 
 private:
   COFFDumper &CD;
   const SectionRef &SR;
   const coff_section *Sec;
   StringRef SectionContents;
 };
 
void COFFDumper::printCodeViewDebugInfo() {
   for (const SectionRef &S : Obj->sections()) {
     StringRef SectionName;
     error(S.getName(SectionName));
     if (SectionName == ".debug$S")
       printCodeViewSymbolSection(SectionName, S);
   }
 }
 
-void COFFDumper::initializeFileAndStringTables(StringRef Data) {
-  while (!Data.empty() && (CVFileChecksumTable.data() == nullptr ||
-                           CVStringTable.data() == nullptr)) {
+void COFFDumper::initializeFileAndStringTables(BinaryStreamReader &Reader) {
+  while (Reader.bytesRemaining() > 0 &&
+         (!CVFileChecksumTable.valid() || !CVStringTable.valid())) {
     // The section consists of a number of subsection in the following format:
     // |SubSectionType|SubSectionSize|Contents...|
     uint32_t SubType, SubSectionSize;
-    error(consume(Data, SubType));
-    error(consume(Data, SubSectionSize));
-    if (SubSectionSize > Data.size())
-      return error(object_error::parse_failed);
+    error(Reader.readInteger(SubType));
+    error(Reader.readInteger(SubSectionSize));
+
+    StringRef Contents;
+    error(Reader.readFixedString(Contents, SubSectionSize));
+
     switch (ModuleDebugFragmentKind(SubType)) {
-    case ModuleDebugFragmentKind::FileChecksums:
-      CVFileChecksumTable = Data.substr(0, SubSectionSize);
-      break;
-    case ModuleDebugFragmentKind::StringTable:
-      CVStringTable = Data.substr(0, SubSectionSize);
+    case ModuleDebugFragmentKind::FileChecksums: {
+      ChecksumContents = BinaryByteStream(Contents, support::little);
+      BinaryStreamReader CSR(ChecksumContents);
+      error(CSR.readArray(CVFileChecksumTable, CSR.getLength()));
       break;
+    }
+    case ModuleDebugFragmentKind::StringTable: {
+      StringTableContents = BinaryByteStream(Contents, support::little);
+      error(CVStringTable.initialize(StringTableContents));
+    } break;
     default:
       break;
     }
+
     uint32_t PaddedSize = alignTo(SubSectionSize, 4);
-    if (PaddedSize > Data.size())
-      error(object_error::parse_failed);
-    Data = Data.drop_front(PaddedSize);
+    error(Reader.skip(PaddedSize - SubSectionSize));
   }
 }
 
 void COFFDumper::printCodeViewSymbolSection(StringRef SectionName,
                                             const SectionRef &Section) {
   StringRef SectionContents;
   error(Section.getContents(SectionContents));
   StringRef Data = SectionContents;
 
   SmallVector<StringRef, 10> FunctionNames;
   StringMap<StringRef> FunctionLineTables;
 
   ListScope D(W, "CodeViewDebugInfo");
   // Print the section to allow correlation with printSections.
   W.printNumber("Section", SectionName, Obj->getSectionID(Section));
 
   uint32_t Magic;
   error(consume(Data, Magic));
   W.printHex("Magic", Magic);
   if (Magic != COFF::DEBUG_SECTION_MAGIC)
     return error(object_error::parse_failed);
 
-  initializeFileAndStringTables(Data);
+  BinaryByteStream FileAndStrings(Data, support::little);
+  BinaryStreamReader FSReader(FileAndStrings);
+  initializeFileAndStringTables(FSReader);
 
   // TODO: Convert this over to using ModuleSubstreamVisitor.
   while (!Data.empty()) {
     // The section consists of a number of subsection in the following format:
     // |SubSectionType|SubSectionSize|Contents...|
     uint32_t SubType, SubSectionSize;
     error(consume(Data, SubType));
     error(consume(Data, SubSectionSize));
case ModuleDebugFragmentKind::FrameData: {
       W.printString("LinkageName", LinkageName);
 
       // To find the active frame description, search this array for the
       // smallest PC range that includes the current PC.
       while (!SR.empty()) {
         const FrameData *FD;
         error(SR.readObject(FD));
 
-        if (FD->FrameFunc >= CVStringTable.size())
-          error(object_error::parse_failed);
-
-        StringRef FrameFunc =
-            CVStringTable.drop_front(FD->FrameFunc).split('\0').first;
+        StringRef FrameFunc = error(CVStringTable.getString(FD->FrameFunc));
 
         DictScope S(W, "FrameData");
         W.printHex("RvaStart", FD->RvaStart);
         W.printHex("CodeSize", FD->CodeSize);
         W.printHex("LocalSize", FD->LocalSize);
         W.printHex("ParamsSize", FD->ParamsSize);
         W.printHex("MaxStackSize", FD->MaxStackSize);
         W.printString("FrameFunc", FrameFunc);
void COFFDumper::printCodeViewFileChecksums(StringRef Subsection) {
   BinaryByteStream S(Subsection, llvm::support::little);
   BinaryStreamReader SR(S);
   ModuleDebugFileChecksumFragmentRef Checksums;
   error(Checksums.initialize(SR));
 
   for (auto &FC : Checksums) {
     DictScope S(W, "FileChecksum");
 
-    if (FC.FileNameOffset >= CVStringTable.size())
-      error(object_error::parse_failed);
-    StringRef Filename =
-        CVStringTable.drop_front(FC.FileNameOffset).split('\0').first;
+    StringRef Filename = error(CVStringTable.getString(FC.FileNameOffset));
     W.printHex("Filename", Filename, FC.FileNameOffset);
     W.printHex("ChecksumSize", FC.Checksum.size());
     W.printEnum("ChecksumKind", uint8_t(FC.Kind),
                 makeArrayRef(FileChecksumKindNames));
 
     W.printBinary("ChecksumBytes", FC.Checksum);
   }
 }
if (Lines.hasExtraFiles()) {
         printFileNameForOffset("FileID", FID);
       }
     }
   }
 }
 
 StringRef COFFDumper::getFileNameForFileOffset(uint32_t FileOffset) {
   // The file checksum subsection should precede all references to it.
-  if (!CVFileChecksumTable.data() || !CVStringTable.data())
-    error(object_error::parse_failed);
-  // Check if the file checksum table offset is valid.
-  if (FileOffset >= CVFileChecksumTable.size())
+  if (!CVFileChecksumTable.valid() || !CVStringTable.valid())
     error(object_error::parse_failed);
 
-  // The string table offset comes first before the file checksum.
-  StringRef Data = CVFileChecksumTable.drop_front(FileOffset);
-  uint32_t StringOffset;
-  error(consume(Data, StringOffset));
+  auto Iter = CVFileChecksumTable.at(FileOffset);
 
-  // Check if the string table offset is valid.
-  if (StringOffset >= CVStringTable.size())
+  // Check if the file checksum table offset is valid.
+  if (Iter == CVFileChecksumTable.end())
     error(object_error::parse_failed);
 
-  // Return the null-terminated string.
-  return CVStringTable.drop_front(StringOffset).split('\0').first;
+  return error(CVStringTable.getString(Iter->FileNameOffset));
 }
 
 void COFFDumper::printFileNameForOffset(StringRef Label, uint32_t FileOffset) {
   W.printHex(Label, getFileNameForFileOffset(FileOffset), FileOffset);
 }
 
 void COFFDumper::mergeCodeViewTypes(TypeTableBuilder &CVIDs,
                                     TypeTableBuilder &CVTypes) {

llvm/trunk/tools/llvm-readobj/llvm-readobj.h

namespace llvm {
   namespace object {
     class RelocationRef;
   }
 
   // Various helper functions.
   LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg);
   void error(std::error_code EC);
   void error(llvm::Error EC);
+  template <typename T> T error(llvm::Expected<T> &&E) {
+    error(E.takeError());
+    return std::move(*E);
+  }
+
   template <class T> T unwrapOrError(ErrorOr<T> EO) {
     if (EO)
       return *EO;
     reportError(EO.getError().message());
   }
   template <class T> T unwrapOrError(Expected<T> EO) {
     if (EO)
       return *EO;

llvm/trunk/unittests/DebugInfo/PDB/ErrorChecking.h

#define EXPECT_EXPECTED(Exp)                                                   \
     auto E = Exp.takeError();                                                  \
     EXPECT_FALSE(static_cast<bool>(E));                                        \
     if (E) {                                                                   \
       consumeError(std::move(E));                                              \
       return;                                                                  \
     }                                                                          \
   }
 
+#define EXPECT_EXPECTED_EQ(Val, Exp)                                           \
+  {                                                                            \
+    auto Result = Exp;                                                         \
+    auto E = Result.takeError();                                               \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+      return;                                                                  \
+    }                                                                          \
+    EXPECT_EQ(Val, *Result);                                                   \
+  }
+
 #define EXPECT_UNEXPECTED(Exp)                                                 \
   {                                                                            \
     auto E = Exp.takeError();                                                  \
     EXPECT_TRUE(static_cast<bool>(E));                                         \
     if (E) {                                                                   \
       consumeError(std::move(E));                                              \
       return;                                                                  \
     }                                                                          \
   }
 
 #endif

llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp

 using namespace llvm;
 using namespace llvm::pdb;
 using namespace llvm::support;
 
 namespace {
 class StringTableBuilderTest : public ::testing::Test {};
 }
 
+template <typename T>
+static void ExpectExpected(Expected<T> &&E, const T &Value) {
+  EXPECT_EXPECTED(E);
+  EXPECT_EQ(Value, *E);
+}
+
 TEST_F(StringTableBuilderTest, Simple) {
   // Create /names table contents.
   PDBStringTableBuilder Builder;
   EXPECT_EQ(1U, Builder.insert("foo"));
   EXPECT_EQ(5U, Builder.insert("bar"));
   EXPECT_EQ(1U, Builder.insert("foo"));
   EXPECT_EQ(9U, Builder.insert("baz"));
 
   std::vector<uint8_t> Buffer(Builder.calculateSerializedSize());
   MutableBinaryByteStream OutStream(Buffer, little);
   BinaryStreamWriter Writer(OutStream);
   EXPECT_NO_ERROR(Builder.commit(Writer));
 
   // Reads the contents back.
   BinaryByteStream InStream(Buffer, little);
   BinaryStreamReader Reader(InStream);
   PDBStringTable Table;
   EXPECT_NO_ERROR(Table.reload(Reader));
 
   EXPECT_EQ(3U, Table.getNameCount());
   EXPECT_EQ(1U, Table.getHashVersion());
-  EXPECT_EQ("foo", Table.getStringForID(1));
-  EXPECT_EQ("bar", Table.getStringForID(5));
-  EXPECT_EQ("baz", Table.getStringForID(9));
-  EXPECT_EQ(1U, Table.getIDForString("foo"));
-  EXPECT_EQ(5U, Table.getIDForString("bar"));
-  EXPECT_EQ(9U, Table.getIDForString("baz"));
+
+  EXPECT_EXPECTED_EQ("foo", Table.getStringForID(1));
+  EXPECT_EXPECTED_EQ("bar", Table.getStringForID(5));
+  EXPECT_EXPECTED_EQ("baz", Table.getStringForID(9));
+  EXPECT_EXPECTED_EQ(1U, Table.getIDForString("foo"));
+  EXPECT_EXPECTED_EQ(5U, Table.getIDForString("bar"));
+  EXPECT_EXPECTED_EQ(9U, Table.getIDForString("baz"));
 }