Commit e639eac2 authored by Giuseppe Ottaviano's avatar Giuseppe Ottaviano Committed by Facebook Github Bot

Fix a corner case in EliasFanoReader::previousValue

Summary:
`previousValue` makes the incorrect assumption that `outer_`
is a multiple of the word size. This is incorrect because `skipToNext`
can reposition at an arbitrary point.

The bug is very rare because it can only happen if there is a gap
larger than the skip quantum after the first element in the upper
bits.

Reviewed By: philippv

Differential Revision: D4607270

fbshipit-source-id: ff016f09f3f1f87314b68370e3dc137b82694f45
parent 3900132e
...@@ -473,8 +473,8 @@ class UpperBitsReader { ...@@ -473,8 +473,8 @@ class UpperBitsReader {
block &= (block_t(1) << inner_) - 1; block &= (block_t(1) << inner_) - 1;
while (UNLIKELY(block == 0)) { while (UNLIKELY(block == 0)) {
DCHECK_GE(outer, sizeof(block_t)); DCHECK_GT(outer, 0);
outer -= sizeof(block_t); outer -= std::min(sizeof(block_t), outer);
block = folly::loadUnaligned<block_t>(start_ + outer); block = folly::loadUnaligned<block_t>(start_ + outer);
} }
......
...@@ -138,6 +138,28 @@ TEST_F(EliasFanoCodingTest, Select64) { ...@@ -138,6 +138,28 @@ TEST_F(EliasFanoCodingTest, Select64) {
} }
} }
TEST_F(EliasFanoCodingTest, BugLargeGapInUpperBits) { // t16274876
typedef EliasFanoEncoderV2<uint32_t, uint32_t, 2, 2> Encoder;
typedef EliasFanoReader<Encoder, instructions::EF_TEST_ARCH> Reader;
constexpr uint32_t kLargeValue = 127;
// Build a list where the upper bits have a large gap after the
// first element, so that we need to reposition in the upper bits
// using skips to position the iterator on the second element.
std::vector<uint32_t> data = {0, kLargeValue};
for (uint32_t i = 0; i < kLargeValue; ++i) {
data.push_back(data.back() + 1);
}
auto list = Encoder::encode(data.begin(), data.end());
{
Reader reader(list);
ASSERT_TRUE(reader.skipTo(kLargeValue - 1));
ASSERT_EQ(kLargeValue, reader.value());
ASSERT_EQ(0, reader.previousValue());
}
}
namespace bm { namespace bm {
typedef EliasFanoEncoderV2<uint32_t, uint32_t, 128, 128> Encoder; typedef EliasFanoEncoderV2<uint32_t, uint32_t, 128, 128> Encoder;
......
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