diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-01 10:26:22 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-01 10:26:22 +0100 |
commit | 487837fa4bb82d67829dae25c86ab4bb6bc9e41c (patch) | |
tree | be5a34613905f9945e204d375e1c60c60215e3d6 | |
parent | cc1bbb0fc5d775d7023bea637739a73d6973047c (diff) | |
parent | 5db1c3a16cf17ad26b059c5a9ba5a14d9ee8a5e4 (diff) |
Merge pull request #25053 from vespa-engine/toregge/add-range-checks-for-bitvector-setbit-and-clearbit
Add range checks for BitVector setBits and clearBits member functions,
5 files changed, 61 insertions, 11 deletions
diff --git a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp index 33f044845aa..2dd402a66f7 100644 --- a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp @@ -69,7 +69,7 @@ AllocatedBitVector::AllocatedBitVector(Index numberOfElements, Index capacityBit if (minCount/8 == numberOfElements/8) { static_cast<Word *>(getStart())[numWords()-1] &= ~endBits(minCount); } - setBit(size()); // Guard bit + set_bit_no_range_check(size()); // Guard bit } updateCount(); } @@ -90,7 +90,7 @@ AllocatedBitVector::AllocatedBitVector(const BitVector & rhs, std::pair<Index, I _capacityBits = computeCapacity(_capacityBits, _alloc.size()); memcpy(_alloc.get(), rhs.getStart(), numBytes(size_capacity.first - rhs.getStartIndex())); init(_alloc.get(), 0, size_capacity.first); - setBit(size()); + set_bit_no_range_check(size()); updateCount(); } diff --git a/searchlib/src/vespa/searchlib/common/bitvector.cpp b/searchlib/src/vespa/searchlib/common/bitvector.cpp index 03fe97cabbb..47ca590c9bc 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvector.cpp @@ -10,6 +10,7 @@ #include <vespa/vespalib/objects/nbostream.h> #include <vespa/fastos/file.h> #include <cassert> +#include <cstdlib> #include <vespa/log/log.h> LOG_SETUP(".searchlib.common.bitvector"); @@ -42,6 +43,8 @@ namespace search { using vespalib::nbostream; +bool BitVector::_enable_range_check = false; + Alloc BitVector::allocatePaddedAndAligned(Index start, Index end, Index capacity, const Alloc* init_alloc) { @@ -79,7 +82,7 @@ void BitVector::clear() { memset(getActiveStart(), '\0', getActiveBytes()); - setBit(size()); // Guard bit + set_bit_no_range_check(size()); // Guard bit setTrueBits(0); } @@ -369,6 +372,15 @@ BitVector::create(const BitVector & rhs) return std::make_unique<AllocatedBitVector>(rhs); } +void +BitVector::consider_enable_range_check() +{ + const char *env = getenv("VESPA_BITVECTOR_RANGE_CHECK"); + if (env != nullptr && strcmp(env, "true") == 0) { + _enable_range_check = true; + } +} + MMappedBitVector::MMappedBitVector(Index numberOfElements, FastOS_FileInterface &file, int64_t offset, Index doccount) : BitVector() @@ -422,5 +434,17 @@ operator>>(nbostream &in, AllocatedBitVector &bv) return in; } +class ConsiderEnableRangeCheckCaller +{ +public: + ConsiderEnableRangeCheckCaller(); +}; + +ConsiderEnableRangeCheckCaller::ConsiderEnableRangeCheckCaller() +{ + BitVector::consider_enable_range_check(); +} + +ConsiderEnableRangeCheckCaller consider_enable_range_check_caller; } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/bitvector.h b/searchlib/src/vespa/searchlib/common/bitvector.h index d3671d61ccf..1d289435f6d 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.h +++ b/searchlib/src/vespa/searchlib/common/bitvector.h @@ -8,6 +8,7 @@ #include <vespa/vespalib/util/atomic.h> #include <vespa/fastos/types.h> #include <algorithm> +#include <cassert> namespace vespalib { class nbostream; @@ -134,23 +135,38 @@ public: } void setSize(Index sz) { - setBit(sz); // Need to place the new stop sign first + set_bit_no_range_check(sz); // Need to place the new stop sign first std::atomic_thread_fence(std::memory_order_release); if (sz > _sz) { // Can only remove the old stopsign if it is ahead of the new. - clearBit(_sz); + clear_bit_no_range_check(_sz); } vespalib::atomic::store_ref_release(_sz, sz); } - void setBit(Index idx) { + void set_bit_no_range_check(Index idx) { store(_words[wordNum(idx)], _words[wordNum(idx)] | mask(idx)); } - void clearBit(Index idx) { + void clear_bit_no_range_check(Index idx) { store(_words[wordNum(idx)], _words[wordNum(idx)] & ~ mask(idx)); } - void flipBit(Index idx) { + void flip_bit_no_range_check(Index idx) { store(_words[wordNum(idx)], _words[wordNum(idx)] ^ mask(idx)); } + void range_check(Index idx) { + assert(!_enable_range_check || (idx >= _startOffset && idx < _sz)); + } + void setBit(Index idx) { + range_check(idx); + set_bit_no_range_check(idx); + } + void clearBit(Index idx) { + range_check(idx); + clear_bit_no_range_check(idx); + } + void flipBit(Index idx) { + range_check(idx); + flip_bit_no_range_check(idx); + } void andWith(const BitVector &right); void orWith(const BitVector &right); @@ -187,6 +203,14 @@ public: incNumBits(); } } + + void clear_bit_and_maintain_count_no_range_check(Index idx) { + if (testBit(idx)) { + clear_bit_no_range_check(idx); + decNumBits(); + } + } + /** * Clears a bit and maintains count of number of bits set. * @param idx @@ -251,6 +275,7 @@ public: static UP create(const BitVector & org, Index start, Index end); static UP create(Index numberOfElements); static UP create(const BitVector & rhs); + static void consider_enable_range_check(); protected: using Alloc = vespalib::alloc::Alloc; VESPA_DLL_LOCAL BitVector(void * buf, Index start, Index end); @@ -291,7 +316,7 @@ private: return (end >= start) ? (numWords(end) - wordNum(start)) : 0; } static Index invalidCount() { return std::numeric_limits<Index>::max(); } - void setGuardBit() { setBit(size()); } + void setGuardBit() { set_bit_no_range_check(size()); } void incNumBits() { if ( isValidCount() ) { _numTrueBits.store(_numTrueBits.load(std::memory_order_relaxed) + 1, std::memory_order_relaxed); @@ -360,6 +385,7 @@ private: Index _startOffset; // This is the official start Index _sz; // This is the official end. mutable std::atomic<Index> _numTrueBits; + static bool _enable_range_check; protected: friend vespalib::nbostream & diff --git a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp index 5f971e21cd3..b0f8ff365ca 100644 --- a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp @@ -28,7 +28,7 @@ GrowableBitVector::grow(BitWord::Index newSize, BitWord::Index newCapacity) if (newCapacity != self.capacity()) { auto tbv = std::make_unique<AllocatedBitVector>(newSize, newCapacity, self._alloc.get(), self.size(), &self._alloc); if (newSize > self.size()) { - tbv->clearBitAndMaintainCount(self.size()); // Clear old guard bit. + tbv->clear_bit_and_maintain_count_no_range_check(self.size()); // Clear old guard bit. } auto to_hold = std::make_unique<GenerationHeldAllocatedBitVector>(std::move(_stored)); _self.store(tbv.get(), std::memory_order_release); diff --git a/searchlib/src/vespa/searchlib/common/partialbitvector.cpp b/searchlib/src/vespa/searchlib/common/partialbitvector.cpp index 128a9ab8452..af4e2efd315 100644 --- a/searchlib/src/vespa/searchlib/common/partialbitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/partialbitvector.cpp @@ -26,7 +26,7 @@ PartialBitVector::PartialBitVector(const BitVector & org, Index start, Index end if (org.size() < end) { clearInterval(org.size(), end); } - setBit(size()); + set_bit_no_range_check(size()); } PartialBitVector::~PartialBitVector() = default; |