[pdb] Fix errors with invalid stream numbers

[zturner]

Summary

This should fix most, if not all of the llvm-pdbdump-fuzzer errors. The problem is that we were not always checking that stream numbers were valid. This fixes that by making the constructor to MappedBlockStream private and only exposing it through a static creator which returns an Expected<unique_ptr<MappedBlockStream>>.

One problem that came up is that it's getting pretty ugly to write 4-5 lines of code to check an Expected<T> and return the error every time. I had the idea to wrap this up in some macros, but unfortunately it didn't go very well. You can see the results here. I could not find a way to do it without 6 different macros. If anyone can think of a way to make this less onerous, I'm all ears. Requirements:

1. It should work if you have a failed Expected<T> and on error you return a failed Expected<U>, where T != U.
2. It should work if you have a failed Expected<T> and you are returning an llvm::Error.
3. It should work if T is a reference type, such as Expected<DbiStream&>.
4. It should work if T is not copyable, such as Expected<std::unique_ptr<MappedBlockStream>>.
5. It should work if you want the "success" value to go into an existing variable (such as a class member variable)
6. It should work if you don't have a success variable declared and you want the macro to auto-declare it.

[ruiu]

I think I honestly don't like these macros. They indeed reduces amount of code but are hard to read. I prefer the current style (explicitly writing control flow in a plain C++) over the magical macros.

[zturner]

Is it because there are so many? Or for a different reason? There was one
case where there were like 15 lines of code jsut to get all the values out
of the functions and do the error checking, I found it really hurt the
ability to read the code.

I do agree that there are too many though. I'm not a C++ wizard, but if
someone can help me figure out how to write them with fewer macros, do you
think that would be better?

[zturner]

I actually I just thought of a better way. I will try it later when I get
in.

[zturner]

Remove macros, resort to manual error checking.

[ruiu]

LGTM

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

   struct HeaderInfo;
 
 public:
-  DbiStream(PDBFile &File);
+  DbiStream(PDBFile &File, std::unique_ptr<MappedBlockStream> Stream);
   ~DbiStream();
   Error reload();
 

   Error initializeFpoRecords();
 
   PDBFile &Pdb;
-  MappedBlockStream Stream;
+  std::unique_ptr<MappedBlockStream> Stream;
 
   std::vector<ModuleInfoEx> ModuleInfos;
   NameHashTable ECNames;

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

+//===- DirectoryStreamData.h ---------------------------------- *- C++ --*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFO_PDB_RAW_DIRECTORYSTREAMDATA_H
+#define LLVM_DEBUGINFO_PDB_RAW_DIRECTORYSTREAMDATA_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/DebugInfo/PDB/Raw/IPDBStreamData.h"
+#include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
+#include "llvm/Support/Endian.h"
+
+namespace llvm {
+namespace pdb {
+class IPDBFile;
+
+class DirectoryStreamData : public IPDBStreamData {
+public:
+  DirectoryStreamData(const PDBFile &File) : File(File) {}
+
+  virtual uint32_t getLength() { return File.getNumDirectoryBytes(); }
+  virtual llvm::ArrayRef<llvm::support::ulittle32_t> getStreamBlocks() {
+    return File.getDirectoryBlockArray();
+  }
+
+private:
+  const PDBFile &File;
+};
+}
+}
+
+#endif

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

 class PDBFile;
 class InfoStream {
 public:
-  InfoStream(const PDBFile &File);
+  InfoStream(std::unique_ptr<MappedBlockStream> Stream);
 
   Error reload();
 

   iterator_range<StringMapConstIterator<uint32_t>> named_streams() const;
 
 private:
-  MappedBlockStream Stream;
+  std::unique_ptr<MappedBlockStream> Stream;
 
   // PDB file format version.  We only support VC70.  See the enumeration
   // `PdbRaw_ImplVer` for the other possible values.

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

 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/CodeView/StreamInterface.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Endian.h"

 
 class IPDBFile;
 class IPDBStreamData;
+class PDBFile;
 
 class MappedBlockStream : public codeview::StreamInterface {
 public:
-  MappedBlockStream(std::unique_ptr<IPDBStreamData> Data, const IPDBFile &File);
-
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const override;
 

 
   uint32_t getNumBytesCopied() const;
 
-private:
+  static Expected<std::unique_ptr<MappedBlockStream>>
+  createIndexedStream(uint32_t StreamIdx, const IPDBFile &File);
+  static Expected<std::unique_ptr<MappedBlockStream>>
+  createDirectoryStream(const PDBFile &File);
+
+protected:
+  MappedBlockStream(std::unique_ptr<IPDBStreamData> Data, const IPDBFile &File);
+
   Error readBytes(uint32_t Offset, MutableArrayRef<uint8_t> Buffer) const;
   bool tryReadContiguously(uint32_t Offset, uint32_t Size,
                            ArrayRef<uint8_t> &Buffer) const;

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

 
 class ModStream {
 public:
-  ModStream(const PDBFile &File, const ModInfo &Module);
+  ModStream(const ModInfo &Module, std::unique_ptr<MappedBlockStream> Stream);
   ~ModStream();
 
   Error reload();

 private:
   const ModInfo &Mod;
 
-  MappedBlockStream Stream;
+  std::unique_ptr<MappedBlockStream> Stream;
 
   codeview::CVSymbolArray SymbolsSubstream;
   codeview::StreamRef LinesSubstream;

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

   struct HeaderInfo;
 
 public:
-  PublicsStream(PDBFile &File, uint32_t StreamNum);
+  PublicsStream(PDBFile &File, std::unique_ptr<MappedBlockStream> Stream);
   ~PublicsStream();
   Error reload();
 
-  uint32_t getStreamNum() const { return StreamNum; }
   uint32_t getSymHash() const;
   uint32_t getAddrMap() const;
   uint32_t getNumBuckets() const { return NumBuckets; }

 private:
   PDBFile &Pdb;
 
-  uint32_t StreamNum;
-  MappedBlockStream Stream;
+  std::unique_ptr<MappedBlockStream> Stream;
   uint32_t NumBuckets = 0;
   ArrayRef<uint8_t> Bitmap;
   codeview::FixedStreamArray<PSHashRecord> HashRecords;

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

 
 class SymbolStream {
 public:
-  SymbolStream(const PDBFile &File, uint32_t StreamNum);
+  SymbolStream(std::unique_ptr<MappedBlockStream> Stream);
   ~SymbolStream();
   Error reload();
 

 
 private:
   codeview::CVSymbolArray SymbolRecords;
-  MappedBlockStream MappedStream;
+  std::unique_ptr<MappedBlockStream> Stream;
 };
 }
 }

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

   struct HeaderInfo;
 
 public:
-  TpiStream(const PDBFile &File, uint32_t StreamIdx);
+  TpiStream(const PDBFile &File, std::unique_ptr<MappedBlockStream> Stream);
   ~TpiStream();
   Error reload();
 

 
 private:
   const PDBFile &Pdb;
-  MappedBlockStream Stream;
+  std::unique_ptr<MappedBlockStream> Stream;
   HashFunctionType HashFunction;
 
   codeview::CVTypeArray TypeRecords;

include/llvm/Support/Error.h

 
 public:
   typedef typename std::conditional<isRef, wrap, T>::type storage_type;
+  typedef T value_type;
 
 private:
   typedef typename std::remove_reference<T>::type &reference;

lib/DebugInfo/PDB/Raw/DbiStream.cpp

   return Error::success();
 }
 
-DbiStream::DbiStream(PDBFile &File)
-    : Pdb(File),
-      Stream(llvm::make_unique<IndexedStreamData>(StreamDBI, File), File),
-      Header(nullptr) {
+DbiStream::DbiStream(PDBFile &File, std::unique_ptr<MappedBlockStream> Stream)
+    : Pdb(File), Stream(std::move(Stream)), Header(nullptr) {
   static_assert(sizeof(HeaderInfo) == 64, "Invalid HeaderInfo size!");
 }
 
 DbiStream::~DbiStream() {}
 
 Error DbiStream::reload() {
-  StreamReader Reader(Stream);
+  StreamReader Reader(*Stream);
 
-  if (Stream.getLength() < sizeof(HeaderInfo))
+  if (Stream->getLength() < sizeof(HeaderInfo))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "DBI Stream does not contain a header.");
   if (auto EC = Reader.readObject(Header))

     return make_error<RawError>(raw_error_code::feature_unsupported,
                                 "Unsupported DBI version.");
 
-  auto InfoStream = Pdb.getPDBInfoStream();
-  if (auto EC = InfoStream.takeError())
-    return EC;
+  auto IS = Pdb.getPDBInfoStream();
+  if (!IS)
+    return IS.takeError();
 
-  if (Header->Age != InfoStream.get().getAge())
+  if (Header->Age != IS->getAge())
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "DBI Age does not match PDB Age.");
 
-  if (Stream.getLength() !=
+  if (Stream->getLength() !=
       sizeof(HeaderInfo) + Header->ModiSubstreamSize +
           Header->SecContrSubstreamSize + Header->SectionMapSize +
           Header->FileInfoSize + Header->TypeServerSize +

   if (StreamNum >= Pdb.getNumStreams())
     return make_error<RawError>(raw_error_code::no_stream);
 
-  SectionHeaderStream.reset(new MappedBlockStream(
-      llvm::make_unique<IndexedStreamData>(StreamNum, Pdb), Pdb));
+  auto SHS = MappedBlockStream::createIndexedStream(StreamNum, Pdb);
+  if (!SHS)
+    return SHS.takeError();
 
-  size_t StreamLen = SectionHeaderStream->getLength();
+  size_t StreamLen = (*SHS)->getLength();
   if (StreamLen % sizeof(object::coff_section))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Corrupted section header stream.");
 
   size_t NumSections = StreamLen / sizeof(object::coff_section);
-  codeview::StreamReader Reader(*SectionHeaderStream);
+  codeview::StreamReader Reader(**SHS);
   if (auto EC = Reader.readArray(SectionHeaders, NumSections))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Could not read a bitmap.");
+
+  SectionHeaderStream = std::move(*SHS);
   return Error::success();
 }
 

   if (StreamNum >= Pdb.getNumStreams())
     return make_error<RawError>(raw_error_code::no_stream);
 
-  FpoStream.reset(new MappedBlockStream(
-      llvm::make_unique<IndexedStreamData>(StreamNum, Pdb), Pdb));
+  auto FS = MappedBlockStream::createIndexedStream(StreamNum, Pdb);
+  if (!FS)
+    return FS.takeError();
 
-  size_t StreamLen = FpoStream->getLength();
+  size_t StreamLen = (*FS)->getLength();
   if (StreamLen % sizeof(object::FpoData))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Corrupted New FPO stream.");
 
   size_t NumRecords = StreamLen / sizeof(object::FpoData);
-  codeview::StreamReader Reader(*FpoStream);
+  codeview::StreamReader Reader(**FS);
   if (auto EC = Reader.readArray(FpoRecords, NumRecords))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Corrupted New FPO stream.");
+  FpoStream = std::move(*FS);
   return Error::success();
 }
 

     uint32_t NumFiles = ModFileCountArray[I];
     ModuleInfos[I].SourceFiles.resize(NumFiles);
     for (size_t J = 0; J < NumFiles; ++J, ++NextFileIndex) {
-      if (auto Name = getFileNameForIndex(NextFileIndex))
-        ModuleInfos[I].SourceFiles[J] = Name.get();
-      else
-        return Name.takeError();
+      auto ThisName = getFileNameForIndex(NextFileIndex);
+      if (!ThisName)
+        return ThisName.takeError();
+      ModuleInfos[I].SourceFiles[J] = *ThisName;
     }
   }
 

lib/DebugInfo/PDB/Raw/InfoStream.cpp

 using namespace llvm;
 using namespace llvm::pdb;
 
-InfoStream::InfoStream(const PDBFile &File)
-    : Stream(llvm::make_unique<IndexedStreamData>(StreamPDB, File), File) {}
+InfoStream::InfoStream(std::unique_ptr<MappedBlockStream> Stream)
+    : Stream(std::move(Stream)) {}
 
 Error InfoStream::reload() {
-  codeview::StreamReader Reader(Stream);
+  codeview::StreamReader Reader(*Stream);
 
   struct Header {
     support::ulittle32_t Version;

lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Raw/DirectoryStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/IPDBStreamData.h"
+#include "llvm/DebugInfo/PDB/Raw/IndexedStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
 using namespace llvm;
 using namespace llvm::pdb;
 
+namespace {
+// This exists so that we can use make_unique while still keeping the
+// constructor of MappedBlockStream private, forcing users to go through
+// the `create` interface.
+class MappedBlockStreamImpl : public MappedBlockStream {
+public:
+  MappedBlockStreamImpl(std::unique_ptr<IPDBStreamData> Data,
+                        const IPDBFile &File)
+      : MappedBlockStream(std::move(Data), File) {}
+};
+}
+
 MappedBlockStream::MappedBlockStream(std::unique_ptr<IPDBStreamData> Data,
                                      const IPDBFile &Pdb)
     : Pdb(Pdb), Data(std::move(Data)) {}

 uint32_t MappedBlockStream::getNumBytesCopied() const {
   return static_cast<uint32_t>(Pool.getBytesAllocated());
 }
+
+Expected<std::unique_ptr<MappedBlockStream>>
+MappedBlockStream::createIndexedStream(uint32_t StreamIdx,
+                                       const IPDBFile &File) {
+  if (StreamIdx >= File.getNumStreams())
+    return make_error<RawError>(raw_error_code::no_stream);
+
+  auto Data = llvm::make_unique<IndexedStreamData>(StreamIdx, File);
+  return llvm::make_unique<MappedBlockStreamImpl>(std::move(Data), File);
+}
+
+Expected<std::unique_ptr<MappedBlockStream>>
+MappedBlockStream::createDirectoryStream(const PDBFile &File) {
+  auto Data = llvm::make_unique<DirectoryStreamData>(File);
+  return llvm::make_unique<MappedBlockStreamImpl>(std::move(Data), File);
+}

lib/DebugInfo/PDB/Raw/ModStream.cpp

 using namespace llvm;
 using namespace llvm::pdb;
 
-ModStream::ModStream(const PDBFile &File, const ModInfo &Module)
-    : Mod(Module), Stream(llvm::make_unique<IndexedStreamData>(
-                              Module.getModuleStreamIndex(), File),
-                          File) {}
+ModStream::ModStream(const ModInfo &Module,
+                     std::unique_ptr<MappedBlockStream> Stream)
+    : Mod(Module), Stream(std::move(Stream)) {}
 
 ModStream::~ModStream() {}
 
 Error ModStream::reload() {
-  codeview::StreamReader Reader(Stream);
+  codeview::StreamReader Reader(*Stream);
 
   uint32_t SymbolSize = Mod.getSymbolDebugInfoByteSize();
   uint32_t C11Size = Mod.getLineInfoByteSize();

lib/DebugInfo/PDB/Raw/PDBFile.cpp

 #include "llvm/DebugInfo/CodeView/StreamArray.h"
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/PDB/Raw/DbiStream.h"
+#include "llvm/DebugInfo/PDB/Raw/DirectoryStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/IndexedStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/InfoStream.h"
 #include "llvm/DebugInfo/PDB/Raw/NameHashTable.h"

   support::ulittle32_t BlockMapAddr;
 };
 
-class DirectoryStreamData : public IPDBStreamData {
-public:
-  DirectoryStreamData(const PDBFile &File) : File(File) {}
-
-  virtual uint32_t getLength() { return File.getNumDirectoryBytes(); }
-  virtual llvm::ArrayRef<llvm::support::ulittle32_t> getStreamBlocks() {
-    return File.getDirectoryBlockArray();
-  }
-
-private:
-  const PDBFile &File;
-};
-
 typedef codeview::FixedStreamArray<support::ulittle32_t> ulittle_array;
 }
 

   // is exactly what we are attempting to parse.  By specifying a custom
   // subclass of IPDBStreamData which only accesses the fields that have already
   // been parsed, we can avoid this and reuse MappedBlockStream.
-  auto SD = llvm::make_unique<DirectoryStreamData>(*this);
-  DirectoryStream = llvm::make_unique<MappedBlockStream>(std::move(SD), *this);
-  codeview::StreamReader Reader(*DirectoryStream);
+  auto DS = MappedBlockStream::createDirectoryStream(*this);
+  if (!DS)
+    return DS.takeError();
+  codeview::StreamReader Reader(**DS);
   if (auto EC = Reader.readInteger(NumStreams))
     return EC;
 

 
   // We should have read exactly SB->NumDirectoryBytes bytes.
   assert(Reader.bytesRemaining() == 0);
+  DirectoryStream = std::move(*DS);
   return Error::success();
 }
 

 
 Expected<InfoStream &> PDBFile::getPDBInfoStream() {
   if (!Info) {
-    Info.reset(new InfoStream(*this));
-    if (auto EC = Info->reload())
+    auto InfoS = MappedBlockStream::createIndexedStream(StreamPDB, *this);
+    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) {
-    Dbi.reset(new DbiStream(*this));
-    if (auto EC = Dbi->reload())
+    auto DbiS = MappedBlockStream::createIndexedStream(StreamDBI, *this);
+    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) {
-    Tpi.reset(new TpiStream(*this, StreamTPI));
-    if (auto EC = Tpi->reload())
+    auto TpiS = MappedBlockStream::createIndexedStream(StreamTPI, *this);
+    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) {
-    Ipi.reset(new TpiStream(*this, StreamIPI));
-    if (auto EC = Ipi->reload())
+    auto IpiS = MappedBlockStream::createIndexedStream(StreamIPI, *this);
+    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 (auto EC = DbiS.takeError())
-      return std::move(EC);
+    if (!DbiS)
+      return DbiS.takeError();
+
     uint32_t PublicsStreamNum = DbiS->getPublicSymbolStreamIndex();
 
-    Publics.reset(new PublicsStream(*this, PublicsStreamNum));
-    if (auto EC = Publics->reload())
+    auto PublicS =
+        MappedBlockStream::createIndexedStream(PublicsStreamNum, *this);
+    if (!PublicS)
+      return PublicS.takeError();
+    auto TempPublics =
+        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 (auto EC = DbiS.takeError())
-      return std::move(EC);
+    if (!DbiS)
+      return DbiS.takeError();
+
     uint32_t SymbolStreamNum = DbiS->getSymRecordStreamIndex();
 
-    Symbols.reset(new SymbolStream(*this, SymbolStreamNum));
-    if (auto EC = Symbols->reload())
+    auto SymbolS =
+        MappedBlockStream::createIndexedStream(SymbolStreamNum, *this);
+    if (!SymbolS)
+      return SymbolS.takeError();
+    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 InfoS = getPDBInfoStream();
-    if (auto EC = InfoS.takeError())
-      return std::move(EC);
-    auto &IS = InfoS.get();
-    uint32_t NameStreamIndex = IS.getNamedStreamIndex("/names");
+    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 SD = llvm::make_unique<IndexedStreamData>(NameStreamIndex, *this);
-    auto S = llvm::make_unique<MappedBlockStream>(std::move(SD), *this);
-    codeview::StreamReader Reader(*S);
+    auto NS = MappedBlockStream::createIndexedStream(NameStreamIndex, *this);
+    if (!NS)
+      return NS.takeError();
+
+    codeview::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(S);
+    StringTableStream = std::move(*NS);
   }
   return *StringTable;
 }

lib/DebugInfo/PDB/Raw/PublicsStream.cpp

   ulittle32_t NumBuckets;
 };
 
-PublicsStream::PublicsStream(PDBFile &File, uint32_t StreamNum)
-    : Pdb(File), StreamNum(StreamNum),
-      Stream(llvm::make_unique<IndexedStreamData>(StreamNum, File), File) {}
+PublicsStream::PublicsStream(PDBFile &File,
+                             std::unique_ptr<MappedBlockStream> Stream)
+    : Pdb(File), Stream(std::move(Stream)) {}
 
 PublicsStream::~PublicsStream() {}
 

 // we skip over the hash table which we believe contains information about
 // public symbols.
 Error PublicsStream::reload() {
-  codeview::StreamReader Reader(Stream);
+  codeview::StreamReader Reader(*Stream);
 
   // Check stream size.
   if (Reader.bytesRemaining() < sizeof(HeaderInfo) + sizeof(GSIHashHeader))

lib/DebugInfo/PDB/Raw/SymbolStream.cpp

 using namespace llvm::support;
 using namespace llvm::pdb;
 
-SymbolStream::SymbolStream(const PDBFile &File, uint32_t StreamNum)
-    : MappedStream(llvm::make_unique<IndexedStreamData>(StreamNum, File),
-                   File) {}
+SymbolStream::SymbolStream(std::unique_ptr<MappedBlockStream> Stream)
+    : Stream(std::move(Stream)) {}
 
 SymbolStream::~SymbolStream() {}
 
 Error SymbolStream::reload() {
-  codeview::StreamReader Reader(MappedStream);
+  codeview::StreamReader Reader(*Stream);
 
-  if (auto EC = Reader.readArray(SymbolRecords, MappedStream.getLength()))
+  if (auto EC = Reader.readArray(SymbolRecords, Stream->getLength()))
     return EC;
 
   return Error::success();

lib/DebugInfo/PDB/Raw/TpiStream.cpp

   EmbeddedBuf HashAdjBuffer;
 };
 
-TpiStream::TpiStream(const PDBFile &File, uint32_t StreamIdx)
-    : Pdb(File),
-      Stream(llvm::make_unique<IndexedStreamData>(StreamIdx, File), File),
-      HashFunction(nullptr) {}
+TpiStream::TpiStream(const PDBFile &File,
+                     std::unique_ptr<MappedBlockStream> Stream)
+    : Pdb(File), Stream(std::move(Stream)), HashFunction(nullptr) {}
 
 TpiStream::~TpiStream() {}
 
 Error TpiStream::reload() {
-  codeview::StreamReader Reader(Stream);
+  codeview::StreamReader Reader(*Stream);
 
   if (Reader.bytesRemaining() < sizeof(HeaderInfo))
     return make_error<RawError>(raw_error_code::corrupt_file,

     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Invalid TPI hash stream index.");
 
-  HashStream.reset(new MappedBlockStream(
-      llvm::make_unique<IndexedStreamData>(Header->HashStreamIndex, Pdb), Pdb));
-  codeview::StreamReader HSR(*HashStream);
+  auto HS =
+      MappedBlockStream::createIndexedStream(Header->HashStreamIndex, Pdb);
+  if (!HS)
+    return HS.takeError();
+  codeview::StreamReader HSR(**HS);
 
   uint32_t NumHashValues = Header->HashValueBuffer.Length / sizeof(ulittle32_t);
   if (NumHashValues != NumTypeRecords())

   if (auto EC = HSR.readArray(HashAdjustments, NumHashAdjustments))
     return EC;
 
+  HashStream = std::move(*HS);
   return Error::success();
 }
 

tools/llvm-pdbdump/LLVMOutputStyle.cpp

   if (!opts::DumpStreamSummary)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  auto TpiS = File.getPDBTpiStream();
-  if (auto EC = TpiS.takeError())
-    return EC;
-  auto IpiS = File.getPDBIpiStream();
-  if (auto EC = IpiS.takeError())
-    return EC;
-  auto InfoS = File.getPDBInfoStream();
-  if (auto EC = InfoS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
-  TpiStream &TS = TpiS.get();
-  TpiStream &TIS = IpiS.get();
-  InfoStream &IS = InfoS.get();
+  auto Dbi = File.getPDBDbiStream();
+  if (!Dbi)
+    return Dbi.takeError();
+
+  auto Tpi = File.getPDBTpiStream();
+  if (!Tpi)
+    return Tpi.takeError();
+
+  auto Ipi = File.getPDBIpiStream();
+  if (!Ipi)
+    return Ipi.takeError();
+
+  auto Info = File.getPDBInfoStream();
+  if (!Info)
+    return Info.takeError();
 
   ListScope L(P, "Streams");
   uint32_t StreamCount = File.getNumStreams();
   std::unordered_map<uint16_t, const ModuleInfoEx *> ModStreams;
   std::unordered_map<uint16_t, std::string> NamedStreams;
 
-  for (auto &ModI : DS.modules()) {
+  for (auto &ModI : Dbi->modules()) {
     uint16_t SN = ModI.Info.getModuleStreamIndex();
     ModStreams[SN] = &ModI;
   }
-  for (auto &NSE : IS.named_streams()) {
+  for (auto &NSE : Info->named_streams()) {
     NamedStreams[NSE.second] = NSE.first();
   }
 

       Value = "TPI Stream";
     else if (StreamIdx == StreamIPI)
       Value = "IPI Stream";
-    else if (StreamIdx == DS.getGlobalSymbolStreamIndex())
+    else if (StreamIdx == Dbi->getGlobalSymbolStreamIndex())
       Value = "Global Symbol Hash";
-    else if (StreamIdx == DS.getPublicSymbolStreamIndex())
+    else if (StreamIdx == Dbi->getPublicSymbolStreamIndex())
       Value = "Public Symbol Hash";
-    else if (StreamIdx == DS.getSymRecordStreamIndex())
+    else if (StreamIdx == Dbi->getSymRecordStreamIndex())
       Value = "Public Symbol Records";
-    else if (StreamIdx == TS.getTypeHashStreamIndex())
+    else if (StreamIdx == Tpi->getTypeHashStreamIndex())
       Value = "TPI Hash";
-    else if (StreamIdx == TS.getTypeHashStreamAuxIndex())
+    else if (StreamIdx == Tpi->getTypeHashStreamAuxIndex())
       Value = "TPI Aux Hash";
-    else if (StreamIdx == TIS.getTypeHashStreamIndex())
+    else if (StreamIdx == Ipi->getTypeHashStreamIndex())
       Value = "IPI Hash";
-    else if (StreamIdx == TIS.getTypeHashStreamAuxIndex())
+    else if (StreamIdx == Ipi->getTypeHashStreamAuxIndex())
       Value = "IPI Aux Hash";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::Exception))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::Exception))
       Value = "Exception Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::Fixup))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::Fixup))
       Value = "Fixup Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::FPO))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::FPO))
       Value = "FPO Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::NewFPO))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::NewFPO))
       Value = "New FPO Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::OmapFromSrc))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::OmapFromSrc))
       Value = "Omap From Source Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::OmapToSrc))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::OmapToSrc))
       Value = "Omap To Source Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::Pdata))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::Pdata))
       Value = "Pdata";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::SectionHdr))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::SectionHdr))
       Value = "Section Header Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::SectionHdrOrig))
+    else if (StreamIdx ==
+             Dbi->getDebugStreamIndex(DbgHeaderType::SectionHdrOrig))
       Value = "Section Header Original Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::TokenRidMap))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::TokenRidMap))
       Value = "Token Rid Data";
-    else if (StreamIdx == DS.getDebugStreamIndex(DbgHeaderType::Xdata))
+    else if (StreamIdx == Dbi->getDebugStreamIndex(DbgHeaderType::Xdata))
       Value = "Xdata";
     else {
       auto ModIter = ModStreams.find(StreamIdx);

   if (DumpStreamNum >= StreamCount)
     return make_error<RawError>(raw_error_code::no_stream);
 
-  MappedBlockStream S(llvm::make_unique<IndexedStreamData>(DumpStreamNum, File),
-                      File);
-  codeview::StreamReader R(S);
+  auto S = MappedBlockStream::createIndexedStream(DumpStreamNum, File);
+  if (!S)
+    return S.takeError();
+  codeview::StreamReader R(**S);
   while (R.bytesRemaining() > 0) {
     ArrayRef<uint8_t> Data;
     uint32_t BytesToReadInBlock = std::min(

 Error LLVMOutputStyle::dumpInfoStream() {
   if (!opts::DumpHeaders)
     return Error::success();
-  auto InfoS = File.getPDBInfoStream();
-  if (auto EC = InfoS.takeError())
-    return EC;
-
-  InfoStream &IS = InfoS.get();
+  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());
-  P.printObject("Guid", IS.getGuid());
+  P.printNumber("Version", IS->getVersion());
+  P.printHex("Signature", IS->getSignature());
+  P.printNumber("Age", IS->getAge());
+  P.printObject("Guid", IS->getGuid());
   return Error::success();
 }
 

   if (opts::DumpStreamDataName.empty())
     return Error::success();
 
-  auto InfoS = File.getPDBInfoStream();
-  if (auto EC = InfoS.takeError())
-    return EC;
-  InfoStream &IS = InfoS.get();
+  auto IS = File.getPDBInfoStream();
+  if (!IS)
+    return IS.takeError();
 
-  uint32_t NameStreamIndex = IS.getNamedStreamIndex(opts::DumpStreamDataName);
+  uint32_t NameStreamIndex = IS->getNamedStreamIndex(opts::DumpStreamDataName);
   if (NameStreamIndex == 0 || NameStreamIndex >= File.getNumStreams())
     return make_error<RawError>(raw_error_code::no_stream);
 

     DictScope D(P, Name);
     P.printNumber("Index", NameStreamIndex);
 
-    MappedBlockStream NameStream(
-        llvm::make_unique<IndexedStreamData>(NameStreamIndex, File), File);
-    codeview::StreamReader Reader(NameStream);
+    auto NameStream =
+        MappedBlockStream::createIndexedStream(NameStreamIndex, File);
+    if (!NameStream)
+      return NameStream.takeError();
+    codeview::StreamReader Reader(**NameStream);
 
     NameHashTable NameTable;
     if (auto EC = NameTable.load(Reader))

   if (!DumpRecordBytes && !DumpRecords && !opts::DumpModuleSyms)
     return Error::success();
 
-  auto TpiS = (StreamIdx == StreamTPI) ? File.getPDBTpiStream()
-                                       : File.getPDBIpiStream();
-  if (auto EC = TpiS.takeError())
-    return EC;
-  TpiStream &Tpi = TpiS.get();
+  auto Tpi = (StreamIdx == StreamTPI) ? File.getPDBTpiStream()
+  if (!Tpi)
+    return Tpi.takeError();
 
   if (DumpRecords || DumpRecordBytes) {
     DictScope D(P, Label);
 
-    P.printNumber(VerLabel, Tpi.getTpiVersion());
-    P.printNumber("Record count", Tpi.NumTypeRecords());
+    P.printNumber(VerLabel, Tpi->getTpiVersion());
+    P.printNumber("Record count", Tpi->NumTypeRecords());
 
     ListScope L(P, "Records");
 
     bool HadError = false;
-    for (auto &Type : Tpi.types(&HadError)) {
+    for (auto &Type : Tpi->types(&HadError)) {
       DictScope DD(P, "");
 
       if (DumpRecords)

       if (DumpRecordBytes)
         P.printBinaryBlock("Bytes", Type.Data);
     }
-    dumpTpiHash(P, Tpi);
+    dumpTpiHash(P, *Tpi);
     if (HadError)
       return make_error<RawError>(raw_error_code::corrupt_file,
                                   "TPI stream contained corrupt record");

     TD.setPrinter(nullptr);
 
     bool HadError = false;
-    for (auto &Type : Tpi.types(&HadError))
+    for (auto &Type : Tpi->types(&HadError))
       TD.dump(Type);
 
     TD.setPrinter(OldP);
-    dumpTpiHash(P, Tpi);
+    dumpTpiHash(P, *Tpi);
     if (HadError)
       return make_error<RawError>(raw_error_code::corrupt_file,
                                   "TPI stream contained corrupt record");

   if (!opts::DumpHeaders && !DumpModules)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
+  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());
-  P.printBoolean("Incremental Linking", DS.isIncrementallyLinked());
-  P.printBoolean("Has CTypes", DS.hasCTypes());
-  P.printBoolean("Is Stripped", DS.isStripped());
-  P.printObject("Machine Type", DS.getMachineType());
-  P.printNumber("Symbol Record Stream Index", DS.getSymRecordStreamIndex());
-  P.printNumber("Public Symbol Stream Index", DS.getPublicSymbolStreamIndex());
-  P.printNumber("Global Symbol Stream Index", DS.getGlobalSymbolStreamIndex());
-
-  uint16_t Major = DS.getBuildMajorVersion();
-  uint16_t Minor = DS.getBuildMinorVersion();
+  P.printNumber("Dbi Version", DS->getDbiVersion());
+  P.printNumber("Age", DS->getAge());
+  P.printBoolean("Incremental Linking", DS->isIncrementallyLinked());
+  P.printBoolean("Has CTypes", DS->hasCTypes());
+  P.printBoolean("Is Stripped", DS->isStripped());
+  P.printObject("Machine Type", DS->getMachineType());
+  P.printNumber("Symbol Record Stream Index", DS->getSymRecordStreamIndex());
+  P.printNumber("Public Symbol Stream Index", DS->getPublicSymbolStreamIndex());
+  P.printNumber("Global Symbol Stream Index", DS->getGlobalSymbolStreamIndex());
+
+  uint16_t Major = DS->getBuildMajorVersion();
+  uint16_t Minor = DS->getBuildMinorVersion();
   P.printVersion("Toolchain Version", Major, Minor);
 
   std::string DllName;
   raw_string_ostream DllStream(DllName);
   DllStream << "mspdb" << Major << Minor << ".dll version";
   DllStream.flush();
-  P.printVersion(DllName, Major, Minor, DS.getPdbDllVersion());
+  P.printVersion(DllName, Major, Minor, DS->getPdbDllVersion());
 
   if (DumpModules) {
     ListScope L(P, "Modules");
-    for (auto &Modi : DS.modules()) {
+    for (auto &Modi : DS->modules()) {
       DictScope DD(P);
       P.printString("Name", Modi.Info.getModuleName().str());
       P.printNumber("Debug Stream Index", Modi.Info.getModuleStreamIndex());

       bool ShouldDumpSymbols =
           (opts::DumpModuleSyms || opts::DumpSymRecordBytes);
       if (HasModuleDI && (ShouldDumpSymbols || opts::DumpLineInfo)) {
-        ModStream ModS(File, Modi.Info);
+        auto ModStreamData = MappedBlockStream::createIndexedStream(
+            Modi.Info.getModuleStreamIndex(), File);
+        if (!ModStreamData)
+          return ModStreamData.takeError();
+        ModStream ModS(Modi.Info, std::move(*ModStreamData));
         if (auto EC = ModS.reload())
           return EC;
 

 
           private:
             Expected<StringRef> getFileNameForOffset(uint32_t Offset) {
-              auto StringT = F.getStringTable();
-              if (auto EC = StringT.takeError())
-                return std::move(EC);
-              NameHashTable &ST = StringT.get();
-              return ST.getStringForID(Offset);
+              auto ST = F.getStringTable();
+              if (!ST)
+                return ST.takeError();
+
+              return ST->getStringForID(Offset);
             }
             Expected<StringRef> getFileNameForOffset2(uint32_t Offset) {
-              auto DbiS = F.getPDBDbiStream();
-              if (auto EC = DbiS.takeError())
-                return std::move(EC);
-              auto &DS = DbiS.get();
-              return DS.getFileNameForIndex(Offset);
+              auto DS = F.getPDBDbiStream();
+              if (!DS)
+                return DS.takeError();
+              return DS->getFileNameForIndex(Offset);
             }
             ScopedPrinter &P;
             PDBFile &F;

   if (!opts::DumpSectionContribs)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
+  auto Dbi = File.getPDBDbiStream();
+  if (!Dbi)
+    return Dbi.takeError();
+
   ListScope L(P, "Section Contributions");
   class Visitor : public ISectionContribVisitor {
   public:

     ScopedPrinter &P;
     DbiStream &DS;
   };
-  Visitor V(P, DS);
-  DS.visitSectionContributions(V);
+  Visitor V(P, *Dbi);
+  Dbi->visitSectionContributions(V);
   return Error::success();
 }
 

   if (!opts::DumpSectionMap)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
+  auto Dbi = File.getPDBDbiStream();
+  if (!Dbi)
+    return Dbi.takeError();
+
   ListScope L(P, "Section Map");
-  for (auto &M : DS.getSectionMap()) {
+  for (auto &M : Dbi->getSectionMap()) {
     DictScope D(P, "Entry");
     P.printFlags("Flags", M.Flags, getOMFSegMapDescFlagNames());
     P.printNumber("Flags", M.Flags);

     return Error::success();
 
   DictScope D(P, "Publics Stream");
-  auto PublicsS = File.getPDBPublicsStream();
-  if (auto EC = PublicsS.takeError())
-    return EC;
-  PublicsStream &Publics = PublicsS.get();
-  P.printNumber("Stream number", Publics.getStreamNum());
-  P.printNumber("SymHash", Publics.getSymHash());
-  P.printNumber("AddrMap", Publics.getAddrMap());
-  P.printNumber("Number of buckets", Publics.getNumBuckets());
-  P.printList("Hash Buckets", Publics.getHashBuckets());
-  P.printList("Address Map", Publics.getAddressMap());
-  P.printList("Thunk Map", Publics.getThunkMap());
-  P.printList("Section Offsets", Publics.getSectionOffsets(),
+  auto Publics = File.getPDBPublicsStream();
+  if (!Publics)
+    return Publics.takeError();
+
+  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());
+  P.printNumber("Number of buckets", Publics->getNumBuckets());
+  P.printList("Hash Buckets", Publics->getHashBuckets());
+  P.printList("Address Map", Publics->getAddressMap());
+  P.printList("Thunk Map", Publics->getThunkMap());
+  P.printList("Section Offsets", Publics->getSectionOffsets(),
               printSectionOffset);
   ListScope L(P, "Symbols");
   codeview::CVSymbolDumper SD(P, TD, nullptr, false);
   bool HadError = false;
-  for (auto S : Publics.getSymbols(&HadError)) {
+  for (auto S : Publics->getSymbols(&HadError)) {
     DictScope DD(P, "");
 
     SD.dump(S);

   if (!opts::DumpSectionHeaders)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
+  auto Dbi = File.getPDBDbiStream();
+  if (!Dbi)
+    return Dbi.takeError();
 
   ListScope D(P, "Section Headers");
-  for (const object::coff_section &Section : DS.getSectionHeaders()) {
+  for (const object::coff_section &Section : Dbi->getSectionHeaders()) {
     DictScope DD(P, "");
 
     // If a name is 8 characters long, there is no NUL character at end.

   if (!opts::DumpFpo)
     return Error::success();
 
-  auto DbiS = File.getPDBDbiStream();
-  if (auto EC = DbiS.takeError())
-    return EC;
-  DbiStream &DS = DbiS.get();
+  auto Dbi = File.getPDBDbiStream();
+  if (!Dbi)
+    return Dbi.takeError();
 
   ListScope D(P, "New FPO");
-  for (const object::FpoData &Fpo : DS.getFpoRecords()) {
+  for (const object::FpoData &Fpo : Dbi->getFpoRecords()) {
     DictScope DD(P, "");
     P.printNumber("Offset", Fpo.Offset);
     P.printNumber("Size", Fpo.Size);