Skip to content

Commit

Permalink
Fixed bug in SmallDenseMap where it wouldn't leave enough space for a…
Browse files Browse the repository at this point in the history
…n empty bucket if the number of values was exactly equal to the small capacity. This led to an infinite loop when finding a non-existent element

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@166492 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
cooperp committed Oct 23, 2012
1 parent 6457001 commit fbaf206
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
4 changes: 2 additions & 2 deletions include/llvm/ADT/DenseMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -826,11 +826,11 @@ class SmallDenseMap
}

void grow(unsigned AtLeast) {
if (AtLeast > InlineBuckets)
if (AtLeast >= InlineBuckets)
AtLeast = std::max<unsigned>(64, NextPowerOf2(AtLeast));

if (Small) {
if (AtLeast <= InlineBuckets)
if (AtLeast < InlineBuckets)
return; // Nothing to do.

// First move the inline buckets into a temporary storage.
Expand Down
33 changes: 33 additions & 0 deletions unittests/ADT/DenseMapTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,4 +330,37 @@ TEST(DenseMapCustomTest, FindAsTest) {
EXPECT_TRUE(map.find_as("d") == map.end());
}

struct ContiguousDenseMapInfo {
static inline unsigned getEmptyKey() { return ~0; }
static inline unsigned getTombstoneKey() { return ~0U - 1; }
static unsigned getHashValue(const unsigned& Val) { return Val; }
static bool isEqual(const unsigned& LHS, const unsigned& RHS) {
return LHS == RHS;
}
};

// Test that filling a small dense map with exactly the number of elements in
// the map grows to have enough space for an empty bucket.
TEST(DenseMapCustomTest, SmallDenseMapGrowTest) {
SmallDenseMap<unsigned, unsigned, 32, ContiguousDenseMapInfo> map;
// Add some number of elements, then delete a few to leave us some tombstones.
// If we just filled the map with 32 elements we'd grow because of not enough
// tombstones which masks the issue here.
for (unsigned i = 0; i < 20; ++i)
map[i] = i + 1;
for (unsigned i = 0; i < 10; ++i)
map.erase(i);
for (unsigned i = 20; i < 32; ++i)
map[i] = i + 1;

// Size tests
EXPECT_EQ(22u, map.size());

// Try to find an element which doesn't exist. There was a bug in
// SmallDenseMap which led to a map with num elements == small capacity not
// having an empty bucket any more. Finding an element not in the map would
// therefore never terminate.
EXPECT_TRUE(map.find(32) == map.end());
}

}

0 comments on commit fbaf206

Please sign in to comment.