From 106b2d8ded04dbc51031022e9b11d147dccb76a3 Mon Sep 17 00:00:00 2001 From: Kevin Boyd Date: Wed, 8 Jul 2020 13:44:30 +0000 Subject: [PATCH] Use signed arithmetic in enumeration helper With a signed enum and the unsigned step, implicit conversion to unsigned could change the value in the subtraction operator. Note that errors could also occur with negative results in pure unsigned operations. A negative result isn't expected for increasing enumerators, but the boost iterator implementation does an == comparison that invokes the diff operator and compares it to 0, and this error actually occurs there in unit tests, and is caught by UBSAN. Added some SFINAE to disable accidental instantiation of non-enum types - before C++ 20 std::underlying_type on a non-enum is undefined. --- src/gromacs/utility/enumerationhelpers.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gromacs/utility/enumerationhelpers.h b/src/gromacs/utility/enumerationhelpers.h index e9119e27dd..33cc0d2a09 100644 --- a/src/gromacs/utility/enumerationhelpers.h +++ b/src/gromacs/utility/enumerationhelpers.h @@ -112,11 +112,12 @@ namespace gmx * \tparam Last Last constant or number thereof (assumes a default 'Count' member). * \tparam Step Step increment. */ -template +template class EnumerationIterator final : public boost::stl_interfaces::iterator_interface, std::random_access_iterator_tag, EnumType> { public: + static_assert(std::is_enum_v, "Enumeration iterator must be over an enum type."); //! Convenience alias using IntegerType = std::underlying_type_t; @@ -143,7 +144,7 @@ public: //! Difference operator constexpr std::ptrdiff_t operator-(const EnumerationIterator other) const noexcept { - return (m_current - other.m_current) / Step; + return (static_cast(m_current) - static_cast(other.m_current)) / Step; } private: -- 2.11.4.GIT