[PATCH] D61277: [PDB] Fix hash function used to write /src/headerblock

[Phabricator via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359515: [PDB] Fix hash function used to write /src/headerblock (authored by nico, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61277?vs=197173&id=197213#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61277/new/

https://reviews.llvm.org/D61277

Files:
  llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h
  llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp
  llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp
  llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp


Index: llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp
===================================================================
--- llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp
+++ llvm/trunk/unittests/DebugInfo/PDB/StringTableBuilderTest.cpp
@@ -19,11 +19,7 @@
 using namespace llvm::pdb;
 using namespace llvm::support;
 
-namespace {
-class StringTableBuilderTest : public ::testing::Test {};
-}
-
-TEST_F(StringTableBuilderTest, Simple) {
+TEST(StringTableBuilderTest, Simple) {
   // Create /names table contents.
   PDBStringTableBuilder Builder;
 
@@ -78,3 +74,21 @@
   EXPECT_THAT_EXPECTED(Table.getIDForString("bazz"), HasValue(BazzID));
   EXPECT_THAT_EXPECTED(Table.getIDForString("barr"), HasValue(BarrID));
 }
+
+TEST(StringTableHashTraitsTest, Simple) {
+  PDBStringTableBuilder Builder;
+
+  // Create more than 64kiB of dummy entries.
+  for (int i = 0; i < 320; ++i) {
+    std::string aaaaa = std::string(220, 'a') + std::to_string(i);
+    Builder.insert(aaaaa);
+  }
+
+  std::string S = "foo.natvis";
+  uint32_t Pos = Builder.insert(S);
+
+  EXPECT_GT(Pos, 0xFFFFu);
+
+  StringTableHashTraits Traits(Builder);
+  EXPECT_LE(Traits.hashLookupKey(S), 0xFFFFu);
+}
Index: llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h
===================================================================
--- llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h
+++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h
@@ -143,7 +143,7 @@
       return EC;
     if (Present.intersects(Deleted))
       return make_error<RawError>(raw_error_code::corrupt_file,
-                                  "Present bit vector interesects deleted!");
+                                  "Present bit vector intersects deleted!");
 
     for (uint32_t P : Present) {
       if (auto EC = Stream.readInteger(Buckets[P].first))
Index: llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp
===================================================================
--- llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp
+++ llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp
@@ -34,6 +34,7 @@
   // Here, the type HASH is a typedef of unsigned short.
   // ** It is not a bug that we truncate the result of hashStringV1, in fact
   //    it is a bug if we do not! **
+  // See NMTNI::hash() in the reference implementation.
   return static_cast<uint16_t>(hashStringV1(S));
 }
 
Index: llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp
===================================================================
--- llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp
+++ llvm/trunk/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp
@@ -26,7 +26,13 @@
     : Table(&Table) {}
 
 uint32_t StringTableHashTraits::hashLookupKey(StringRef S) const {
-  return Table->getIdForString(S);
+  // The reference implementation doesn't include code for /src/headerblock
+  // handling, but it can only read natvis entries lld's PDB files if
+  // this hash function truncates the hash to 16 bit.
+  // PDB/include/misc.h in the reference implementation has a hashSz() function
+  // that returns an unsigned short, that seems what's being used for
+  // /src/headerblock.
+  return static_cast<uint16_t>(Table->getIdForString(S));
 }
 
 StringRef StringTableHashTraits::storageKeyToLookupKey(uint32_t Offset) const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61277.197213.patch
Type: text/x-patch
Size: 3351 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190429/8e38fed2/attachment.bin>