From 98cb365d4c28e7b72f8547b734a03a4068acd3f5 Mon Sep 17 00:00:00 2001 From: Roland Reichwein Date: Tue, 7 Jul 2026 12:32:56 +0200 Subject: [PATCH] Fix etl::histogram copy/move not preserving start_index (#1485) The run-time-offset specialization of etl::histogram stores a start_index member used to map keys to bins (accumulator[key - start_index]). Its copy constructor, move constructor, copy assignment, and move assignment copied only the accumulator and left start_index uninitialized, so a copied or moved histogram indexed the wrong bin in operator[]/add(), causing out-of-bounds access (undefined behavior). All four special member functions now also copy start_index. The compile-time-offset specialization is unaffected, as its start index is a template constant rather than a data member. Add regression tests covering copy/move construction and assignment. Co-authored-by: John Wellbelove --- include/etl/histogram.h | 4 +++ test/test_histogram.cpp | 62 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/include/etl/histogram.h b/include/etl/histogram.h index 95943b76..aad21b18 100644 --- a/include/etl/histogram.h +++ b/include/etl/histogram.h @@ -296,6 +296,7 @@ namespace etl histogram(const histogram& other) { this->accumulator = other.accumulator; + start_index = other.start_index; } #if ETL_USING_CPP11 @@ -305,6 +306,7 @@ namespace etl histogram(histogram&& other) { this->accumulator = etl::move(other.accumulator); + start_index = other.start_index; } #endif @@ -314,6 +316,7 @@ namespace etl histogram& operator=(const histogram& rhs) { this->accumulator = rhs.accumulator; + start_index = rhs.start_index; return *this; } @@ -325,6 +328,7 @@ namespace etl histogram& operator=(histogram&& rhs) { this->accumulator = etl::move(rhs.accumulator); + start_index = rhs.start_index; return *this; } diff --git a/test/test_histogram.cpp b/test/test_histogram.cpp index 624c0e68..40f5de20 100644 --- a/test/test_histogram.cpp +++ b/test/test_histogram.cpp @@ -248,6 +248,68 @@ namespace CHECK_EQUAL(0U, histogram2.count()); } + //************************************************************************* + // operator[] of the run time offset histogram uses the stored start index, + // so copying/moving must preserve it. Regression test: the copy/move + // constructors and assignment operators used to forget to copy start_index. + //************************************************************************* + TEST(test_int_runtime_offset_copy_constructor_preserves_start_index) + { + IntRuntimeOffsetHistogram histogram1(Start, input2.begin(), input2.end()); + + IntRuntimeOffsetHistogram histogram2(histogram1); + + for (int key = Start; key < Start + static_cast(Size); ++key) + { + CHECK_EQUAL(static_cast(histogram1[key]), static_cast(histogram2[key])); + } + } + + //************************************************************************* + TEST(test_int_runtime_offset_copy_assignment_preserves_start_index) + { + IntRuntimeOffsetHistogram histogram1(Start, input2.begin(), input2.end()); + + IntRuntimeOffsetHistogram histogram2(Start); + histogram2 = histogram1; + + for (int key = Start; key < Start + static_cast(Size); ++key) + { + CHECK_EQUAL(static_cast(histogram1[key]), static_cast(histogram2[key])); + } + } + +#if ETL_USING_CPP11 + //************************************************************************* + TEST(test_int_runtime_offset_move_constructor_preserves_start_index) + { + IntRuntimeOffsetHistogram histogram1(Start, input2.begin(), input2.end()); + IntRuntimeOffsetHistogram histogram_source(Start, input2.begin(), input2.end()); + + IntRuntimeOffsetHistogram histogram2(etl::move(histogram_source)); + + for (int key = Start; key < Start + static_cast(Size); ++key) + { + CHECK_EQUAL(static_cast(histogram1[key]), static_cast(histogram2[key])); + } + } + + //************************************************************************* + TEST(test_int_runtime_offset_move_assignment_preserves_start_index) + { + IntRuntimeOffsetHistogram histogram1(Start, input2.begin(), input2.end()); + IntRuntimeOffsetHistogram histogram_source(Start, input2.begin(), input2.end()); + + IntRuntimeOffsetHistogram histogram2(Start); + histogram2 = etl::move(histogram_source); + + for (int key = Start; key < Start + static_cast(Size); ++key) + { + CHECK_EQUAL(static_cast(histogram1[key]), static_cast(histogram2[key])); + } + } +#endif + //************************************************************************* TEST(test_string_histogram_constructor) {