[pdb] Don't verify TPI hash values up front.

Verifying the hash values as we are currently doing
results in iterating every type record before the user
even tries to access the first one, and the API user
has no control over, or ability to hook into this
process.

As a result, when the user wants to iterate over types
to print them or index them, this results in a second
iteration over the same list of types.  When there's
upwards of 1,000,000 type records, this is obviously
quite undesirable.

This patch raises the verification outside of TpiStream
, and llvm-pdbdump hooks a hash verification visitor
into the normal dumping process.  So we still verify
the hash records, but we can do it while not requiring
a second iteration over the type stream.

Differential Revision: https://reviews.llvm.org/D32873

llvm-svn: 302206

3 files changed, 21 insertions, 33 deletions

diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/TpiStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/TpiStream.h
index 62dde0ef08b..9fef9bee5e1 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/TpiStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/TpiStream.h
@@ -40,12 +40,12 @@ public:
 
   uint32_t TypeIndexBegin() const;
   uint32_t TypeIndexEnd() const;
-  uint32_t NumTypeRecords() const;
+  uint32_t getNumTypeRecords() const;
   uint16_t getTypeHashStreamIndex() const;
   uint16_t getTypeHashStreamAuxIndex() const;
 
   uint32_t getHashKeySize() const;
-  uint32_t NumHashBuckets() const;
+  uint32_t getNumHashBuckets() const;
   FixedStreamArray<support::ulittle32_t> getHashValues() const;
   FixedStreamArray<TypeIndexOffset> getTypeIndexOffsets() const;
   HashTable &getHashAdjusters();
@@ -55,8 +55,6 @@ public:
   Error commit();
 
 private:
-  Error verifyHashValues();
-
   const PDBFile &Pdb;
   std::unique_ptr<msf::MappedBlockStream> Stream;
 
diff --git a/llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp b/llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp
index 5fef3edf8c2..c0999d93dbb 100644
--- a/llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/TpiStream.cpp
@@ -39,20 +39,6 @@ TpiStream::TpiStream(const PDBFile &File,
 
 TpiStream::~TpiStream() = default;
 
-// Verifies that a given type record matches with a given hash value.
-// Currently we only verify SRC_LINE records.
-Error TpiStream::verifyHashValues() {
-  TpiHashVerifier Verifier(HashValues, Header->NumHashBuckets);
-  TypeDeserializer Deserializer;
-
-  TypeVisitorCallbackPipeline Pipeline;
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(Verifier);
-
-  CVTypeVisitor Visitor(Pipeline);
-  return Visitor.visitTypeStream(TypeRecords);
-}
-
 Error TpiStream::reload() {
   BinaryStreamReader Reader(*Stream);
 
@@ -98,7 +84,7 @@ Error TpiStream::reload() {
     // There should be a hash value for every type record, or no hashes at all.
     uint32_t NumHashValues =
         Header->HashValueBuffer.Length / sizeof(ulittle32_t);
-    if (NumHashValues != NumTypeRecords() && NumHashValues != 0)
+    if (NumHashValues != getNumTypeRecords() && NumHashValues != 0)
       return make_error<RawError>(
           raw_error_code::corrupt_file,
           "TPI hash count does not match with the number of type records.");
@@ -122,12 +108,6 @@ Error TpiStream::reload() {
     }
 
     HashStream = std::move(HS);
-
-    // TPI hash table is a parallel array for the type records.
-    // Verify that the hash values match with type records.
-    if (NumHashValues > 0)
-      if (auto EC = verifyHashValues())
-        return EC;
   }
 
   return Error::success();
@@ -142,7 +122,7 @@ uint32_t TpiStream::TypeIndexBegin() const { return Header->TypeIndexBegin; }
 
 uint32_t TpiStream::TypeIndexEnd() const { return Header->TypeIndexEnd; }
 
-uint32_t TpiStream::NumTypeRecords() const {
+uint32_t TpiStream::getNumTypeRecords() const {
   return TypeIndexEnd() - TypeIndexBegin();
 }
 
@@ -154,7 +134,7 @@ uint16_t TpiStream::getTypeHashStreamAuxIndex() const {
   return Header->HashAuxStreamIndex;
 }
 
-uint32_t TpiStream::NumHashBuckets() const { return Header->NumHashBuckets; }
+uint32_t TpiStream::getNumHashBuckets() const { return Header->NumHashBuckets; }
 uint32_t TpiStream::getHashKeySize() const { return Header->HashKeySize; }
 
 FixedStreamArray<support::ulittle32_t> TpiStream::getHashValues() const {
diff --git a/llvm/tools/llvm-pdbdump/LLVMOutputStyle.cpp b/llvm/tools/llvm-pdbdump/LLVMOutputStyle.cpp
index 1af62b4e8df..ca7efa69939 100644
--- a/llvm/tools/llvm-pdbdump/LLVMOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbdump/LLVMOutputStyle.cpp
@@ -39,6 +39,7 @@
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Native/PublicsStream.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
+#include "llvm/DebugInfo/PDB/Native/TpiHashing.h"
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 #include "llvm/DebugInfo/PDB/PDBExtras.h"
 #include "llvm/Object/COFF.h"
@@ -622,7 +623,7 @@ Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
 
   StreamScope = llvm::make_unique<DictScope>(P, Label);
   P.printNumber(VerLabel, Tpi->getTpiVersion());
-  P.printNumber("Record count", Tpi->NumTypeRecords());
+  P.printNumber("Record count", Tpi->getNumTypeRecords());
 
   Optional<TypeDatabase> &StreamDB = (StreamIdx == StreamTPI) ? TypeDB : ItemDB;
 
@@ -682,7 +683,7 @@ Error LLVMOutputStyle::dumpTpiStream(uint32_t StreamIdx) {
 
   if (DumpTpiHash) {
     DictScope DD(P, "Hash");
-    P.printNumber("Number of Hash Buckets", Tpi->NumHashBuckets());
+    P.printNumber("Number of Hash Buckets", Tpi->getNumHashBuckets());
     P.printNumber("Hash Key Size", Tpi->getHashKeySize());
     P.printList("Values", Tpi->getHashValues());
 
@@ -723,16 +724,25 @@ Error LLVMOutputStyle::buildTypeDatabase(uint32_t SN) {
 
   DB.emplace();
 
+  auto Tpi =
+      (SN == StreamTPI) ? File.getPDBTpiStream() : File.getPDBIpiStream();
+
+  if (!Tpi)
+    return Tpi.takeError();
+
   TypeVisitorCallbackPipeline Pipeline;
   TypeDeserializer Deserializer;
   TypeDatabaseVisitor DBV(*DB);
   Pipeline.addCallbackToPipeline(Deserializer);
   Pipeline.addCallbackToPipeline(DBV);
 
-  auto Tpi =
-      (SN == StreamTPI) ? File.getPDBTpiStream() : File.getPDBIpiStream();
-  if (!Tpi)
-    return Tpi.takeError();
+  auto HashValues = Tpi->getHashValues();
+  std::unique_ptr<TpiHashVerifier> HashVerifier;
+  if (!HashValues.empty()) {
+    HashVerifier =
+        make_unique<TpiHashVerifier>(HashValues, Tpi->getNumHashBuckets());
+    Pipeline.addCallbackToPipeline(*HashVerifier);
+  }
 
   CVTypeVisitor Visitor(Pipeline);
   return Visitor.visitTypeStream(Tpi->types(nullptr));