Skip to content

Commit

Permalink
utils/chunked_vector: Fix sigsegv during reserve()
Browse files Browse the repository at this point in the history
Fixes the case of make_room() invoked with last_chunk_capacity_deficit
but _size not in the last reserved chunk.

Found during code review, no known user impact.

Fixes #10363.

Message-Id: <20220411222605.641614-1-tgrabiec@scylladb.com>
(cherry picked from commit 01eeb33)

[avi: make max_chunk_capacity() public for backport]
  • Loading branch information
tgrabiec authored and avikivity committed Apr 13, 2022
1 parent a205f64 commit e50452b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
29 changes: 29 additions & 0 deletions test/boost/chunked_vector_test.cc
Expand Up @@ -191,3 +191,32 @@ BOOST_AUTO_TEST_CASE(tests_reserve_partial) {
BOOST_REQUIRE_EQUAL(v.capacity(), orig_size);
}
}

// Tests the case of make_room() invoked with last_chunk_capacity_deficit but _size not in
// the last reserved chunk.
BOOST_AUTO_TEST_CASE(test_shrinking_and_expansion_involving_chunk_boundary) {
using vector_type = utils::chunked_vector<std::unique_ptr<uint64_t>>;
vector_type v;

// Fill two chunks
v.reserve(vector_type::max_chunk_capacity() * 3 / 2);
for (uint64_t i = 0; i < vector_type::max_chunk_capacity() * 3 / 2; ++i) {
v.emplace_back(std::make_unique<uint64_t>(i));
}

// Make the last chunk smaller than max size to trigger the last_chunk_capacity_deficit path in make_room()
v.shrink_to_fit();

// Leave the last chunk reserved but empty
for (uint64_t i = 0; i < vector_type::max_chunk_capacity(); ++i) {
v.pop_back();
}

// Try to reserve more than the currently reserved capacity and trigger last_chunk_capacity_deficit path
// with _size not in the last chunk. Should not sigsegv.
v.reserve(vector_type::max_chunk_capacity() * 4);

for (uint64_t i = 0; i < vector_type::max_chunk_capacity() * 2; ++i) {
v.emplace_back(std::make_unique<uint64_t>(i));
}
}
7 changes: 5 additions & 2 deletions utils/chunked_vector.hh
Expand Up @@ -52,10 +52,11 @@ class chunked_vector {
utils::small_vector<chunk_ptr, 1> _chunks;
size_t _size = 0;
size_t _capacity = 0;
private:
public:
static size_t max_chunk_capacity() {
return std::max(max_contiguous_allocation / sizeof(T), size_t(1));
}
private:
void reserve_for_push_back() {
if (_size == _capacity) {
do_reserve_for_push_back();
Expand Down Expand Up @@ -387,7 +388,9 @@ chunked_vector<T, max_contiguous_allocation>::make_room(size_t n, bool stop_afte
auto new_last_chunk_capacity = last_chunk_capacity + capacity_increase;
// FIXME: realloc? maybe not worth the complication; only works for PODs
auto new_last_chunk = new_chunk(new_last_chunk_capacity);
migrate(addr(_capacity - last_chunk_capacity), addr(_size), new_last_chunk.get());
if (_size > _capacity - last_chunk_capacity) {
migrate(addr(_capacity - last_chunk_capacity), addr(_size), new_last_chunk.get());
}
_chunks.back() = std::move(new_last_chunk);
_capacity += capacity_increase;
}
Expand Down

0 comments on commit e50452b

Please sign in to comment.