[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

8 files changed, 103 insertions, 158 deletions

diff --git a/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp b/llvm/lib/DebugInfo/CodeView/RecordSerialization.cpp
index 6446670f60d..bff9a619a84 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 9b1f37943e6..5840ab1e36b 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 d77676dbd1c..00000000000
--- 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 f59d587898f..3fb26c8927b 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 d3d28113e8b..1fe35a691c3 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 0b6492efc70..f19e70ee8e3 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 3b4e18de8f9..7a48e3d1596 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 28c4a8fc35d..2232f51da86 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"