From 843cacffbf6b8afb0a09b0d4262430d939d4f336 Mon Sep 17 00:00:00 2001 From: Hans Dembinski Date: Wed, 20 Jul 2022 09:51:16 +0200 Subject: [PATCH] fix indexed crash for histograms with axes of zero size (physical or logical) (#356) --- include/boost/histogram/indexed.hpp | 17 ++++++++-- test/CMakeLists.txt | 1 + test/Jamfile | 1 + test/issue_353_test.cpp | 48 +++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 test/issue_353_test.cpp diff --git a/include/boost/histogram/indexed.hpp b/include/boost/histogram/indexed.hpp index 7eae5dc0..c636abf5 100644 --- a/include/boost/histogram/indexed.hpp +++ b/include/boost/histogram/indexed.hpp @@ -286,7 +286,7 @@ public: private: iterator(value_iterator i, histogram_type& h) : iter_(i), indices_(&h) {} - value_iterator iter_; + value_iterator iter_; // original histogram iterator struct index_data { axis::index_type idx, begin, end; @@ -319,6 +319,9 @@ public: template > indexed_range(histogram_type& hist, Iterable&& range) : begin_(hist.begin(), hist), end_(hist.end(), hist) { + // if histogram is empty, incrementing begin_.iter_ may be undefined behavior + if (begin_ == end_) return; + auto r_begin = std::begin(range); assert(std::distance(r_begin, std::end(range)) == static_cast(hist.rank())); @@ -339,12 +342,20 @@ public: ca->begin_skip = static_cast(ca->begin - start) * stride; ca->end_skip = static_cast(stop - ca->end) * stride; begin_.iter_ += ca->begin_skip; + end_.iter_ -= ca->end_skip; stride *= stop - start; ++ca; ++r_begin; }); + // check if selected range is empty + if (end_.iter_ < begin_.iter_) { + begin_ = end_; + } else { + // reset end_ to hist.end(), since end_skips are done in operator++ + end_.iter_ = hist.end(); + } } iterator begin() noexcept { return begin_; } @@ -358,8 +369,8 @@ private: (*it)[0] = 0; (*it)[1] = a.size(); if (cov == coverage::all) { - (*it)[0] -= 1; - (*it)[1] += 1; + (*it)[0] -= 1; // making this wider than actual range is safe + (*it)[1] += 1; // making this wider than actual range is safe } else assert(cov == coverage::inner); ++it; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 007c0f56..1e08d801 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -89,6 +89,7 @@ boost_test(TYPE run SOURCES storage_adaptor_test.cpp) boost_test(TYPE run SOURCES unlimited_storage_test.cpp) boost_test(TYPE run SOURCES utility_test.cpp) boost_test(TYPE run SOURCES issue_327_test.cpp) +boost_test(TYPE run SOURCES issue_353_test.cpp) find_package(Threads) if (Threads_FOUND) diff --git a/test/Jamfile b/test/Jamfile index 1fc81092..789c33a0 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -88,6 +88,7 @@ alias cxx14 : [ run unlimited_storage_test.cpp ] [ run utility_test.cpp ] [ run issue_327_test.cpp ] + [ run issue_353_test.cpp ] ; alias cxx17 : diff --git a/test/issue_353_test.cpp b/test/issue_353_test.cpp new file mode 100644 index 00000000..8f522659 --- /dev/null +++ b/test/issue_353_test.cpp @@ -0,0 +1,48 @@ +#include +#include +#include +#include "throw_exception.hpp" + +namespace bh = boost::histogram; + +struct empty { + int index(double) { return 0; } + int size() const { return 0; } +}; + +int main() { + { + auto h = + bh::make_histogram_with(std::vector(), bh::axis::integer<>(0, 2), empty()); + + auto ind1 = bh::indexed(h, bh::coverage::all); + BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 0); + + auto ind2 = bh::indexed(h, bh::coverage::inner); + BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 0); + } + + { + auto h = bh::make_histogram_with(std::vector(), bh::axis::integer<>(0, 2), + bh::axis::integer<>(0, 1)); + + auto ind1 = bh::indexed(h, bh::coverage::all); + BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 12); + + auto ind2 = bh::indexed(h, bh::coverage::inner); + BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 2); + } + + { + auto h = bh::make_histogram_with(std::vector(), bh::axis::integer<>(0, 2), + bh::axis::integer<>(0, 0)); + + auto ind1 = bh::indexed(h, bh::coverage::all); + BOOST_TEST_EQ(std::distance(ind1.begin(), ind1.end()), 8); + + auto ind2 = bh::indexed(h, bh::coverage::inner); + BOOST_TEST_EQ(std::distance(ind2.begin(), ind2.end()), 0); + } + + return boost::report_errors(); +} \ No newline at end of file