Commit 46c141a8 authored by Edwin Smith's avatar Edwin Smith Committed by Facebook Github Bot

Fix visitContiguousRanges() for empty and single chunk containers

Summary:
lastOccupiedChunks has UB with an empty container, because
the end() iterator itemPtr_ is null.

And with a single chunk, we must enter the iteration loop.

Reviewed By: nbronson

Differential Revision: D8897447

fbshipit-source-id: 7e7fd06ec7a588236eebf28f43af9ef4ab2d39cd
parent 7bd9849f
...@@ -1557,6 +1557,7 @@ class F14Table : public Policy { ...@@ -1557,6 +1557,7 @@ class F14Table : public Policy {
} }
ChunkPtr lastOccupiedChunk() const { ChunkPtr lastOccupiedChunk() const {
FOLLY_SAFE_DCHECK(size() > 0, "");
if (Policy::kEnableItemIteration) { if (Policy::kEnableItemIteration) {
return begin().chunk(); return begin().chunk();
} else { } else {
...@@ -2139,9 +2140,12 @@ class F14Table : public Policy { ...@@ -2139,9 +2140,12 @@ class F14Table : public Policy {
// visitor should take an Item const& // visitor should take an Item const&
template <typename V> template <typename V>
void visitItems(V&& visitor) const { void visitItems(V&& visitor) const {
if (empty()) {
return;
}
std::size_t maxChunkIndex = lastOccupiedChunk() - chunks_; std::size_t maxChunkIndex = lastOccupiedChunk() - chunks_;
auto chunk = &chunks_[0]; auto chunk = &chunks_[0];
for (std::size_t i = 0; i < maxChunkIndex; ++i, ++chunk) { for (std::size_t i = 0; i <= maxChunkIndex; ++i, ++chunk) {
auto iter = chunk->occupiedIter(); auto iter = chunk->occupiedIter();
if (Policy::prefetchBeforeCopy()) { if (Policy::prefetchBeforeCopy()) {
for (auto piter = iter; piter.hasNext();) { for (auto piter = iter; piter.hasNext();) {
...@@ -2157,9 +2161,12 @@ class F14Table : public Policy { ...@@ -2157,9 +2161,12 @@ class F14Table : public Policy {
// visitor should take two Item const* // visitor should take two Item const*
template <typename V> template <typename V>
void visitContiguousItemRanges(V&& visitor) const { void visitContiguousItemRanges(V&& visitor) const {
if (empty()) {
return;
}
std::size_t maxChunkIndex = lastOccupiedChunk() - chunks_; std::size_t maxChunkIndex = lastOccupiedChunk() - chunks_;
auto chunk = &chunks_[0]; auto chunk = &chunks_[0];
for (std::size_t i = 0; i < maxChunkIndex; ++i, ++chunk) { for (std::size_t i = 0; i <= maxChunkIndex; ++i, ++chunk) {
for (auto iter = chunk->occupiedRangeIter(); iter.hasNext();) { for (auto iter = chunk->occupiedRangeIter(); iter.hasNext();) {
auto be = iter.next(); auto be = iter.next();
FOLLY_SAFE_DCHECK( FOLLY_SAFE_DCHECK(
......
...@@ -107,10 +107,10 @@ TEST(F14Map, getAllocatedMemorySize) { ...@@ -107,10 +107,10 @@ TEST(F14Map, getAllocatedMemorySize) {
} }
template <typename M> template <typename M>
void runVisitContiguousRangesTest() { void runVisitContiguousRangesTest(int n) {
M map; M map;
for (int i = 0; i < 1000; ++i) { for (int i = 0; i < n; ++i) {
map[i] = i; map[i] = i;
map.erase(i / 2); map.erase(i / 2);
} }
...@@ -128,6 +128,18 @@ void runVisitContiguousRangesTest() { ...@@ -128,6 +128,18 @@ void runVisitContiguousRangesTest() {
iter->second = true; iter->second = true;
} }
}); });
// ensure no entries were skipped
for (auto& e : visited) {
EXPECT_TRUE(e.second);
}
}
template <typename M>
void runVisitContiguousRangesTest() {
runVisitContiguousRangesTest<M>(0); // empty
runVisitContiguousRangesTest<M>(5); // single chunk
runVisitContiguousRangesTest<M>(1000); // many chunks
} }
TEST(F14ValueMap, visitContiguousRanges) { TEST(F14ValueMap, visitContiguousRanges) {
......
...@@ -130,10 +130,10 @@ TEST(F14Set, getAllocatedMemorySize) { ...@@ -130,10 +130,10 @@ TEST(F14Set, getAllocatedMemorySize) {
} }
template <typename S> template <typename S>
void runVisitContiguousRangesTest() { void runVisitContiguousRangesTest(int n) {
S set; S set;
for (int i = 0; i < 1000; ++i) { for (int i = 0; i < n; ++i) {
set.insert(i); set.insert(i);
set.erase(i / 2); set.erase(i / 2);
} }
...@@ -151,6 +151,18 @@ void runVisitContiguousRangesTest() { ...@@ -151,6 +151,18 @@ void runVisitContiguousRangesTest() {
iter->second = true; iter->second = true;
} }
}); });
// ensure no entries were skipped
for (auto& e : visited) {
EXPECT_TRUE(e.second);
}
}
template <typename S>
void runVisitContiguousRangesTest() {
runVisitContiguousRangesTest<S>(0); // empty
runVisitContiguousRangesTest<S>(5); // single chunk
runVisitContiguousRangesTest<S>(1000); // many chunks
} }
TEST(F14ValueSet, visitContiguousRanges) { TEST(F14ValueSet, visitContiguousRanges) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment