From: Reid Kleckner <rnk@google.com>
Date: Wed, 26 Jul 2017 00:40:36 +0000 (+0000)
Subject: [PDB] Improve GSI hash table dumping for publics and globals
X-Git-Tag: llvmorg-6.0.0-rc1~11777
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=14d90fd05cbed5fd3fcee492f072a4e5816f20b5;p=platform%2Fupstream%2Fllvm.git

[PDB] Improve GSI hash table dumping for publics and globals

The PDB "symbol stream" actually contains symbol records for the publics
and the globals stream. The globals and publics streams are essentially
hash tables that point into a single stream of records. In order to
match cvdump's behavior, we need to only dump symbol records referenced
from the hash table. This patch implements that, and then implements
global stream dumping, since it's just a subset of public stream
dumping.

Now we shouldn't see S_PROCREF or S_GDATA32 records when dumping
publics, and instead we should see those record in the globals stream.

llvm-svn: 309066
---

diff --git a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
index 44040e0..9f3a753 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
@@ -61,30 +61,38 @@ template <typename Kind> struct RemappedRecord {
   SmallVector<std::pair<uint32_t, TypeIndex>, 8> Mappings;
 };
 
+/// Read a complete record from a stream at a random offset.
+template <typename Kind>
+inline Expected<CVRecord<Kind>> readCVRecordFromStream(BinaryStreamRef Stream,
+                                                       uint32_t Offset) {
+  const RecordPrefix *Prefix = nullptr;
+  BinaryStreamReader Reader(Stream);
+  Reader.setOffset(Offset);
+
+  if (auto EC = Reader.readObject(Prefix))
+    return std::move(EC);
+  if (Prefix->RecordLen < 2)
+    return make_error<CodeViewError>(cv_error_code::corrupt_record);
+  Kind K = static_cast<Kind>(uint16_t(Prefix->RecordKind));
+
+  Reader.setOffset(Offset);
+  ArrayRef<uint8_t> RawData;
+  if (auto EC = Reader.readBytes(RawData, Prefix->RecordLen + sizeof(uint16_t)))
+    return std::move(EC);
+  return codeview::CVRecord<Kind>(K, RawData);
+}
+
 } // end namespace codeview
 
 template <typename Kind>
 struct VarStreamArrayExtractor<codeview::CVRecord<Kind>> {
   Error operator()(BinaryStreamRef Stream, uint32_t &Len,
                    codeview::CVRecord<Kind> &Item) {
-    using namespace codeview;
-    const RecordPrefix *Prefix = nullptr;
-    BinaryStreamReader Reader(Stream);
-    uint32_t Offset = Reader.getOffset();
-
-    if (auto EC = Reader.readObject(Prefix))
-      return EC;
-    if (Prefix->RecordLen < 2)
-      return make_error<CodeViewError>(cv_error_code::corrupt_record);
-    Kind K = static_cast<Kind>(uint16_t(Prefix->RecordKind));
-
-    Reader.setOffset(Offset);
-    ArrayRef<uint8_t> RawData;
-    if (auto EC =
-            Reader.readBytes(RawData, Prefix->RecordLen + sizeof(uint16_t)))
-      return EC;
-    Item = codeview::CVRecord<Kind>(K, RawData);
-    Len = Item.length();
+    auto ExpectedRec = codeview::readCVRecordFromStream<Kind>(Stream, 0);
+    if (!ExpectedRec)
+      return ExpectedRec.takeError();
+    Item = *ExpectedRec;
+    Len = ExpectedRec->length();
     return Error::success();
   }
 };
diff --git a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
index f3086cf..934944a 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h
@@ -945,6 +945,9 @@ public:
 using CVSymbol = CVRecord<SymbolKind>;
 using CVSymbolArray = VarStreamArray<CVSymbol>;
 
+Expected<CVSymbol> readSymbolFromStream(BinaryStreamRef Stream,
+                                        uint32_t Offset);
+
 } // end namespace codeview
 } // end namespace llvm
 
diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
index 8a7326f..615ca74 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
@@ -16,28 +16,61 @@
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
 #include "llvm/Support/BinaryStreamArray.h"
 #include "llvm/Support/Error.h"
+#include "llvm/ADT/iterator.h"
 
 namespace llvm {
 namespace pdb {
 class DbiStream;
 class PDBFile;
 
+/// Iterator over hash records producing symbol record offsets. Abstracts away
+/// the fact that symbol record offsets on disk are off-by-one.
+class GSIHashIterator
+    : public iterator_adaptor_base<
+          GSIHashIterator, FixedStreamArrayIterator<PSHashRecord>,
+          std::random_access_iterator_tag, const uint32_t> {
+public:
+  GSIHashIterator() = default;
+
+  template <typename T>
+  GSIHashIterator(T &&v)
+      : GSIHashIterator::iterator_adaptor_base(std::forward<T &&>(v)) {}
+
+  uint32_t operator*() const {
+    uint32_t Off = this->I->Off;
+    return --Off;
+  }
+};
+
+/// From https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/dbi/gsi.cpp
+enum : unsigned { IPHR_HASH = 4096 };
+
+/// A readonly view of a hash table used in the globals and publics streams.
+/// Most clients will only want to iterate this to get symbol record offsets
+/// into the PDB symbol stream.
+class GSIHashTable {
+public:
+  const GSIHashHeader *HashHdr;
+  FixedStreamArray<PSHashRecord> HashRecords;
+  ArrayRef<uint8_t> HashBitmap;
+  FixedStreamArray<support::ulittle32_t> HashBuckets;
+
+  Error read(BinaryStreamReader &Reader);
+
+  typedef GSIHashHeader iterator;
+  GSIHashIterator begin() const { return GSIHashIterator(HashRecords.begin()); }
+  GSIHashIterator end() const { return GSIHashIterator(HashRecords.end()); }
+};
+
 class GlobalsStream {
 public:
   explicit GlobalsStream(std::unique_ptr<msf::MappedBlockStream> Stream);
   ~GlobalsStream();
-  Error commit();
-  FixedStreamArray<support::ulittle32_t> getHashBuckets() const {
-    return HashBuckets;
-  }
-  uint32_t getNumBuckets() const { return NumBuckets; }
+  const GSIHashTable &getGlobalsTable() const { return GlobalsTable; }
   Error reload();
 
 private:
-  FixedStreamArray<PSHashRecord> HashRecords;
-  ArrayRef<uint8_t> HashBitmap;
-  FixedStreamArray<support::ulittle32_t> HashBuckets;
-  uint32_t NumBuckets;
+  GSIHashTable GlobalsTable;
   std::unique_ptr<msf::MappedBlockStream> Stream;
 };
 }
diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
index d79745c..f28628d 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
@@ -12,6 +12,7 @@
 
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 #include "llvm/DebugInfo/PDB/Native/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Native/RawTypes.h"
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
@@ -26,21 +27,13 @@ class PDBFile;
 
 class PublicsStream {
 public:
-  PublicsStream(PDBFile &File, std::unique_ptr<msf::MappedBlockStream> Stream);
+  PublicsStream(std::unique_ptr<msf::MappedBlockStream> Stream);
   ~PublicsStream();
   Error reload();
 
   uint32_t getSymHash() const;
   uint32_t getAddrMap() const;
-  uint32_t getNumBuckets() const { return NumBuckets; }
-  Expected<const codeview::CVSymbolArray &> getSymbolArray() const;
-  iterator_range<codeview::CVSymbolArray::Iterator>
-  getSymbols(bool *HadError) const;
-  FixedStreamArray<PSHashRecord> getHashRecords() const { return HashRecords; }
-  ArrayRef<uint8_t> getHashBitmap() const { return HashBitmap; }
-  FixedStreamArray<support::ulittle32_t> getHashBuckets() const {
-    return HashBuckets;
-  }
+  const GSIHashTable &getPublicsTable() const { return PublicsTable; }
   FixedStreamArray<support::ulittle32_t> getAddressMap() const {
     return AddressMap;
   }
@@ -51,22 +44,14 @@ public:
     return SectionOffsets;
   }
 
-  Error commit();
-
 private:
-  PDBFile &Pdb;
-
   std::unique_ptr<msf::MappedBlockStream> Stream;
-  uint32_t NumBuckets = 0;
-  FixedStreamArray<PSHashRecord> HashRecords;
-  ArrayRef<uint8_t> HashBitmap;
-  FixedStreamArray<support::ulittle32_t> HashBuckets;
+  GSIHashTable PublicsTable;
   FixedStreamArray<support::ulittle32_t> AddressMap;
   FixedStreamArray<support::ulittle32_t> ThunkMap;
   FixedStreamArray<SectionOffset> SectionOffsets;
 
   const PublicsStreamHeader *Header;
-  const GSIHashHeader *HashHdr;
 };
 }
 }
diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/RawTypes.h b/llvm/include/llvm/DebugInfo/PDB/Native/RawTypes.h
index b6321cb..8cc0836 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/RawTypes.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/RawTypes.h
@@ -23,6 +23,20 @@ struct SectionOffset {
   char Padding[2];
 };
 
+/// Header of the hash tables found in the globals and publics sections.
+/// Based on GSIHashHdr in
+/// https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/dbi/gsi.h
+struct GSIHashHeader {
+  enum : unsigned {
+    HdrSignature = ~0U,
+    HdrVersion = 0xeffe0000 + 19990810,
+  };
+  support::ulittle32_t VerSignature;
+  support::ulittle32_t VerHdr;
+  support::ulittle32_t HrSize;
+  support::ulittle32_t NumBuckets;
+};
+
 // This is HRFile.
 struct PSHashRecord {
   support::ulittle32_t Off; // Offset in the symbol record stream
diff --git a/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp b/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp
index 6446670..bff9a61 100644
--- a/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp
+++ b/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/DebugInfo/CodeView/CodeViewError.h"
+#include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/Support/BinaryByteStream.h"
 
@@ -147,3 +148,8 @@ Error llvm::codeview::consume(BinaryStreamReader &Reader, StringRef &Item) {
 
   return Reader.readCString(Item);
 }
+
+Expected<CVSymbol> llvm::codeview::readSymbolFromStream(BinaryStreamRef Stream,
+                                                        uint32_t Offset) {
+  return readCVRecordFromStream<SymbolKind>(Stream, Offset);
+}
diff --git a/llvm/lib/DebugInfo/PDB/CMakeLists.txt b/llvm/lib/DebugInfo/PDB/CMakeLists.txt
index 9b1f379..5840ab1 100644
--- a/llvm/lib/DebugInfo/PDB/CMakeLists.txt
+++ b/llvm/lib/DebugInfo/PDB/CMakeLists.txt
@@ -35,7 +35,6 @@ add_pdb_impl_folder(Native
   Native/DbiStreamBuilder.cpp
   Native/EnumTables.cpp
   Native/GlobalsStream.cpp
-  Native/GSI.cpp
   Native/Hash.cpp
   Native/HashTable.cpp
   Native/InfoStream.cpp
diff --git a/llvm/lib/DebugInfo/PDB/Native/GSI.cpp b/llvm/lib/DebugInfo/PDB/Native/GSI.cpp
deleted file mode 100644
index d77676d..0000000
--- a/llvm/lib/DebugInfo/PDB/Native/GSI.cpp
+++ /dev/null
@@ -1,93 +0,0 @@
-//===- GSI.cpp - Common Functions for GlobalsStream and PublicsStream  ----===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "GSI.h"
-
-#include "llvm/DebugInfo/PDB/Native/RawError.h"
-#include "llvm/DebugInfo/PDB/Native/RawTypes.h"
-#include "llvm/Support/BinaryStreamArray.h"
-#include "llvm/Support/BinaryStreamReader.h"
-
-#include "llvm/Support/Error.h"
-
-namespace llvm {
-namespace pdb {
-
-static Error checkHashHdrVersion(const GSIHashHeader *HashHdr) {
-  if (HashHdr->VerHdr != GSIHashHeader::HdrVersion)
-    return make_error<RawError>(
-        raw_error_code::feature_unsupported,
-        "Encountered unsupported globals stream version.");
-
-  return Error::success();
-}
-
-Error readGSIHashBuckets(FixedStreamArray<support::ulittle32_t> &HashBuckets,
-                         ArrayRef<uint8_t> &HashBitmap,
-                         const GSIHashHeader *HashHdr,
-                         BinaryStreamReader &Reader) {
-  if (auto EC = checkHashHdrVersion(HashHdr))
-    return EC;
-
-  // Before the actual hash buckets, there is a bitmap of length determined by
-  // IPHR_HASH.
-  size_t BitmapSizeInBits = alignTo(IPHR_HASH + 1, 32);
-  uint32_t NumBitmapEntries = BitmapSizeInBits / 8;
-  if (auto EC = Reader.readBytes(HashBitmap, NumBitmapEntries))
-    return joinErrors(std::move(EC),
-                      make_error<RawError>(raw_error_code::corrupt_file,
-                                           "Could not read a bitmap."));
-  uint32_t NumBuckets = 0;
-  for (uint8_t B : HashBitmap)
-    NumBuckets += countPopulation(B);
-
-  // Hash buckets follow.
-  if (auto EC = Reader.readArray(HashBuckets, NumBuckets))
-    return joinErrors(std::move(EC),
-                      make_error<RawError>(raw_error_code::corrupt_file,
-                                           "Hash buckets corrupted."));
-
-  return Error::success();
-}
-
-Error readGSIHashHeader(const GSIHashHeader *&HashHdr,
-                        BinaryStreamReader &Reader) {
-  if (Reader.readObject(HashHdr))
-    return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Stream does not contain a GSIHashHeader.");
-
-  if (HashHdr->VerSignature != GSIHashHeader::HdrSignature)
-    return make_error<RawError>(
-        raw_error_code::feature_unsupported,
-        "GSIHashHeader signature (0xffffffff) not found.");
-
-  return Error::success();
-}
-
-Error readGSIHashRecords(FixedStreamArray<PSHashRecord> &HashRecords,
-                         const GSIHashHeader *HashHdr,
-                         BinaryStreamReader &Reader) {
-  if (auto EC = checkHashHdrVersion(HashHdr))
-    return EC;
-
-  // HashHdr->HrSize specifies the number of bytes of PSHashRecords we have.
-  // Verify that we can read them all.
-  if (HashHdr->HrSize % sizeof(PSHashRecord))
-    return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Invalid HR array size.");
-  uint32_t NumHashRecords = HashHdr->HrSize / sizeof(PSHashRecord);
-  if (auto EC = Reader.readArray(HashRecords, NumHashRecords))
-    return joinErrors(std::move(EC),
-                      make_error<RawError>(raw_error_code::corrupt_file,
-                                           "Error reading hash records."));
-
-  return Error::success();
-}
-}
-}
diff --git a/llvm/lib/DebugInfo/PDB/Native/GSI.h b/llvm/lib/DebugInfo/PDB/Native/GSI.h
index f59d587..3fb26c8 100644
--- a/llvm/lib/DebugInfo/PDB/Native/GSI.h
+++ b/llvm/lib/DebugInfo/PDB/Native/GSI.h
@@ -37,23 +37,6 @@ class BinaryStreamReader;
 
 namespace pdb {
 
-/// From https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/dbi/gsi.cpp
-static const unsigned IPHR_HASH = 4096;
-
-/// Header of the hash tables found in the globals and publics sections.
-/// Based on GSIHashHdr in
-/// https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/dbi/gsi.h
-struct GSIHashHeader {
-  enum : unsigned {
-    HdrSignature = ~0U,
-    HdrVersion = 0xeffe0000 + 19990810,
-  };
-  support::ulittle32_t VerSignature;
-  support::ulittle32_t VerHdr;
-  support::ulittle32_t HrSize;
-  support::ulittle32_t NumBuckets;
-};
-
 Error readGSIHashBuckets(FixedStreamArray<support::ulittle32_t> &HashBuckets,
                          ArrayRef<uint8_t> &HashBitmap,
                          const GSIHashHeader *HashHdr,
diff --git a/llvm/lib/DebugInfo/PDB/Native/GlobalsStream.cpp b/llvm/lib/DebugInfo/PDB/Native/GlobalsStream.cpp
index d3d2811..1fe35a6 100644
--- a/llvm/lib/DebugInfo/PDB/Native/GlobalsStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/GlobalsStream.cpp
@@ -6,9 +6,21 @@
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
+//
+// The on-disk structores used in this file are based on the reference
+// implementation which is available at
+// https://github.com/Microsoft/microsoft-pdb/blob/master/PDB/dbi/gsi.h
+//
+// When you are reading the reference source code, you'd find the
+// information below useful.
+//
+//  - ppdb1->m_fMinimalDbgInfo seems to be always true.
+//  - SMALLBUCKETS macro is defined.
+//
+//===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
-#include "GSI.h"
+#include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Error.h"
 #include <algorithm>
@@ -24,19 +36,88 @@ GlobalsStream::~GlobalsStream() = default;
 
 Error GlobalsStream::reload() {
   BinaryStreamReader Reader(*Stream);
+  if (auto E = GlobalsTable.read(Reader))
+    return E;
+  return Error::success();
+}
 
-  const GSIHashHeader *HashHdr;
-  if (auto EC = readGSIHashHeader(HashHdr, Reader))
-    return EC;
+static Error checkHashHdrVersion(const GSIHashHeader *HashHdr) {
+  if (HashHdr->VerHdr != GSIHashHeader::HdrVersion)
+    return make_error<RawError>(
+        raw_error_code::feature_unsupported,
+        "Encountered unsupported globals stream version.");
 
-  if (auto EC = readGSIHashRecords(HashRecords, HashHdr, Reader))
+  return Error::success();
+}
+
+static Error readGSIHashHeader(const GSIHashHeader *&HashHdr,
+                               BinaryStreamReader &Reader) {
+  if (Reader.readObject(HashHdr))
+    return make_error<RawError>(raw_error_code::corrupt_file,
+                                "Stream does not contain a GSIHashHeader.");
+
+  if (HashHdr->VerSignature != GSIHashHeader::HdrSignature)
+    return make_error<RawError>(
+        raw_error_code::feature_unsupported,
+        "GSIHashHeader signature (0xffffffff) not found.");
+
+  return Error::success();
+}
+
+static Error readGSIHashRecords(FixedStreamArray<PSHashRecord> &HashRecords,
+                                const GSIHashHeader *HashHdr,
+                                BinaryStreamReader &Reader) {
+  if (auto EC = checkHashHdrVersion(HashHdr))
     return EC;
 
-  if (auto EC = readGSIHashBuckets(HashBuckets, HashBitmap, HashHdr, Reader))
+  // HashHdr->HrSize specifies the number of bytes of PSHashRecords we have.
+  // Verify that we can read them all.
+  if (HashHdr->HrSize % sizeof(PSHashRecord))
+    return make_error<RawError>(raw_error_code::corrupt_file,
+                                "Invalid HR array size.");
+  uint32_t NumHashRecords = HashHdr->HrSize / sizeof(PSHashRecord);
+  if (auto EC = Reader.readArray(HashRecords, NumHashRecords))
+    return joinErrors(std::move(EC),
+                      make_error<RawError>(raw_error_code::corrupt_file,
+                                           "Error reading hash records."));
+
+  return Error::success();
+}
+
+static Error
+readGSIHashBuckets(FixedStreamArray<support::ulittle32_t> &HashBuckets,
+                   ArrayRef<uint8_t> &HashBitmap, const GSIHashHeader *HashHdr,
+                   BinaryStreamReader &Reader) {
+  if (auto EC = checkHashHdrVersion(HashHdr))
     return EC;
-  NumBuckets = HashBuckets.size();
+
+  // Before the actual hash buckets, there is a bitmap of length determined by
+  // IPHR_HASH.
+  size_t BitmapSizeInBits = alignTo(IPHR_HASH + 1, 32);
+  uint32_t NumBitmapEntries = BitmapSizeInBits / 8;
+  if (auto EC = Reader.readBytes(HashBitmap, NumBitmapEntries))
+    return joinErrors(std::move(EC),
+                      make_error<RawError>(raw_error_code::corrupt_file,
+                                           "Could not read a bitmap."));
+  uint32_t NumBuckets = 0;
+  for (uint8_t B : HashBitmap)
+    NumBuckets += countPopulation(B);
+
+  // Hash buckets follow.
+  if (auto EC = Reader.readArray(HashBuckets, NumBuckets))
+    return joinErrors(std::move(EC),
+                      make_error<RawError>(raw_error_code::corrupt_file,
+                                           "Hash buckets corrupted."));
 
   return Error::success();
 }
 
-Error GlobalsStream::commit() { return Error::success(); }
+Error GSIHashTable::read(BinaryStreamReader &Reader) {
+  if (auto EC = readGSIHashHeader(HashHdr, Reader))
+    return EC;
+  if (auto EC = readGSIHashRecords(HashRecords, HashHdr, Reader))
+    return EC;
+  if (auto EC = readGSIHashBuckets(HashBuckets, HashBitmap, HashHdr, Reader))
+    return EC;
+  return Error::success();
+}
diff --git a/llvm/lib/DebugInfo/PDB/Native/PDBFile.cpp b/llvm/lib/DebugInfo/PDB/Native/PDBFile.cpp
index 0b6492e..f19e70e 100644
--- a/llvm/lib/DebugInfo/PDB/Native/PDBFile.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/PDBFile.cpp
@@ -318,8 +318,7 @@ Expected<PublicsStream &> PDBFile::getPDBPublicsStream() {
         ContainerLayout, *Buffer, DbiS->getPublicSymbolStreamIndex());
     if (!PublicS)
       return PublicS.takeError();
-    auto TempPublics =
-        llvm::make_unique<PublicsStream>(*this, std::move(*PublicS));
+    auto TempPublics = llvm::make_unique<PublicsStream>(std::move(*PublicS));
     if (auto EC = TempPublics->reload())
       return std::move(EC);
     Publics = std::move(TempPublics);
diff --git a/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp b/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
index 3b4e18d..7a48e3d 100644
--- a/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
@@ -41,9 +41,8 @@ using namespace llvm::msf;
 using namespace llvm::support;
 using namespace llvm::pdb;
 
-PublicsStream::PublicsStream(PDBFile &File,
-                             std::unique_ptr<MappedBlockStream> Stream)
-    : Pdb(File), Stream(std::move(Stream)) {}
+PublicsStream::PublicsStream(std::unique_ptr<MappedBlockStream> Stream)
+    : Stream(std::move(Stream)) {}
 
 PublicsStream::~PublicsStream() = default;
 
@@ -64,20 +63,14 @@ Error PublicsStream::reload() {
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Publics Stream does not contain a header.");
 
-  // Read PSGSIHDR and GSIHashHdr structs.
+  // Read PSGSIHDR struct.
   if (Reader.readObject(Header))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Publics Stream does not contain a header.");
 
-  if (auto EC = readGSIHashHeader(HashHdr, Reader))
-    return EC;
-
-  if (auto EC = readGSIHashRecords(HashRecords, HashHdr, Reader))
-    return EC;
-
-  if (auto EC = readGSIHashBuckets(HashBuckets, HashBitmap, HashHdr, Reader))
-    return EC;
-  NumBuckets = HashBuckets.size();
+  // Read the hash table.
+  if (auto E = PublicsTable.read(Reader))
+    return E;
 
   // Something called "address map" follows.
   uint32_t NumAddressMapEntries = Header->AddrMap / sizeof(uint32_t);
@@ -105,26 +98,3 @@ Error PublicsStream::reload() {
                                 "Corrupted publics stream.");
   return Error::success();
 }
-
-iterator_range<codeview::CVSymbolArray::Iterator>
-PublicsStream::getSymbols(bool *HadError) const {
-  auto SymbolS = Pdb.getPDBSymbolStream();
-  if (SymbolS.takeError()) {
-    codeview::CVSymbolArray::Iterator Iter;
-    return make_range(Iter, Iter);
-  }
-  SymbolStream &SS = SymbolS.get();
-
-  return SS.getSymbols(HadError);
-}
-
-Expected<const codeview::CVSymbolArray &>
-PublicsStream::getSymbolArray() const {
-  auto SymbolS = Pdb.getPDBSymbolStream();
-  if (!SymbolS)
-    return SymbolS.takeError();
-
-  return SymbolS->getSymbolArray();
-}
-
-Error PublicsStream::commit() { return Error::success(); }
diff --git a/llvm/lib/DebugInfo/PDB/Native/PublicsStreamBuilder.cpp b/llvm/lib/DebugInfo/PDB/Native/PublicsStreamBuilder.cpp
index 28c4a8f..2232f51 100644
--- a/llvm/lib/DebugInfo/PDB/Native/PublicsStreamBuilder.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/PublicsStreamBuilder.cpp
@@ -8,10 +8,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/PublicsStreamBuilder.h"
-
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 
 #include "GSI.h"
 
diff --git a/llvm/test/DebugInfo/PDB/pdbdump-headers.test b/llvm/test/DebugInfo/PDB/pdbdump-headers.test
index 14fe4bb..bbf5002 100644
--- a/llvm/test/DebugInfo/PDB/pdbdump-headers.test
+++ b/llvm/test/DebugInfo/PDB/pdbdump-headers.test
@@ -452,16 +452,20 @@ ALL-NEXT:            0x100D: ` Kits\8.1\include\um" -I"C:\Program Files (x86)\Wi
 ALL:        Type Index Offsets:
 ALL-NEXT:     TI: 0x1000, Offset: 0
 ALL:        Hash Adjusters:
+ALL:                             Global Symbols
+ALL-NEXT: ============================================================
+ALL-NEXT:   56 | S_PROCREF [size = 20] `main`
+ALL-NEXT:       module = 1, sum name = 0, offset = 120
+ALL-NEXT:   76 | S_GDATA32 [size = 28] `__purecall`
+ALL-NEXT:       type = 0x0403 (void*), addr = 0003:0000
+ALL-NOT:   S_PUB32
 ALL:                             Public Symbols
 ALL-NEXT: ============================================================
 ALL-NEXT:    0 | S_PUB32 [size = 36] `?__purecall@@3PAXA`
 ALL-NEXT:       flags = none, addr = 0003:0000
 ALL-NEXT:   36 | S_PUB32 [size = 20] `_main`
 ALL-NEXT:       flags = function, addr = 0001:0016
-ALL-NEXT:   56 | S_PROCREF [size = 20] `main`
-ALL-NEXT:       module = 1, sum name = 0, offset = 120
-ALL-NEXT:   76 | S_GDATA32 [size = 28] `__purecall`
-ALL-NEXT:       type = 0x0403 (void*), addr = 0003:0000
+ALL-NOT:   S_PROCREF
 ALL:                                Symbols
 ALL-NEXT: ============================================================
 ALL-NEXT:   Mod 0000 | `d:\src\llvm\test\DebugInfo\PDB\Inputs\empty.obj`:
diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
index 07fc38d..605fff9 100644
--- a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
@@ -49,6 +49,7 @@
 #include "llvm/DebugInfo/PDB/Native/ModuleDebugStream.h"
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Native/PublicsStream.h"
+#include "llvm/DebugInfo/PDB/Native/SymbolStream.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/DebugInfo/PDB/Native/TpiHashing.h"
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
@@ -129,6 +130,11 @@ Error DumpOutputStyle::dump() {
       return EC;
   }
 
+  if (opts::dump::DumpGlobals) {
+    if (auto EC = dumpGlobals())
+      return EC;
+  }
+
   if (opts::dump::DumpPublics) {
     if (auto EC = dumpPublics())
       return EC;
@@ -851,58 +857,38 @@ Error DumpOutputStyle::dumpModuleSyms() {
   return Error::success();
 }
 
+Error DumpOutputStyle::dumpGlobals() {
+  printHeader(P, "Global Symbols");
+  AutoIndent Indent(P);
+  if (!File.hasPDBGlobalsStream()) {
+    P.formatLine("Globals stream not present");
+    return Error::success();
+  }
+  ExitOnError Err("Error dumping globals stream");
+  auto &Globals = Err(File.getPDBGlobalsStream());
+
+  const GSIHashTable &Table = Globals.getGlobalsTable();
+  Err(dumpSymbolsFromGSI(Table, opts::dump::DumpGlobalExtras));
+  return Error::success();
+}
+
 Error DumpOutputStyle::dumpPublics() {
   printHeader(P, "Public Symbols");
-
   AutoIndent Indent(P);
   if (!File.hasPDBPublicsStream()) {
     P.formatLine("Publics stream not present");
     return Error::success();
   }
-
   ExitOnError Err("Error dumping publics stream");
-
-  auto &Types = Err(initializeTypes(StreamTPI));
   auto &Publics = Err(File.getPDBPublicsStream());
-  SymbolVisitorCallbackPipeline Pipeline;
-  SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
-  MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, Types);
 
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(Dumper);
-  CVSymbolVisitor Visitor(Pipeline);
-
-  auto ExpectedSymbols = Publics.getSymbolArray();
-  if (!ExpectedSymbols) {
-    P.formatLine("Could not read public symbol record stream");
-    return Error::success();
-  }
-
-  if (auto EC = Visitor.visitSymbolStream(*ExpectedSymbols, 0))
-    P.formatLine("Error while processing public symbol records.  {0}",
-                 toString(std::move(EC)));
+  const GSIHashTable &PublicsTable = Publics.getPublicsTable();
+  Err(dumpSymbolsFromGSI(PublicsTable, opts::dump::DumpPublicExtras));
 
-  // Return early if we aren't dumping public hash table and address map info.
+  // Skip the rest if we aren't dumping extras.
   if (!opts::dump::DumpPublicExtras)
     return Error::success();
 
-  P.formatLine("Hash Records");
-  {
-    AutoIndent Indent2(P);
-    for (const PSHashRecord &HR : Publics.getHashRecords())
-      P.formatLine("off = {0}, refcnt = {1}", uint32_t(HR.Off),
-                   uint32_t(HR.CRef));
-  }
-
-  // FIXME: Dump the bitmap.
-
-  P.formatLine("Hash Buckets");
-  {
-    AutoIndent Indent2(P);
-    for (uint32_t Hash : Publics.getHashBuckets())
-      P.formatLine("{0:x8}", Hash);
-  }
-
   P.formatLine("Address Map");
   {
     // These are offsets into the publics stream sorted by secidx:secrel.
@@ -931,6 +917,56 @@ Error DumpOutputStyle::dumpPublics() {
   return Error::success();
 }
 
+Error DumpOutputStyle::dumpSymbolsFromGSI(const GSIHashTable &Table,
+                                          bool HashExtras) {
+  auto ExpectedSyms = File.getPDBSymbolStream();
+  if (!ExpectedSyms)
+    return ExpectedSyms.takeError();
+  auto ExpectedTypes = initializeTypes(StreamTPI);
+  if (!ExpectedTypes)
+    return ExpectedTypes.takeError();
+  SymbolVisitorCallbackPipeline Pipeline;
+  SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
+  MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, *ExpectedTypes);
+
+  Pipeline.addCallbackToPipeline(Deserializer);
+  Pipeline.addCallbackToPipeline(Dumper);
+  CVSymbolVisitor Visitor(Pipeline);
+
+  BinaryStreamRef SymStream =
+      ExpectedSyms->getSymbolArray().getUnderlyingStream();
+  for (uint32_t PubSymOff : Table) {
+    Expected<CVSymbol> Sym = readSymbolFromStream(SymStream, PubSymOff);
+    if (!Sym)
+      return Sym.takeError();
+    if (auto E = Visitor.visitSymbolRecord(*Sym, PubSymOff))
+      return E;
+  }
+
+  // Return early if we aren't dumping public hash table and address map info.
+  if (!HashExtras)
+    return Error::success();
+
+  P.formatLine("Hash Records");
+  {
+    AutoIndent Indent2(P);
+    for (const PSHashRecord &HR : Table.HashRecords)
+      P.formatLine("off = {0}, refcnt = {1}", uint32_t(HR.Off),
+                   uint32_t(HR.CRef));
+  }
+
+  // FIXME: Dump the bitmap.
+
+  P.formatLine("Hash Buckets");
+  {
+    AutoIndent Indent2(P);
+    for (uint32_t Hash : Table.HashBuckets)
+      P.formatLine("{0:x8}", Hash);
+  }
+
+  return Error::success();
+}
+
 static std::string formatSectionCharacteristics(uint32_t IndentLevel,
                                                 uint32_t C) {
   using SC = COFF::SectionCharacteristics;
diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.h b/llvm/tools/llvm-pdbutil/DumpOutputStyle.h
index 4c52289..d1d3e1d 100644
--- a/llvm/tools/llvm-pdbutil/DumpOutputStyle.h
+++ b/llvm/tools/llvm-pdbutil/DumpOutputStyle.h
@@ -26,6 +26,8 @@ class LazyRandomTypeCollection;
 }
 
 namespace pdb {
+class GSIHashTable;
+
 class DumpOutputStyle : public OutputStyle {
 public:
   DumpOutputStyle(PDBFile &File);
@@ -46,7 +48,9 @@ private:
   Error dumpModules();
   Error dumpModuleFiles();
   Error dumpModuleSyms();
+  Error dumpGlobals();
   Error dumpPublics();
+  Error dumpSymbolsFromGSI(const GSIHashTable &Table, bool HashExtras);
   Error dumpSectionContribs();
   Error dumpSectionMap();
 
diff --git a/llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp b/llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
index 338e4ee..aae5de2 100644
--- a/llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
+++ b/llvm/tools/llvm-pdbutil/llvm-pdbutil.cpp
@@ -450,6 +450,10 @@ cl::opt<bool> DumpTypeDependents(
     cl::cat(TypeOptions), cl::sub(DumpSubcommand));
 
 // SYMBOL OPTIONS
+cl::opt<bool> DumpGlobals("globals", cl::desc("dump Globals symbol records"),
+                          cl::cat(SymbolOptions), cl::sub(DumpSubcommand));
+cl::opt<bool> DumpGlobalExtras("global-extras", cl::desc("dump Globals hashes"),
+                               cl::cat(SymbolOptions), cl::sub(DumpSubcommand));
 cl::opt<bool> DumpPublics("publics", cl::desc("dump Publics stream data"),
                           cl::cat(SymbolOptions), cl::sub(DumpSubcommand));
 cl::opt<bool> DumpPublicExtras("public-extras",
@@ -1066,6 +1070,7 @@ int main(int argc_, const char *argv_[]) {
       opts::dump::DumpXme = true;
       opts::dump::DumpXmi = true;
       opts::dump::DumpIds = true;
+      opts::dump::DumpGlobals = true;
       opts::dump::DumpPublics = true;
       opts::dump::DumpSectionContribs = true;
       opts::dump::DumpSectionMap = true;
diff --git a/llvm/tools/llvm-pdbutil/llvm-pdbutil.h b/llvm/tools/llvm-pdbutil/llvm-pdbutil.h
index 4aeff99..0c01fae 100644
--- a/llvm/tools/llvm-pdbutil/llvm-pdbutil.h
+++ b/llvm/tools/llvm-pdbutil/llvm-pdbutil.h
@@ -143,6 +143,8 @@ extern llvm::cl::opt<bool> DumpIdExtras;
 extern llvm::cl::list<uint32_t> DumpIdIndex;
 extern llvm::cl::opt<bool> DumpSymbols;
 extern llvm::cl::opt<bool> DumpSymRecordBytes;
+extern llvm::cl::opt<bool> DumpGlobals;
+extern llvm::cl::opt<bool> DumpGlobalExtras;
 extern llvm::cl::opt<bool> DumpPublics;
 extern llvm::cl::opt<bool> DumpPublicExtras;
 extern llvm::cl::opt<bool> DumpSectionContribs;