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

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu May 4 16:53:54 PDT 2017


Author: zturner
Date: Thu May  4 18:53:54 2017
New Revision: 302206

URL: http://llvm.org/viewvc/llvm-project?rev=302206&view=rev
Log:
[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

Modified:
    llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h
    llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp
    llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp

Modified: llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h?rev=302206&r1=302205&r2=302206&view=diff
==============================================================================
--- llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h (original)
+++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h Thu May  4 18:53:54 2017
@@ -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;
 

Modified: llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp?rev=302206&r1=302205&r2=302206&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp (original)
+++ llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp Thu May  4 18:53:54 2017
@@ -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() con
 
 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::getTypeHashStreamAux
   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 {

Modified: llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp?rev=302206&r1=302205&r2=302206&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp (original)
+++ llvm/trunk/tools/llvm-pdbdump/LLVMOutputStyle.cpp Thu May  4 18:53:54 2017
@@ -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(uin
 
   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(uin
 
   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
 
   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));




More information about the llvm-commits mailing list