From 442c6baac95fd5e2d1de2b4aacfdedb452e76dd8 Mon Sep 17 00:00:00 2001 From: cristianchiriac <80774326+cristianchiriac@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:40:48 +0300 Subject: [PATCH 1/3] Fix vector insert of a value that refers to an element of the vector insert(position, value) and insert(position, n, value) shifted the elements before reading value, so a value taken from the vector at or after the insert position read the shifted element instead. Fixes #1629 --- include/etl/vector.h | 23 +++++++++++++++--- test/test_vector.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/include/etl/vector.h b/include/etl/vector.h index beac6cdc..ae3bb89a 100644 --- a/include/etl/vector.h +++ b/include/etl/vector.h @@ -611,9 +611,18 @@ namespace etl } else { + // value may refer to an element that is about to be shifted. + const_pointer p_value = etl::addressof(value); + create_back(back()); etl::move_backward(position_, p_end - 2, p_end - 1); - *position_ = value; + + if ((p_value >= position_) && (p_value < p_end)) + { + ++p_value; + } + + *position_ = *p_value; } return position_; @@ -836,6 +845,14 @@ namespace etl size_t copy_new_n = construct_old_n; size_t construct_new_n = insert_n - copy_new_n; + // value may refer to an element that is about to be shifted. + const_pointer p_value = etl::addressof(value); + + if ((p_value >= position_) && (p_value < p_end)) + { + p_value += n; + } + // Construct old. etl::uninitialized_move(p_end - construct_old_n, p_end, p_construct_old); ETL_ADD_DEBUG_COUNT(construct_old_n); @@ -844,11 +861,11 @@ namespace etl etl::move_backward(p_buffer + insert_begin, p_buffer + insert_begin + copy_old_n, p_buffer + insert_end + copy_old_n); // Construct new. - etl::uninitialized_fill_n(p_end, construct_new_n, value); + etl::uninitialized_fill_n(p_end, construct_new_n, *p_value); ETL_ADD_DEBUG_COUNT(construct_new_n); // Copy new. - etl::fill_n(p_buffer + insert_begin, copy_new_n, value); + etl::fill_n(p_buffer + insert_begin, copy_new_n, *p_value); p_end += n; } diff --git a/test/test_vector.cpp b/test/test_vector.cpp index 32799337..e14542c7 100644 --- a/test/test_vector.cpp +++ b/test/test_vector.cpp @@ -777,6 +777,33 @@ namespace } } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_insert_position_value_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + Compare_Data compare_data; + Data data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.insert(data.cbegin() + offset, data[source]); + compare_data.insert(compare_data.cbegin() + static_cast(offset), compare_data[source]); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin()); + + CHECK(is_equal); + } + } + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_insert_position_value_outofbounds) { @@ -971,6 +998,36 @@ namespace } } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_insert_position_n_value_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t insert_size = 1; insert_size <= 4; ++insert_size) + { + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + Compare_Data compare_data; + Data data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.insert(data.cbegin() + offset, insert_size, data[source]); + compare_data.insert(compare_data.cbegin() + static_cast(offset), insert_size, compare_data[source]); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin()); + + CHECK(is_equal); + } + } + } + } + //************************************************************************* #include "etl/private/diagnostic_uninitialized_push.h" TEST_FIXTURE(SetupFixture, test_insert_position_n_value_outofbounds) From 35542a36f27797c0fb8f43074dbf9e87d4f4da40 Mon Sep 17 00:00:00 2001 From: cristianchiriac <80774326+cristianchiriac@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:24:24 +0300 Subject: [PATCH 2/3] Fix vector rvalue insert and emplace of a value that refers to an element of the vector insert(position, T&&) moved from value after shifting the elements, so a value referring to an element at or after the insert position read the shifted element. It now uses the same address adjustment as the lvalue insert. emplace(position, args...) destroyed the element at the insert position before constructing the new one, so arguments referring to elements of the vector could read a shifted or destroyed element. The new value is now constructed before the elements are shifted, for both the variadic and the C++03 overloads. --- include/etl/vector.h | 84 +++++++++++++++++--------------- test/test_vector.cpp | 58 ++++++++++++++++++++++ test/test_vector_non_trivial.cpp | 81 ++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 39 deletions(-) diff --git a/include/etl/vector.h b/include/etl/vector.h index ae3bb89a..3285577f 100644 --- a/include/etl/vector.h +++ b/include/etl/vector.h @@ -649,9 +649,18 @@ namespace etl } else { + // value may refer to an element that is about to be shifted. + pointer p_value = etl::addressof(value); + create_back(etl::move(back())); etl::move_backward(position_, p_end - 2, p_end - 1); - *position_ = etl::move(value); + + if ((p_value >= position_) && (p_value < p_end)) + { + ++p_value; + } + + *position_ = etl::move(*p_value); } return position_; @@ -670,25 +679,26 @@ namespace etl iterator position_ = to_iterator(position); - void* p; - if (position_ == end()) { - p = p_end++; + #include "private/diagnostic_sign_conversion_push.h" + ::new (p_end) T(etl::forward(args)...); + #include "private/diagnostic_pop.h" + ++p_end; ETL_INCREMENT_DEBUG_COUNT; } else { - p = etl::addressof(*position_); - create_back(back()); - etl::move_backward(position_, p_end - 2, p_end - 1); - (*position_).~T(); - } - + // args may refer to elements that are about to be shifted, so construct the value first. #include "private/diagnostic_sign_conversion_push.h" - ::new (p) T(etl::forward(args)...); + T value(etl::forward(args)...); #include "private/diagnostic_pop.h" + create_back(etl::move(back())); + etl::move_backward(position_, p_end - 2, p_end - 1); + *position_ = etl::move(value); + } + return position_; } #else @@ -700,23 +710,22 @@ namespace etl iterator position_ = to_iterator(position); - void* p; - if (position_ == end()) { - p = p_end++; + ::new (p_end) T(value1); + ++p_end; ETL_INCREMENT_DEBUG_COUNT; } else { - p = etl::addressof(*position_); + // The values may refer to elements that are about to be shifted, so construct the value first. + T value(value1); + create_back(back()); etl::move_backward(position_, p_end - 2, p_end - 1); - (*position_).~T(); + *position_ = value; } - ::new (p) T(value1); - return position_; } @@ -728,23 +737,22 @@ namespace etl iterator position_ = to_iterator(position); - void* p; - if (position_ == end()) { - p = p_end++; + ::new (p_end) T(value1, value2); + ++p_end; ETL_INCREMENT_DEBUG_COUNT; } else { - p = etl::addressof(*position_); + // The values may refer to elements that are about to be shifted, so construct the value first. + T value(value1, value2); + create_back(back()); etl::move_backward(position_, p_end - 2, p_end - 1); - (*position_).~T(); + *position_ = value; } - ::new (p) T(value1, value2); - return position_; } @@ -756,23 +764,22 @@ namespace etl iterator position_ = to_iterator(position); - void* p; - if (position_ == end()) { - p = p_end++; + ::new (p_end) T(value1, value2, value3); + ++p_end; ETL_INCREMENT_DEBUG_COUNT; } else { - p = etl::addressof(*position_); + // The values may refer to elements that are about to be shifted, so construct the value first. + T value(value1, value2, value3); + create_back(back()); etl::move_backward(position_, p_end - 2, p_end - 1); - (*position_).~T(); + *position_ = value; } - ::new (p) T(value1, value2, value3); - return position_; } @@ -784,23 +791,22 @@ namespace etl iterator position_ = to_iterator(position); - void* p; - if (position_ == end()) { - p = p_end++; + ::new (p_end) T(value1, value2, value3, value4); + ++p_end; ETL_INCREMENT_DEBUG_COUNT; } else { - p = etl::addressof(*position_); + // The values may refer to elements that are about to be shifted, so construct the value first. + T value(value1, value2, value3, value4); + create_back(back()); etl::move_backward(position_, p_end - 2, p_end - 1); - (*position_).~T(); + *position_ = value; } - ::new (p) T(value1, value2, value3, value4); - return position_; } #endif diff --git a/test/test_vector.cpp b/test/test_vector.cpp index e14542c7..3612887f 100644 --- a/test/test_vector.cpp +++ b/test/test_vector.cpp @@ -804,6 +804,35 @@ namespace } } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_insert_position_rvalue_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + Compare_Data compare_data; + Data data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.insert(data.cbegin() + offset, std::move(data[source])); + + int value = compare_data[source]; + compare_data.insert(compare_data.cbegin() + static_cast(offset), value); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin()); + + CHECK(is_equal); + } + } + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_insert_position_value_outofbounds) { @@ -841,6 +870,35 @@ namespace } } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_emplace_position_value_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + Compare_Data compare_data; + Data data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.emplace(data.cbegin() + offset, data[source]); + + int value = compare_data[source]; + compare_data.insert(compare_data.cbegin() + static_cast(offset), value); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin()); + + CHECK(is_equal); + } + } + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_emplace_position_value_outofbounds) { diff --git a/test/test_vector_non_trivial.cpp b/test/test_vector_non_trivial.cpp index cde6dc8d..a8031521 100644 --- a/test/test_vector_non_trivial.cpp +++ b/test/test_vector_non_trivial.cpp @@ -995,6 +995,87 @@ namespace } } + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_emplace_position_value_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + CompareDataNDC compare_data; + DataNDC data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.emplace(data.begin() + offset, data[source]); + + NDC value(compare_data[source]); + compare_data.insert(compare_data.begin() + static_cast(offset), value); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.begin(), data.end(), compare_data.begin()); + + CHECK(is_equal); + } + } + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_emplace_position_member_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + CompareDataNDC compare_data; + DataNDC data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + data.emplace(data.begin() + offset, data[source].value, data[source].index); + + NDC value(compare_data[source].value, compare_data[source].index); + compare_data.insert(compare_data.begin() + static_cast(offset), value); + + CHECK_EQUAL(compare_data.size(), data.size()); + + bool is_equal = std::equal(data.begin(), data.end(), compare_data.begin()); + + CHECK(is_equal); + } + } + } + + //************************************************************************* + TEST_FIXTURE(SetupFixture, test_insert_position_rvalue_from_self) + { + const size_t INITIAL_SIZE = 5; + + for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset) + { + for (size_t source = 0; source < INITIAL_SIZE; ++source) + { + DataNDC data; + + data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE); + + const NDC expected(data[source]); + + data.insert(data.begin() + offset, std::move(data[source])); + + CHECK_EQUAL(INITIAL_SIZE + 1, data.size()); + CHECK(expected == data[offset]); + } + } + } + //************************************************************************* TEST_FIXTURE(SetupFixture, test_emplace_position_value_outofbounds) { From cd81277383dde5d23a7bc3fa0af8acee1aa02a88 Mon Sep 17 00:00:00 2001 From: cristianchiriac <80774326+cristianchiriac@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:58:09 +0000 Subject: [PATCH 3/3] Keep the emplace comment after the diagnostic include for clang-format clang-format indented the comment to align it with the preprocessor line that followed it, so the formatting check failed on vector.h. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ce8Hr56xHurqiV5wEkVmxj --- include/etl/vector.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/etl/vector.h b/include/etl/vector.h index 3285577f..16ecaab0 100644 --- a/include/etl/vector.h +++ b/include/etl/vector.h @@ -689,8 +689,8 @@ namespace etl } else { - // args may refer to elements that are about to be shifted, so construct the value first. #include "private/diagnostic_sign_conversion_push.h" + // args may refer to elements that are about to be shifted, so construct the value first. T value(etl::forward(args)...); #include "private/diagnostic_pop.h"