[pdb] handle missing pdb streams more gracefully

[inglorion]

Summary

The code we use to read PDBs assumed that streams we ask it to read exist, and would read memory outside a vector and crash if this wasn't the case. This would, for example, cause llvm-pdbdump to crash on PDBs generated by lld. This patch handles such cases more gracefully: the PDB reading code in LLVM now reports errors when asked to get a stream that is not present, and llvm-pdbdump will report missing streams and continue processing streams that are present.

[ruiu]

include/llvm/DebugInfo/PDB/Raw/PDBFile.h

99–105 ↗

(On Diff #80000)

Why some are const and some are not?

lib/DebugInfo/PDB/Raw/PDBFile.cpp

233 ↗

(On Diff #80000)

Please run clang-format-diff.

[inglorion]

include/llvm/DebugInfo/PDB/Raw/PDBFile.h

99–105 ↗

(On Diff #80000)

The various get* methods above are not const, so when the has* method has to call a get* method, it can't be const. The common case is when a has* method needs to look at the DBI stream to determine the index of the stream whose existence is being tested.

[inglorion]

clang-format

[inglorion]

@ruiu, is this good to go now?

[ruiu]

LGTM

llvm/trunk/include/llvm/DebugInfo/PDB/Raw/PDBFile.h

 #include "llvm/Support/MathExtras.h"
 
 #include <memory>
 
 namespace llvm {
 
 namespace msf {
 class MappedBlockStream;
-class WritableStream;
 }
 
 namespace pdb {
 class DbiStream;
 class GlobalsStream;
 class InfoStream;
 class NameHashTable;
 class PDBFileBuilder;
public:
   Expected<TpiStream &> getPDBTpiStream();
   Expected<TpiStream &> getPDBIpiStream();
   Expected<PublicsStream &> getPDBPublicsStream();
   Expected<SymbolStream &> getPDBSymbolStream();
   Expected<NameHashTable &> getStringTable();
 
   BumpPtrAllocator &getAllocator() { return Allocator; }
 
+  bool hasPDBDbiStream() const;
+  bool hasPDBGlobalsStream();
+  bool hasPDBInfoStream();
+  bool hasPDBIpiStream() const;
+  bool hasPDBPublicsStream();
+  bool hasPDBSymbolStream();
+  bool hasPDBTpiStream() const;
+  bool hasStringTable();
+
  private:
+  Expected<std::unique_ptr<msf::MappedBlockStream>> safelyCreateIndexedStream(
+      const msf::MSFLayout &Layout, const msf::ReadableStream &MsfData,
+      uint32_t StreamIndex) const;
+
   BumpPtrAllocator &Allocator;
 
   std::unique_ptr<msf::ReadableStream> Buffer;
 
   std::vector<uint32_t> FpmPages;
   msf::MSFLayout ContainerLayout;
 
   std::unique_ptr<GlobalsStream> Globals;

llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp

MappedBlockStream::createStream(uint32_t BlockSize, uint32_t NumBlocks,
   return llvm::make_unique<MappedBlockStreamImpl<MappedBlockStream>>(
       BlockSize, NumBlocks, Layout, MsfData);
 }
 
 std::unique_ptr<MappedBlockStream>
 MappedBlockStream::createIndexedStream(const MSFLayout &Layout,
                                        const ReadableStream &MsfData,
                                        uint32_t StreamIndex) {
+  assert(StreamIndex < Layout.StreamMap.size() && "Invalid stream index");
   MSFStreamLayout SL;
   SL.Blocks = Layout.StreamMap[StreamIndex];
   SL.Length = Layout.StreamSizes[StreamIndex];
   return llvm::make_unique<MappedBlockStreamImpl<MappedBlockStream>>(
       Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
 }
 
 std::unique_ptr<MappedBlockStream>
WritableMappedBlockStream::createStream(uint32_t BlockSize, uint32_t NumBlocks,
   return llvm::make_unique<MappedBlockStreamImpl<WritableMappedBlockStream>>(
       BlockSize, NumBlocks, Layout, MsfData);
 }
 
 std::unique_ptr<WritableMappedBlockStream>
 WritableMappedBlockStream::createIndexedStream(const MSFLayout &Layout,
                                                const WritableStream &MsfData,
                                                uint32_t StreamIndex) {
+  assert(StreamIndex < Layout.StreamMap.size() && "Invalid stream index");
   MSFStreamLayout SL;
   SL.Blocks = Layout.StreamMap[StreamIndex];
   SL.Length = Layout.StreamSizes[StreamIndex];
   return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
 }
 
 std::unique_ptr<WritableMappedBlockStream>
 WritableMappedBlockStream::createDirectoryStream(

llvm/trunk/lib/DebugInfo/PDB/Raw/PDBFile.cpp

 }
 
 Expected<GlobalsStream &> PDBFile::getPDBGlobalsStream() {
   if (!Globals) {
     auto DbiS = getPDBDbiStream();
     if (!DbiS)
       return DbiS.takeError();
 
-    auto GlobalS = MappedBlockStream::createIndexedStream(
+    auto GlobalS = safelyCreateIndexedStream(
         ContainerLayout, *Buffer, DbiS->getGlobalSymbolStreamIndex());
-    auto TempGlobals = llvm::make_unique<GlobalsStream>(std::move(GlobalS));
+    if (!GlobalS) return GlobalS.takeError();
+    auto TempGlobals = llvm::make_unique<GlobalsStream>(std::move(*GlobalS));
     if (auto EC = TempGlobals->reload())
       return std::move(EC);
     Globals = std::move(TempGlobals);
   }
   return *Globals;
 }
 
 Expected<InfoStream &> PDBFile::getPDBInfoStream() {
   if (!Info) {
-    auto InfoS = MappedBlockStream::createIndexedStream(ContainerLayout,
-                                                        *Buffer, StreamPDB);
-    auto TempInfo = llvm::make_unique<InfoStream>(std::move(InfoS));
+    auto InfoS = safelyCreateIndexedStream(ContainerLayout, *Buffer, StreamPDB);
+    if (!InfoS) return InfoS.takeError();
+    auto TempInfo = llvm::make_unique<InfoStream>(std::move(*InfoS));
     if (auto EC = TempInfo->reload())
       return std::move(EC);
     Info = std::move(TempInfo);
   }
   return *Info;
 }
 
 Expected<DbiStream &> PDBFile::getPDBDbiStream() {
   if (!Dbi) {
-    auto DbiS = MappedBlockStream::createIndexedStream(ContainerLayout, *Buffer,
-                                                       StreamDBI);
-    auto TempDbi = llvm::make_unique<DbiStream>(*this, std::move(DbiS));
+    auto DbiS = safelyCreateIndexedStream(ContainerLayout, *Buffer, StreamDBI);
+    if (!DbiS) return DbiS.takeError();
+    auto TempDbi = llvm::make_unique<DbiStream>(*this, std::move(*DbiS));
     if (auto EC = TempDbi->reload())
       return std::move(EC);
     Dbi = std::move(TempDbi);
   }
   return *Dbi;
 }
 
 Expected<TpiStream &> PDBFile::getPDBTpiStream() {
   if (!Tpi) {
-    auto TpiS = MappedBlockStream::createIndexedStream(ContainerLayout, *Buffer,
-                                                       StreamTPI);
-    auto TempTpi = llvm::make_unique<TpiStream>(*this, std::move(TpiS));
+    auto TpiS = safelyCreateIndexedStream(ContainerLayout, *Buffer, StreamTPI);
+    if (!TpiS) return TpiS.takeError();
+    auto TempTpi = llvm::make_unique<TpiStream>(*this, std::move(*TpiS));
     if (auto EC = TempTpi->reload())
       return std::move(EC);
     Tpi = std::move(TempTpi);
   }
   return *Tpi;
 }
 
 Expected<TpiStream &> PDBFile::getPDBIpiStream() {
   if (!Ipi) {
-    auto IpiS = MappedBlockStream::createIndexedStream(ContainerLayout, *Buffer,
-                                                       StreamIPI);
-    auto TempIpi = llvm::make_unique<TpiStream>(*this, std::move(IpiS));
+    auto IpiS = safelyCreateIndexedStream(ContainerLayout, *Buffer, StreamIPI);
+    if (!IpiS) return IpiS.takeError();
+    auto TempIpi = llvm::make_unique<TpiStream>(*this, std::move(*IpiS));
     if (auto EC = TempIpi->reload())
       return std::move(EC);
     Ipi = std::move(TempIpi);
   }
   return *Ipi;
 }
 
 Expected<PublicsStream &> PDBFile::getPDBPublicsStream() {
   if (!Publics) {
     auto DbiS = getPDBDbiStream();
     if (!DbiS)
       return DbiS.takeError();
 
-    uint32_t PublicsStreamNum = DbiS->getPublicSymbolStreamIndex();
-
-    auto PublicS = MappedBlockStream::createIndexedStream(
-        ContainerLayout, *Buffer, PublicsStreamNum);
+    auto PublicS = safelyCreateIndexedStream(
+        ContainerLayout, *Buffer, DbiS->getPublicSymbolStreamIndex());
+    if (!PublicS) return PublicS.takeError();
     auto TempPublics =
-        llvm::make_unique<PublicsStream>(*this, std::move(PublicS));
+        llvm::make_unique<PublicsStream>(*this, std::move(*PublicS));
     if (auto EC = TempPublics->reload())
       return std::move(EC);
     Publics = std::move(TempPublics);
   }
   return *Publics;
 }
 
 Expected<SymbolStream &> PDBFile::getPDBSymbolStream() {
   if (!Symbols) {
     auto DbiS = getPDBDbiStream();
     if (!DbiS)
       return DbiS.takeError();
 
     uint32_t SymbolStreamNum = DbiS->getSymRecordStreamIndex();
-    auto SymbolS = MappedBlockStream::createIndexedStream(
-        ContainerLayout, *Buffer, SymbolStreamNum);
+    auto SymbolS =
+        safelyCreateIndexedStream(ContainerLayout, *Buffer, SymbolStreamNum);
+    if (!SymbolS) return SymbolS.takeError();
 
-    auto TempSymbols = llvm::make_unique<SymbolStream>(std::move(SymbolS));
+    auto TempSymbols = llvm::make_unique<SymbolStream>(std::move(*SymbolS));
     if (auto EC = TempSymbols->reload())
       return std::move(EC);
     Symbols = std::move(TempSymbols);
   }
   return *Symbols;
 }
 
 Expected<NameHashTable &> PDBFile::getStringTable() {
   if (!StringTable || !StringTableStream) {
     auto IS = getPDBInfoStream();
     if (!IS)
       return IS.takeError();
 
     uint32_t NameStreamIndex = IS->getNamedStreamIndex("/names");
 
-    if (NameStreamIndex == 0)
-      return make_error<RawError>(raw_error_code::no_stream);
-    if (NameStreamIndex >= getNumStreams())
-      return make_error<RawError>(raw_error_code::no_stream);
-    auto NS = MappedBlockStream::createIndexedStream(ContainerLayout, *Buffer,
-                                                     NameStreamIndex);
+    auto NS =
+        safelyCreateIndexedStream(ContainerLayout, *Buffer, NameStreamIndex);
+    if (!NS) return NS.takeError();
 
-    StreamReader Reader(*NS);
+    StreamReader Reader(**NS);
     auto N = llvm::make_unique<NameHashTable>();
     if (auto EC = N->load(Reader))
       return std::move(EC);
     StringTable = std::move(N);
-    StringTableStream = std::move(NS);
+    StringTableStream = std::move(*NS);
   }
   return *StringTable;
 }
+
+bool PDBFile::hasPDBDbiStream() const { return StreamDBI < getNumStreams(); }
+
+bool PDBFile::hasPDBGlobalsStream() {
+  auto DbiS = getPDBDbiStream();
+  if (!DbiS) return false;
+  return DbiS->getGlobalSymbolStreamIndex() < getNumStreams();
+}
+
+bool PDBFile::hasPDBInfoStream() { return StreamPDB < getNumStreams(); }
+
+bool PDBFile::hasPDBIpiStream() const { return StreamIPI < getNumStreams(); }
+
+bool PDBFile::hasPDBPublicsStream() {
+  auto DbiS = getPDBDbiStream();
+  if (!DbiS) return false;
+  return DbiS->getPublicSymbolStreamIndex() < getNumStreams();
+}
+
+bool PDBFile::hasPDBSymbolStream() {
+  auto DbiS = getPDBDbiStream();
+  if (!DbiS) return false;
+  return DbiS->getSymRecordStreamIndex() < getNumStreams();
+}
+
+bool PDBFile::hasPDBTpiStream() const { return StreamTPI < getNumStreams(); }
+
+bool PDBFile::hasStringTable() {
+  auto IS = getPDBInfoStream();
+  if (!IS) return false;
+  return IS->getNamedStreamIndex("/names") < getNumStreams();
+}
+
+/// Wrapper around MappedBlockStream::createIndexedStream()
+/// that checks if a stream with that index actually exists.
+/// If it does not, the return value will have an MSFError with
+/// code msf_error_code::no_stream. Else, the return value will
+/// contain the stream returned by createIndexedStream().
+Expected<std::unique_ptr<MappedBlockStream>> PDBFile::safelyCreateIndexedStream(
+    const MSFLayout &Layout, const ReadableStream &MsfData,
+    uint32_t StreamIndex) const {
+  if (StreamIndex >= getNumStreams())
+    return make_error<RawError>(raw_error_code::no_stream);
+  return MappedBlockStream::createIndexedStream(Layout, MsfData, StreamIndex);
+}

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

for (uint32_t I = 0, E = V.size(); I != E; ++I)
     if (V[I])
       Vec.push_back(I);
   P.printList(Name, Vec);
 }
 
 Error LLVMOutputStyle::dumpGlobalsStream() {
   if (!opts::raw::DumpGlobals)
     return Error::success();
+  if (!File.hasPDBGlobalsStream()) {
+    P.printString("Globals Stream not present");
+    return Error::success();
+  }
 
-  DictScope D(P, "Globals Stream");
   auto Globals = File.getPDBGlobalsStream();
   if (!Globals)
-    return Globals.takeError();
+    return handleErrors(Globals.takeError(),
+                        [&](const msf::MSFError &E) -> Error {
+                          if (E.Code == msf::msf_error_code::no_stream) {
+                            P.printString("Globals Stream not present");
+                            return Error::success();
+                          } else {
+                            return make_error<msf::MSFError>(E);
+                          }
+                        });
+  DictScope D(P, "Globals Stream");
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   P.printNumber("Stream number", Dbi->getGlobalSymbolStreamIndex());
   P.printNumber("Number of buckets", Globals->getNumBuckets());
   P.printList("Hash Buckets", Globals->getHashBuckets());
for (uint32_t SI : opts::raw::DumpStreamData) {
     P.printBinaryBlock("Data", StreamData);
   }
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpInfoStream() {
   if (!opts::raw::DumpHeaders)
     return Error::success();
+  if (!File.hasPDBInfoStream()) {
+    P.printString("PDB Stream not present");
+    return Error::success();
+  }
   auto IS = File.getPDBInfoStream();
   if (!IS)
     return IS.takeError();
 
   DictScope D(P, "PDB Stream");
   P.printNumber("Version", IS->getVersion());
   P.printHex("Signature", IS->getSignature());
   P.printNumber("Age", IS->getAge());

 Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
   assert(StreamIdx == StreamTPI || StreamIdx == StreamIPI);
 
   bool DumpRecordBytes = false;
   bool DumpRecords = false;
   StringRef Label;
   StringRef VerLabel;
   if (StreamIdx == StreamTPI) {
+    if (!File.hasPDBTpiStream()) {
+      P.printString("Type Info Stream (TPI) not present");
+      return Error::success();
+    }
     DumpRecordBytes = opts::raw::DumpTpiRecordBytes;
     DumpRecords = opts::raw::DumpTpiRecords;
     Label = "Type Info Stream (TPI)";
     VerLabel = "TPI Version";
   } else if (StreamIdx == StreamIPI) {
+    if (!File.hasPDBIpiStream()) {
+      P.printString("Type Info Stream (IPI) not present");
+      return Error::success();
+    }
     DumpRecordBytes = opts::raw::DumpIpiRecordBytes;
     DumpRecords = opts::raw::DumpIpiRecords;
     Label = "Type Info Stream (IPI)";
     VerLabel = "IPI Version";
   }
   if (!DumpRecordBytes && !DumpRecords && !opts::raw::DumpModuleSyms)
     return Error::success();
 
Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpDbiStream() {
   bool DumpModules = opts::raw::DumpModules || opts::raw::DumpModuleSyms ||
                      opts::raw::DumpModuleFiles || opts::raw::DumpLineInfo;
   if (!opts::raw::DumpHeaders && !DumpModules)
     return Error::success();
+  if (!File.hasPDBDbiStream()) {
+    P.printString("DBI Stream not present");
+    return Error::success();
+  }
 
   auto DS = File.getPDBDbiStream();
   if (!DS)
     return DS.takeError();
 
   DictScope D(P, "DBI Stream");
   P.printNumber("Dbi Version", DS->getDbiVersion());
   P.printNumber("Age", DS->getAge());
if (DumpModules) {
     }
   }
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpSectionContribs() {
   if (!opts::raw::DumpSectionContribs)
     return Error::success();
+  if (!File.hasPDBDbiStream()) {
+    P.printString("DBI Stream not present");
+    return Error::success();
+  }
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   ListScope L(P, "Section Contributions");
   class Visitor : public ISectionContribVisitor {
   public:
Error LLVMOutputStyle::dumpSectionContribs() {
   Visitor V(P, *Dbi);
   Dbi->visitSectionContributions(V);
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpSectionMap() {
   if (!opts::raw::DumpSectionMap)
     return Error::success();
+  if (!File.hasPDBDbiStream()) {
+    P.printString("DBI Stream not present");
+    return Error::success();
+  }
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   ListScope L(P, "Section Map");
   for (auto &M : Dbi->getSectionMap()) {
     DictScope D(P, "Entry");
     P.printFlags("Flags", M.Flags, getOMFSegMapDescFlagNames());
     P.printNumber("Ovl", M.Ovl);
     P.printNumber("Group", M.Group);
     P.printNumber("Frame", M.Frame);
     P.printNumber("SecName", M.SecName);
     P.printNumber("ClassName", M.ClassName);
     P.printNumber("Offset", M.Offset);
     P.printNumber("SecByteLength", M.SecByteLength);
     P.flush();
   }
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpPublicsStream() {
   if (!opts::raw::DumpPublics)
     return Error::success();
+  if (!File.hasPDBPublicsStream()) {
+    P.printString("Publics Stream not present");
+    return Error::success();
+  }
 
-  DictScope D(P, "Publics Stream");
   auto Publics = File.getPDBPublicsStream();
   if (!Publics)
     return Publics.takeError();
+  DictScope D(P, "Publics Stream");
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   P.printNumber("Stream number", Dbi->getPublicSymbolStreamIndex());
   P.printNumber("SymHash", Publics->getSymHash());
   P.printNumber("AddrMap", Publics->getAddrMap());
return make_error<RawError>(
         "Public symbol stream contained corrupt record");
 
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpSectionHeaders() {
   if (!opts::raw::DumpSectionHeaders)
     return Error::success();
+  if (!File.hasPDBDbiStream()) {
+    P.printString("DBI Stream not present");
+    return Error::success();
+  }
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   ListScope D(P, "Section Headers");
   for (const object::coff_section &Section : Dbi->getSectionHeaders()) {
     DictScope DD(P, "");
P.printFlags("Characteristics", Section.Characteristics,
                  getImageSectionCharacteristicNames());
   }
   return Error::success();
 }
 
 Error LLVMOutputStyle::dumpFpoStream() {
   if (!opts::raw::DumpFpo)
     return Error::success();
+  if (!File.hasPDBDbiStream()) {
+    P.printString("DBI Stream not present");
+    return Error::success();
+  }
 
   auto Dbi = File.getPDBDbiStream();
   if (!Dbi)
     return Dbi.takeError();
 
   ListScope D(P, "New FPO");
   for (const object::FpoData &Fpo : Dbi->getFpoRecords()) {
     DictScope DD(P, "");