llvm.org GIT mirror llvm / 1ea77ad
[PDB] Don't ignore bucket 0 when writing the PDB string table. The hash table is a list of buckets, and the *value* stored in the bucket cannot be 0 since that is reserved. However, the code here was incorrectly skipping over the 0'th bucket entirely. The 0'th bucket is perfectly fine, just none of these buckets can contain the value 0. As a result, whenever there was a string where hash(S) % Size was equal to 0, we would write the value in the next bucket instead. We never caught this in our tests due to *another* bug, which is that we would iterate the entire list of buckets looking for the value, only using the hash value as a starting point. However, the real algorithm stops when it finds 0 in a bucket since it takes that to mean "the item is not in the hash table". The unit test is updated to carefully construct a set of hash values that will cause one item to hash to 0 mod bucket count, and the reader is also updated to return an error indicating that the item is not found when it encounters a 0 bucket. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@328162 91177308-0d34-0410-b5e6-96231b3b80d8 Zachary Turner 1 year, 7 months ago
3 changed file(s) with 40 addition(s) and 13 deletion(s). Raw diff Collapse all Expand all
121121 // we iterate the entire array.
122122 uint32_t Index = (Start + I) % Count;
123123
124 // If we find 0, it means the item isn't in the hash table.
124125 uint32_t ID = IDs[Index];
126 if (ID == 0)
127 return make_error(raw_error_code::no_entry);
125128 auto ExpectedStr = getStringForID(ID);
126129 if (!ExpectedStr)
127130 return ExpectedStr.takeError();
8888
8989 for (uint32_t I = 0; I != BucketCount; ++I) {
9090 uint32_t Slot = (Hash + I) % BucketCount;
91 if (Slot == 0)
92 continue; // Skip reserved slot
9391 if (Buckets[Slot] != 0)
9492 continue;
9593 Buckets[Slot] = Offset;
2626 TEST_F(StringTableBuilderTest, Simple) {
2727 // Create /names table contents.
2828 PDBStringTableBuilder Builder;
29 EXPECT_EQ(1U, Builder.insert("foo"));
30 EXPECT_EQ(5U, Builder.insert("bar"));
31 EXPECT_EQ(1U, Builder.insert("foo"));
32 EXPECT_EQ(9U, Builder.insert("baz"));
29
30 // This test case is carefully constructed to ensure that at least one
31 // string gets bucketed into slot 0, *and* to ensure that at least one
32 // has a hash collision at the end of the bucket list so it has to
33 // wrap around.
34 uint32_t FooID = Builder.insert("foo");
35 uint32_t BarID = Builder.insert("bar");
36 uint32_t BazID = Builder.insert("baz");
37 uint32_t BuzzID = Builder.insert("buzz");
38 uint32_t BazzID = Builder.insert("bazz");
39 uint32_t BarrID = Builder.insert("barr");
40
41 // Re-inserting the same item should return the same id.
42 EXPECT_EQ(FooID, Builder.insert("foo"));
43 EXPECT_EQ(BarID, Builder.insert("bar"));
44 EXPECT_EQ(BazID, Builder.insert("baz"));
45 EXPECT_EQ(BuzzID, Builder.insert("buzz"));
46 EXPECT_EQ(BazzID, Builder.insert("bazz"));
47 EXPECT_EQ(BarrID, Builder.insert("barr"));
48
49 // Each ID should be distinct.
50 std::set Distinct{FooID, BarID, BazID, BuzzID, BazzID, BarrID};
51 EXPECT_EQ(6U, Distinct.size());
3352
3453 std::vector Buffer(Builder.calculateSerializedSize());
3554 MutableBinaryByteStream OutStream(Buffer, little);
4261 PDBStringTable Table;
4362 EXPECT_THAT_ERROR(Table.reload(Reader), Succeeded());
4463
45 EXPECT_EQ(3U, Table.getNameCount());
64 EXPECT_EQ(6U, Table.getNameCount());
4665 EXPECT_EQ(1U, Table.getHashVersion());
4766
48 EXPECT_THAT_EXPECTED(Table.getStringForID(1), HasValue("foo"));
49 EXPECT_THAT_EXPECTED(Table.getStringForID(5), HasValue("bar"));
50 EXPECT_THAT_EXPECTED(Table.getStringForID(9), HasValue("baz"));
51 EXPECT_THAT_EXPECTED(Table.getIDForString("foo"), HasValue(1U));
52 EXPECT_THAT_EXPECTED(Table.getIDForString("bar"), HasValue(5U));
53 EXPECT_THAT_EXPECTED(Table.getIDForString("baz"), HasValue(9U));
67 EXPECT_THAT_EXPECTED(Table.getStringForID(FooID), HasValue("foo"));
68 EXPECT_THAT_EXPECTED(Table.getStringForID(BarID), HasValue("bar"));
69 EXPECT_THAT_EXPECTED(Table.getStringForID(BazID), HasValue("baz"));
70 EXPECT_THAT_EXPECTED(Table.getStringForID(BuzzID), HasValue("buzz"));
71 EXPECT_THAT_EXPECTED(Table.getStringForID(BazzID), HasValue("bazz"));
72 EXPECT_THAT_EXPECTED(Table.getStringForID(BarrID), HasValue("barr"));
73
74 EXPECT_THAT_EXPECTED(Table.getIDForString("foo"), HasValue(FooID));
75 EXPECT_THAT_EXPECTED(Table.getIDForString("bar"), HasValue(BarID));
76 EXPECT_THAT_EXPECTED(Table.getIDForString("baz"), HasValue(BazID));
77 EXPECT_THAT_EXPECTED(Table.getIDForString("buzz"), HasValue(BuzzID));
78 EXPECT_THAT_EXPECTED(Table.getIDForString("bazz"), HasValue(BazzID));
79 EXPECT_THAT_EXPECTED(Table.getIDForString("barr"), HasValue(BarrID));
5480 }