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 <jwellbelove@users.noreply.github.com>
This commit is contained in:
Roland Reichwein 2026-07-07 12:32:56 +02:00 committed by GitHub
parent 5cd2f28cd5
commit 98cb365d4c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 66 additions and 0 deletions

View File

@ -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;
}

View File

@ -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<int>(Size); ++key)
{
CHECK_EQUAL(static_cast<int>(histogram1[key]), static_cast<int>(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<int>(Size); ++key)
{
CHECK_EQUAL(static_cast<int>(histogram1[key]), static_cast<int>(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<int>(Size); ++key)
{
CHECK_EQUAL(static_cast<int>(histogram1[key]), static_cast<int>(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<int>(Size); ++key)
{
CHECK_EQUAL(static_cast<int>(histogram1[key]), static_cast<int>(histogram2[key]));
}
}
#endif
//*************************************************************************
TEST(test_string_histogram_constructor)
{