Commit c37507e4 authored by Philip Pronin's avatar Philip Pronin Committed by Facebook GitHub Bot

fix overflow in EF::UpperBitsReader::reposition

Reviewed By: ot

Differential Revision: D23802800

fbshipit-source-id: 993060123790058b44d13c0b38eb4a04a28462a2
parent f8e41896
...@@ -77,7 +77,7 @@ struct EliasFanoCompressedListBase { ...@@ -77,7 +77,7 @@ struct EliasFanoCompressedListBase {
// WARNING: EliasFanoCompressedList has no ownership of data. The 7 bytes // WARNING: EliasFanoCompressedList has no ownership of data. The 7 bytes
// following the last byte should be readable if kUpperFirst = false, 8 bytes // following the last byte should be readable if kUpperFirst = false, 8 bytes
// otherwise. // otherwise.
folly::Range<Pointer> data; Range<Pointer> data;
Pointer skipPointers = nullptr; Pointer skipPointers = nullptr;
Pointer forwardPointers = nullptr; Pointer forwardPointers = nullptr;
...@@ -85,8 +85,8 @@ struct EliasFanoCompressedListBase { ...@@ -85,8 +85,8 @@ struct EliasFanoCompressedListBase {
Pointer upper = nullptr; Pointer upper = nullptr;
}; };
typedef EliasFanoCompressedListBase<const uint8_t*> EliasFanoCompressedList; using EliasFanoCompressedList = EliasFanoCompressedListBase<const uint8_t*>;
typedef EliasFanoCompressedListBase<uint8_t*> MutableEliasFanoCompressedList; using MutableEliasFanoCompressedList = EliasFanoCompressedListBase<uint8_t*>;
template < template <
class Value, class Value,
...@@ -97,14 +97,14 @@ template < ...@@ -97,14 +97,14 @@ template <
bool kUpperFirst = false> bool kUpperFirst = false>
struct EliasFanoEncoderV2 { struct EliasFanoEncoderV2 {
static_assert( static_assert(
std::is_integral<Value>::value && std::is_unsigned<Value>::value, std::is_integral_v<Value> && std::is_unsigned_v<Value>,
"Value should be unsigned integral"); "Value should be unsigned integral");
typedef EliasFanoCompressedList CompressedList; using CompressedList = EliasFanoCompressedList;
typedef MutableEliasFanoCompressedList MutableCompressedList; using MutableCompressedList = MutableEliasFanoCompressedList;
typedef Value ValueType; using ValueType = Value;
typedef SkipValue SkipValueType; using SkipValueType = SkipValue;
struct Layout; struct Layout;
static constexpr size_t skipQuantum = kSkipQuantum; static constexpr size_t skipQuantum = kSkipQuantum;
...@@ -119,7 +119,7 @@ struct EliasFanoEncoderV2 { ...@@ -119,7 +119,7 @@ struct EliasFanoEncoderV2 {
// "floor(a) - floor(b) - 1 <= floor(a - b) <= floor(a) - floor(b)". // "floor(a) - floor(b) - 1 <= floor(a - b) <= floor(a) - floor(b)".
// Assuming "candidate = floor(log(upperBound)) - floor(log(upperBound))", // Assuming "candidate = floor(log(upperBound)) - floor(log(upperBound))",
// then result is either "candidate - 1" or "candidate". // then result is either "candidate - 1" or "candidate".
auto candidate = folly::findLastSet(upperBound) - folly::findLastSet(size); auto candidate = findLastSet(upperBound) - findLastSet(size);
// NOTE: As size != 0, "candidate" is always < 64. // NOTE: As size != 0, "candidate" is always < 64.
return (size > (upperBound >> candidate)) ? candidate - 1 : candidate; return (size > (upperBound >> candidate)) ? candidate - 1 : candidate;
} }
...@@ -175,7 +175,7 @@ struct EliasFanoEncoderV2 { ...@@ -175,7 +175,7 @@ struct EliasFanoEncoderV2 {
fillSkipPointersUpTo(upperBits); fillSkipPointersUpTo(upperBits);
if /* constexpr */ (forwardQuantum != 0) { if constexpr (forwardQuantum != 0) {
if ((size_ + 1) % forwardQuantum == 0) { if ((size_ + 1) % forwardQuantum == 0) {
const auto k = size_ / forwardQuantum; const auto k = size_ / forwardQuantum;
// Store the number of preceding 0-bits. // Store the number of preceding 0-bits.
...@@ -200,7 +200,7 @@ struct EliasFanoEncoderV2 { ...@@ -200,7 +200,7 @@ struct EliasFanoEncoderV2 {
private: private:
void fillSkipPointersUpTo(ValueType fillBoundary) { void fillSkipPointersUpTo(ValueType fillBoundary) {
if /* constexpr */ (skipQuantum != 0) { if constexpr (skipQuantum != 0) {
while ((skipPointersSize_ + 1) * skipQuantum <= fillBoundary) { while ((skipPointersSize_ + 1) * skipQuantum <= fillBoundary) {
// Store the number of preceding 1-bits. // Store the number of preceding 1-bits.
skipPointers_[skipPointersSize_++] = static_cast<SkipValueType>(size_); skipPointers_[skipPointersSize_++] = static_cast<SkipValueType>(size_);
...@@ -213,9 +213,9 @@ struct EliasFanoEncoderV2 { ...@@ -213,9 +213,9 @@ struct EliasFanoEncoderV2 {
DCHECK_LE(uint32_t(len), 56); DCHECK_LE(uint32_t(len), 56);
DCHECK_EQ(0, value & ~((uint64_t(1) << len) - 1)); DCHECK_EQ(0, value & ~((uint64_t(1) << len) - 1));
unsigned char* const ptr = data + (pos / 8); unsigned char* const ptr = data + (pos / 8);
uint64_t ptrv = folly::loadUnaligned<uint64_t>(ptr); uint64_t ptrv = loadUnaligned<uint64_t>(ptr);
ptrv |= value << (pos % 8); ptrv |= value << (pos % 8);
folly::storeUnaligned<uint64_t>(ptr, ptrv); storeUnaligned<uint64_t>(ptr, ptrv);
} }
unsigned char* lower_ = nullptr; unsigned char* lower_ = nullptr;
...@@ -276,7 +276,7 @@ struct EliasFanoEncoderV2< ...@@ -276,7 +276,7 @@ struct EliasFanoEncoderV2<
// *** Skip pointers. // *** Skip pointers.
// Store (1-indexed) position of every skipQuantum-th // Store (1-indexed) position of every skipQuantum-th
// 0-bit in upper bits sequence. // 0-bit in upper bits sequence.
if /* constexpr */ (skipQuantum != 0) { if constexpr (skipQuantum != 0) {
// 8 * upper is used here instead of upperSizeBits, as that is // 8 * upper is used here instead of upperSizeBits, as that is
// more serialization-friendly way (upperSizeBits doesn't need // more serialization-friendly way (upperSizeBits doesn't need
// to be known by this function, unlike upper). // to be known by this function, unlike upper).
...@@ -288,7 +288,7 @@ struct EliasFanoEncoderV2< ...@@ -288,7 +288,7 @@ struct EliasFanoEncoderV2<
// *** Forward pointers. // *** Forward pointers.
// Store (1-indexed) position of every forwardQuantum-th // Store (1-indexed) position of every forwardQuantum-th
// 1-bit in upper bits sequence. // 1-bit in upper bits sequence.
if /* constexpr */ (forwardQuantum != 0) { if constexpr (forwardQuantum != 0) {
size_t numForwardPointers = size / forwardQuantum; size_t numForwardPointers = size / forwardQuantum;
layout.forwardPointers = numForwardPointers * sizeof(SkipValueType); layout.forwardPointers = numForwardPointers * sizeof(SkipValueType);
} }
...@@ -317,7 +317,7 @@ struct EliasFanoEncoderV2< ...@@ -317,7 +317,7 @@ struct EliasFanoEncoderV2<
result.skipPointers = advance(skipPointers); result.skipPointers = advance(skipPointers);
result.forwardPointers = advance(forwardPointers); result.forwardPointers = advance(forwardPointers);
if /* constexpr */ (kUpperFirst) { if constexpr (kUpperFirst) {
result.upper = advance(upper); result.upper = advance(upper);
result.lower = advance(lower); result.lower = advance(lower);
} else { } else {
...@@ -338,7 +338,7 @@ struct EliasFanoEncoderV2< ...@@ -338,7 +338,7 @@ struct EliasFanoEncoderV2<
if (size > 0) { if (size > 0) {
buf = static_cast<uint8_t*>(malloc(bytes() + 8)); buf = static_cast<uint8_t*>(malloc(bytes() + 8));
} }
folly::MutableByteRange bufRange(buf, bytes()); MutableByteRange bufRange(buf, bytes());
return openList(bufRange); return openList(bufRange);
} }
...@@ -357,10 +357,10 @@ namespace detail { ...@@ -357,10 +357,10 @@ namespace detail {
template <class Encoder, class Instructions, class SizeType> template <class Encoder, class Instructions, class SizeType>
class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
SkipPointers<Encoder::skipQuantum> { SkipPointers<Encoder::skipQuantum> {
typedef typename Encoder::SkipValueType SkipValueType; using SkipValueType = typename Encoder::SkipValueType;
public: public:
typedef typename Encoder::ValueType ValueType; using ValueType = typename Encoder::ValueType;
explicit UpperBitsReader(const typename Encoder::CompressedList& list) explicit UpperBitsReader(const typename Encoder::CompressedList& list)
: ForwardPointers<Encoder::forwardQuantum>(list.forwardPointers), : ForwardPointers<Encoder::forwardQuantum>(list.forwardPointers),
...@@ -380,6 +380,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -380,6 +380,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
FOLLY_ALWAYS_INLINE SizeType position() const { FOLLY_ALWAYS_INLINE SizeType position() const {
return position_; return position_;
} }
FOLLY_ALWAYS_INLINE ValueType value() const { FOLLY_ALWAYS_INLINE ValueType value() const {
return value_; return value_;
} }
...@@ -388,7 +389,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -388,7 +389,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
size_t inner; size_t inner;
block_t block; block_t block;
getPreviousInfo(block, inner, outer_); getPreviousInfo(block, inner, outer_);
block_ = folly::loadUnaligned<block_t>(start_ + outer_); block_ = loadUnaligned<block_t>(start_ + outer_);
block_ ^= block; block_ ^= block;
--position_; --position_;
return setValue(inner); return setValue(inner);
...@@ -398,7 +399,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -398,7 +399,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
// Skip to the first non-zero block. // Skip to the first non-zero block.
while (block_ == 0) { while (block_ == 0) {
outer_ += sizeof(block_t); outer_ += sizeof(block_t);
block_ = folly::loadUnaligned<block_t>(start_ + outer_); block_ = loadUnaligned<block_t>(start_ + outer_);
} }
++position_; ++position_;
...@@ -416,7 +417,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -416,7 +417,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
// Use forward pointer. // Use forward pointer.
if (Encoder::forwardQuantum > 0 && UNLIKELY(n > Encoder::forwardQuantum)) { if (Encoder::forwardQuantum > 0 && UNLIKELY(n > Encoder::forwardQuantum)) {
const size_t steps = position_ / Encoder::forwardQuantum; const size_t steps = position_ / Encoder::forwardQuantum;
const size_t dest = folly::loadUnaligned<SkipValueType>( const size_t dest = loadUnaligned<SkipValueType>(
this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType)); this->forwardPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest + steps * Encoder::forwardQuantum); reposition(dest + steps * Encoder::forwardQuantum);
...@@ -428,7 +429,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -428,7 +429,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
while ((cnt = Instructions::popcount(block_)) < n) { while ((cnt = Instructions::popcount(block_)) < n) {
n -= cnt; n -= cnt;
outer_ += sizeof(block_t); outer_ += sizeof(block_t);
block_ = folly::loadUnaligned<block_t>(start_ + outer_); block_ = loadUnaligned<block_t>(start_ + outer_);
} }
// Skip to the n-th one in the block. // Skip to the n-th one in the block.
...@@ -448,7 +449,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -448,7 +449,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
if (Encoder::skipQuantum > 0 && if (Encoder::skipQuantum > 0 &&
UNLIKELY(v >= value_ + Encoder::skipQuantum)) { UNLIKELY(v >= value_ + Encoder::skipQuantum)) {
const size_t steps = v / Encoder::skipQuantum; const size_t steps = v / Encoder::skipQuantum;
const size_t dest = folly::loadUnaligned<SkipValueType>( const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType)); this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
reposition(dest + Encoder::skipQuantum * steps); reposition(dest + Encoder::skipQuantum * steps);
...@@ -470,7 +471,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -470,7 +471,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
skip -= cnt; skip -= cnt;
position_ += kBitsPerBlock - cnt; position_ += kBitsPerBlock - cnt;
outer_ += sizeof(block_t); outer_ += sizeof(block_t);
block_ = folly::loadUnaligned<block_t>(start_ + outer_); block_ = loadUnaligned<block_t>(start_ + outer_);
} }
if (LIKELY(skip)) { if (LIKELY(skip)) {
...@@ -495,7 +496,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -495,7 +496,7 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
if (Encoder::skipQuantum > 0 && v >= value_ + Encoder::skipQuantum) { if (Encoder::skipQuantum > 0 && v >= value_ + Encoder::skipQuantum) {
auto outer = outer_; auto outer = outer_;
const size_t steps = v / Encoder::skipQuantum; const size_t steps = v / Encoder::skipQuantum;
const size_t dest = folly::loadUnaligned<SkipValueType>( const size_t dest = loadUnaligned<SkipValueType>(
this->skipPointers_ + (steps - 1) * sizeof(SkipValueType)); this->skipPointers_ + (steps - 1) * sizeof(SkipValueType));
position = dest - 1; position = dest - 1;
...@@ -539,35 +540,37 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>, ...@@ -539,35 +540,37 @@ class UpperBitsReader : ForwardPointers<Encoder::forwardQuantum>,
} }
private: private:
using block_t = uint64_t;
// The size in bytes of the upper bits is limited by n + universe / 8,
// so a type that can hold either sizes or values is sufficient.
using OuterType = typename std::common_type_t<ValueType, SizeType>;
FOLLY_ALWAYS_INLINE ValueType setValue(size_t inner) { FOLLY_ALWAYS_INLINE ValueType setValue(size_t inner) {
value_ = static_cast<ValueType>(8 * outer_ + inner - position_); value_ = static_cast<ValueType>(8 * outer_ + inner - position_);
return value_; return value_;
} }
FOLLY_ALWAYS_INLINE void reposition(SizeType dest) { // NOTE: dest is a position in the bit vector, use size_t as SizeType may
// not be sufficient here.
FOLLY_ALWAYS_INLINE void reposition(size_t dest) {
outer_ = dest / 8; outer_ = dest / 8;
block_ = folly::loadUnaligned<block_t>(start_ + outer_); block_ = loadUnaligned<block_t>(start_ + outer_);
block_ &= ~((block_t(1) << (dest % 8)) - 1); block_ &= ~((block_t(1) << (dest % 8)) - 1);
} }
using block_t = uint64_t;
// The size in bytes of the upper bits is limited by n + universe / 8,
// so a type that can hold either sizes or values is sufficient.
using OuterType = typename std::common_type<ValueType, SizeType>::type;
FOLLY_ALWAYS_INLINE void FOLLY_ALWAYS_INLINE void
getPreviousInfo(block_t& block, size_t& inner, OuterType& outer) const { getPreviousInfo(block_t& block, size_t& inner, OuterType& outer) const {
DCHECK_NE(position(), std::numeric_limits<SizeType>::max()); DCHECK_NE(position(), std::numeric_limits<SizeType>::max());
DCHECK_GT(position(), 0); DCHECK_GT(position(), 0);
outer = outer_; outer = outer_;
block = folly::loadUnaligned<block_t>(start_ + outer); block = loadUnaligned<block_t>(start_ + outer);
inner = size_t(value_) - 8 * outer_ + position_; inner = size_t(value_) - 8 * outer_ + position_;
block &= (block_t(1) << inner) - 1; block &= (block_t(1) << inner) - 1;
while (UNLIKELY(block == 0)) { while (UNLIKELY(block == 0)) {
DCHECK_GT(outer, 0); DCHECK_GT(outer, 0);
outer -= std::min<OuterType>(sizeof(block_t), outer); outer -= std::min<OuterType>(sizeof(block_t), outer);
block = folly::loadUnaligned<block_t>(start_ + outer); block = loadUnaligned<block_t>(start_ + outer);
} }
inner = 8 * sizeof(block_t) - 1 - Instructions::clz(block); inner = 8 * sizeof(block_t) - 1 - Instructions::clz(block);
} }
...@@ -594,8 +597,8 @@ template < ...@@ -594,8 +597,8 @@ template <
class SizeType = typename Encoder::SkipValueType> class SizeType = typename Encoder::SkipValueType>
class EliasFanoReader { class EliasFanoReader {
public: public:
typedef Encoder EncoderType; using EncoderType = Encoder;
typedef typename Encoder::ValueType ValueType; using ValueType = typename Encoder::ValueType;
explicit EliasFanoReader(const typename Encoder::CompressedList& list) explicit EliasFanoReader(const typename Encoder::CompressedList& list)
: upper_(list), : upper_(list),
...@@ -614,7 +617,7 @@ class EliasFanoReader { ...@@ -614,7 +617,7 @@ class EliasFanoReader {
ValueType lastUpperValue = ValueType(8 * list.upperSizeBytes - size_); ValueType lastUpperValue = ValueType(8 * list.upperSizeBytes - size_);
auto it = list.upper + list.upperSizeBytes - 1; auto it = list.upper + list.upperSizeBytes - 1;
DCHECK_NE(*it, 0); DCHECK_NE(*it, 0);
lastUpperValue -= 8 - folly::findLastSet(*it); lastUpperValue -= 8 - findLastSet(*it);
lastValue_ = readLowerPart(size_ - 1) | (lastUpperValue << numLowerBits_); lastValue_ = readLowerPart(size_ - 1) | (lastUpperValue << numLowerBits_);
} }
...@@ -735,7 +738,8 @@ class EliasFanoReader { ...@@ -735,7 +738,8 @@ class EliasFanoReader {
return true; return true;
} }
// We might be in the middle of a run, iterate backwards to the beginning. // We might be in the middle of a run of equal values, reposition by
// iterating backwards to its first element.
auto valueLower = Instructions::bzhi(value_, numLowerBits_); auto valueLower = Instructions::bzhi(value_, numLowerBits_);
while (!upper_.isAtBeginningOfRun() && while (!upper_.isAtBeginningOfRun() &&
readLowerPart(upper_.position() - 1) == valueLower) { readLowerPart(upper_.position() - 1) == valueLower) {
...@@ -776,6 +780,7 @@ class EliasFanoReader { ...@@ -776,6 +780,7 @@ class EliasFanoReader {
SizeType position() const { SizeType position() const {
return upper_.position(); return upper_.position();
} }
ValueType value() const { ValueType value() const {
DCHECK(valid()); DCHECK(valid());
return value_; return value_;
...@@ -795,7 +800,7 @@ class EliasFanoReader { ...@@ -795,7 +800,7 @@ class EliasFanoReader {
DCHECK_LT(i, size_); DCHECK_LT(i, size_);
const size_t pos = i * numLowerBits_; const size_t pos = i * numLowerBits_;
const unsigned char* ptr = lower_ + (pos / 8); const unsigned char* ptr = lower_ + (pos / 8);
const uint64_t ptrv = folly::loadUnaligned<uint64_t>(ptr); const uint64_t ptrv = loadUnaligned<uint64_t>(ptr);
// This removes the branch in the fallback implementation of // This removes the branch in the fallback implementation of
// bzhi. The condition is verified at encoding time. // bzhi. The condition is verified at encoding time.
assume(numLowerBits_ < sizeof(ValueType) * 8); assume(numLowerBits_ < sizeof(ValueType) * 8);
......
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