From 74a456f904c5eb6d3eca10ac8dad795fd035ccbc Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Wed, 1 Jun 2022 08:30:44 +0000 Subject: fix undefined behavior in unit tests -- WIP --- searchlib/src/tests/sortspec/multilevelsort.cpp | 2 +- searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp | 4 +++- .../src/vespa/searchlib/attribute/multi_value_mapping.hpp | 11 ++--------- searchlib/src/vespa/searchlib/attribute/postingchange.cpp | 2 +- .../src/vespa/searchlib/attribute/postinglistattribute.cpp | 8 ++++---- .../src/vespa/searchlib/features/rankingexpressionfeature.cpp | 2 +- vespalib/src/vespa/vespalib/datastore/array_store.h | 6 ++++++ vespalib/src/vespa/vespalib/datastore/datastore.h | 2 +- vespalib/src/vespa/vespalib/datastore/datastorebase.h | 5 +++++ 9 files changed, 24 insertions(+), 18 deletions(-) diff --git a/searchlib/src/tests/sortspec/multilevelsort.cpp b/searchlib/src/tests/sortspec/multilevelsort.cpp index 18c1ef9a615..005fe6bd9d5 100644 --- a/searchlib/src/tests/sortspec/multilevelsort.cpp +++ b/searchlib/src/tests/sortspec/multilevelsort.cpp @@ -56,7 +56,7 @@ private: T getRandomValue() { T min = std::numeric_limits::min(); T max = std::numeric_limits::max(); - return min + static_cast((max - min) * (((float)rand() / (float)RAND_MAX))); + return min + static_cast(double(max - min) * (((float)rand() / (float)RAND_MAX))); } template void fill(IntegerAttribute *attr, uint32_t size, uint32_t unique = 0); diff --git a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp index 6054d473c1f..3d1127e6bc4 100644 --- a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp +++ b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp @@ -168,7 +168,9 @@ public: EXPECT_EQ(rv.size(), 3); EXPECT_LE(rv[0].distance, rv[1].distance); double thr = (rv[0].distance + rv[1].distance) * 0.5; - auto got_by_docid = index->find_top_k_with_filter(k, qv, *global_filter, k, thr); + auto got_by_docid = (global_filter) + ? index->find_top_k_with_filter(k, qv, *global_filter, k, thr) + : index->find_top_k(k, qv, k, thr); EXPECT_EQ(got_by_docid.size(), 1); EXPECT_EQ(got_by_docid[0].docid, rv[0].docid); for (const auto & hit : got_by_docid) { diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp index d5b50b591df..7ad00587640 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp @@ -11,15 +11,8 @@ template MultiValueMapping::MultiValueMapping(const vespalib::datastore::ArrayStoreConfig &storeCfg, const vespalib::GrowStrategy &gs, std::shared_ptr memory_allocator) -#ifdef __clang__ -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wuninitialized" -#endif - : MultiValueMappingBase(gs, _store.getGenerationHolder(), memory_allocator), -#ifdef __clang__ -#pragma clang diagnostic pop -#endif - _store(storeCfg, std::move(memory_allocator)) + : MultiValueMappingBase(gs, ArrayStore::getGenerationHolderLocation(_store), memory_allocator), + _store(storeCfg, std::move(memory_allocator)) { } diff --git a/searchlib/src/vespa/searchlib/attribute/postingchange.cpp b/searchlib/src/vespa/searchlib/attribute/postingchange.cpp index c3e6d344fb0..7f3ba027d7a 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingchange.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postingchange.cpp @@ -308,7 +308,7 @@ compute(const MultivalueMapping & mvm, const DocIndices & docIndices, for (const auto & docIndex : docIndices) { vespalib::ConstArrayRef oldIndices(mvm.get(docIndex.first)); added.clear(), changed.clear(), removed.clear(); - actualChange.compute(&docIndex.second[0], docIndex.second.size(), &oldIndices[0], oldIndices.size(), + actualChange.compute(docIndex.second.data(), docIndex.second.size(), oldIndices.data(), oldIndices.size(), added, changed, removed); for (const auto & wi : added) { changePost[EnumPostingPair(wi.value_ref().load_relaxed(), &compare)].add(docIndex.first, wi.weight()); diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp index 742b67f20ae..0e0dceaf254 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp @@ -112,10 +112,10 @@ PostingListAttributeBase

::updatePostings(PostingMap &changePost, auto updater= [this, &change](EntryRef posting_idx) -> EntryRef { _postingList.apply(posting_idx, - &change._additions[0], - &change._additions[0] + change._additions.size(), - &change._removals[0], - &change._removals[0] + change._removals.size()); + change._additions.data(), + change._additions.data() + change._additions.size(), + change._removals.data(), + change._removals.data() + change._removals.size()); return posting_idx; }; _dictionary.update_posting_list(idx, cmp, updater); diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index efb5898fde4..ff68c75a362 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -192,7 +192,7 @@ CompiledRankingExpressionExecutor::execute(uint32_t) for (; i < _params.size(); ++i) { _params[i] = inputs().get_number(i); } - outputs().set_number(0, _ranking_function(&_params[0])); + outputs().set_number(0, _ranking_function(_params.data())); } //----------------------------------------------------------------------------- diff --git a/vespalib/src/vespa/vespalib/datastore/array_store.h b/vespalib/src/vespa/vespalib/datastore/array_store.h index 30e0c3ab91f..7bf24194ce2 100644 --- a/vespalib/src/vespa/vespalib/datastore/array_store.h +++ b/vespalib/src/vespa/vespalib/datastore/array_store.h @@ -101,6 +101,12 @@ public: vespalib::GenerationHolder &getGenerationHolder() { return _store.getGenerationHolder(); } void setInitializing(bool initializing) { _store.setInitializing(initializing); } + // need object location before construction + static vespalib::GenerationHolder &getGenerationHolderLocation(ArrayStore &self) { + return DataStoreBase::getGenerationHolderLocation(self._store); + } + + // Should only be used for unit testing const BufferState &bufferState(EntryRef ref) const; diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.h b/vespalib/src/vespa/vespalib/datastore/datastore.h index 3ede2ada953..bb460900606 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.h +++ b/vespalib/src/vespa/vespalib/datastore/datastore.h @@ -35,7 +35,7 @@ public: DataStoreT(const DataStoreT &rhs) = delete; DataStoreT &operator=(const DataStoreT &rhs) = delete; DataStoreT(); - ~DataStoreT(); + ~DataStoreT() override; /** * Increase number of dead elements in buffer. diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 0063f497558..20104670085 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -370,6 +370,11 @@ public: return _genHolder; } + // need object location before construction + static vespalib::GenerationHolder &getGenerationHolderLocation(DataStoreBase &self) { + return self._genHolder; + } + uint32_t startCompactWorstBuffer(uint32_t typeId); std::vector startCompactWorstBuffers(CompactionSpec compaction_spec, const CompactionStrategy &compaction_strategy); uint64_t get_compaction_count() const { return _compaction_count.load(std::memory_order_relaxed); } -- cgit v1.2.3