assert: split CHECK_LIMIT into INCLUSIVE and INDEX

LIMIT hid an off-by-one bug when tests used end iterators. We rename the
assertion to uncover all uses of the flawed implementation, and split it
into an identical assertion, and one intended to protect against
iterator ends.
This commit is contained in:
Danny Robson 2020-09-24 08:03:41 +10:00
parent dd281fbe1e
commit fdaa5e1392
13 changed files with 51 additions and 27 deletions

View File

@ -127,7 +127,8 @@ namespace cruft {
void erase (iterator pos) void erase (iterator pos)
{ {
CHECK_LIMIT (pos, begin (), end ()); CHECK_GE (pos, begin ());
CHECK_LT (pos, end ());
for (auto cursor = pos + 1; cursor != end (); ++cursor) for (auto cursor = pos + 1; cursor != end (); ++cursor)
*(cursor - 1) = std::move (*cursor); *(cursor - 1) = std::move (*cursor);
@ -147,7 +148,8 @@ namespace cruft {
iterator insert (const_iterator pos, size_type count, ValueT const &val) iterator insert (const_iterator pos, size_type count, ValueT const &val)
{ {
// Ensure we have enough space // Ensure we have enough space
CHECK_LIMIT (pos, begin (), end ()); CHECK_GE (pos, begin ());
CHECK_LT (pos, end ());
CHECK_LE (m_size, CapacityV - count); CHECK_LE (m_size, CapacityV - count);
std::move_backward (pos, cend (), end () + count); std::move_backward (pos, cend (), end () + count);

View File

@ -29,7 +29,7 @@ template <typename DataT, typename SizeT>
DataT& DataT&
parray<DataT, SizeT>::operator[] (SizeT idx) parray<DataT, SizeT>::operator[] (SizeT idx)
{ {
CHECK_LIMIT(idx, 0u, m_size); CHECK_INDEX (idx, m_size);
return data ()[idx]; return data ()[idx];
} }
@ -39,7 +39,7 @@ template <typename DataT, typename SizeT>
const DataT& const DataT&
parray<DataT, SizeT>::operator[] (SizeT idx) const parray<DataT, SizeT>::operator[] (SizeT idx) const
{ {
CHECK_LIMIT(idx, 0u, m_size); CHECK_INDEX (idx, m_size);
return data ()[idx]; return data ()[idx];
} }

View File

@ -59,8 +59,8 @@ namespace cruft {
//--------------------------------------------------------------------- //---------------------------------------------------------------------
std::size_t size (void) const { return S; } std::size_t size (void) const { return S; }
T& operator[] (std::size_t i) { CHECK_LIMIT (i, 0u, S); return m_data[i]; } T& operator[] (std::size_t i) { CHECK_INDEX (i, S); return m_data[i]; }
const T& operator[] (std::size_t i) const { CHECK_LIMIT (i, 0u, S); return m_data[i]; } const T& operator[] (std::size_t i) const { CHECK_INDEX (i, S); return m_data[i]; }
auto data (void) { return m_data; } auto data (void) { return m_data; }
auto data (void) const { return m_data; } auto data (void) const { return m_data; }

View File

@ -18,7 +18,7 @@ namespace cruft {
point2f point2f
bezier<2>::eval (float t) const bezier<2>::eval (float t) const
{ {
CHECK_LIMIT (t, 0.f, 1.f); CHECK_INCLUSIVE(t, 0.f, 1.f);
const auto &P0 = m_coeffs[0]; const auto &P0 = m_coeffs[0];
const auto &P1 = m_coeffs[1]; const auto &P1 = m_coeffs[1];
@ -56,7 +56,7 @@ namespace cruft {
cruft::vector2f cruft::vector2f
bezier<2>::d1 (const float t) const noexcept bezier<2>::d1 (const float t) const noexcept
{ {
CHECK_LIMIT (t, 0.f, 1.f); CHECK_INCLUSIVE (t, 0.f, 1.f);
const auto &P0 = m_coeffs[0]; const auto &P0 = m_coeffs[0];
const auto &P1 = m_coeffs[1]; const auto &P1 = m_coeffs[1];

View File

@ -118,7 +118,7 @@ namespace cruft {
cruft::vector2f cruft::vector2f
bezier<3>::tangent (const float t) const bezier<3>::tangent (const float t) const
{ {
CHECK_LIMIT (t, 0.f, 1.f); CHECK_INCLUSIVE (t, 0.f, 1.f);
return mix ( return mix (
mix (m_coeffs[1] - m_coeffs[0], m_coeffs[2] - m_coeffs[1], t), mix (m_coeffs[1] - m_coeffs[0], m_coeffs[2] - m_coeffs[1], t),

View File

@ -154,7 +154,7 @@ template <typename ValueT>
typename circular<ValueT>::iterator typename circular<ValueT>::iterator
circular<ValueT>::constrain (iterator cursor) circular<ValueT>::constrain (iterator cursor)
{ {
CHECK_LIMIT (cursor, m_begin, m_begin + 2 * size ()); CHECK_INCLUSIVE (cursor, m_begin, m_begin + 2 * size ());
return m_begin + (cursor - m_begin) % size (); return m_begin + (cursor - m_begin) % size ();
} }
@ -165,8 +165,8 @@ template <typename ValueT>
cruft::view<typename circular<ValueT>::iterator> cruft::view<typename circular<ValueT>::iterator>
circular<ValueT>::constrain (cruft::view<iterator> window) circular<ValueT>::constrain (cruft::view<iterator> window)
{ {
CHECK_LIMIT (window.begin (), m_begin, m_begin + 2 * size ()); CHECK_INCLUSIVE (window.begin (), m_begin, m_begin + 2 * size ());
CHECK_LIMIT (window.end (), m_begin, m_begin + 2 * size ()); CHECK_INCLUSIVE (window.end (), m_begin, m_begin + 2 * size ());
auto first = window.begin (); auto first = window.begin ();
auto last = first + window.size (); auto last = first + window.size ();

View File

@ -1453,7 +1453,7 @@ namespace cruft {
constexpr auto constexpr auto
rshift (const K k, const int num, const K fill) rshift (const K k, const int num, const K fill)
{ {
CHECK_LIMIT (num, 0, int (K::elements)); CHECK_INCLUSIVE (num, 0, int (K::elements));
K res {}; K res {};

View File

@ -236,14 +236,14 @@ constexpr bool debug_enabled = false;
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
#define CHECK_LIMIT(V,L,H) do { \ #define CHECK_INCLUSIVE(V,L,H) do { \
DEBUG_ONLY ( \ DEBUG_ONLY ( \
const auto &__v = (V); \ const auto &__v = (V); \
const auto &__l = (L); \ const auto &__l = (L); \
const auto &__h = (H); \ const auto &__h = (H); \
\ \
if (__v < __l || __v > __h) { \ if (__v < __l || __v > __h) { \
std::cerr << "expected limit\n" \ std::cerr << "expected inclusive\n" \
"__l: " << #L << " is " << +__l << "\n" \ "__l: " << #L << " is " << +__l << "\n" \
"__h: " << #H << " is " << +__h << "\n" \ "__h: " << #H << " is " << +__h << "\n" \
"__v: " << #V << " is " << +__v << "\n"; \ "__v: " << #V << " is " << +__v << "\n"; \
@ -252,6 +252,28 @@ constexpr bool debug_enabled = false;
); \ ); \
} while (0) } while (0)
///////////////////////////////////////////////////////////////////////////////
#define CHECK_INDEX(V,H) do { \
DEBUG_ONLY ( \
const auto &__v = (V); \
const auto &__h = (H); \
\
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wtype-limits\"") \
\
if (intmax_t (__v) < 0 || __v >= __h) { \
std::cerr << "expected index\n" \
"__h: " << #H << " is " << +__h << "\n" \
"__v: " << #V << " is " << +__v << "\n"; \
breakpoint (); \
}; \
\
_Pragma("GCC diagnostic pop") \
); \
} while (0)
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
#define CHECK_NEQ(A,B) do { \ #define CHECK_NEQ(A,B) do { \
DEBUG_ONLY( \ DEBUG_ONLY( \

View File

@ -166,7 +166,7 @@ template <typename T>
cruft::matrix4<T> cruft::matrix4<T>
cruft::perspective (T fov, T aspect, range<T> Z) cruft::perspective (T fov, T aspect, range<T> Z)
{ {
CHECK_LIMIT (fov, 0, 2 * cruft::pi<T>); CHECK_INCLUSIVE (fov, 0, 2 * cruft::pi<T>);
CHECK_GE (Z.lo, 0); CHECK_GE (Z.lo, 0);
CHECK_GE (Z.hi, 0); CHECK_GE (Z.hi, 0);

View File

@ -225,11 +225,8 @@ namespace cruft {
/////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////
size_t index (T const *ptr) const size_t index (T const *ptr) const
{ {
CHECK_LIMIT ( CHECK_GE (ptr, reinterpret_cast<T*> (m_store));
ptr, CHECK_LT (ptr, reinterpret_cast<T*> (m_store) + capacity ());
reinterpret_cast<T*> (m_store),
reinterpret_cast<T*> (m_store) + capacity ()
);
return ptr - reinterpret_cast<T*> (m_store); return ptr - reinterpret_cast<T*> (m_store);
} }

View File

@ -248,7 +248,7 @@ template <typename T>
cruft::region<2,T> cruft::region<2,T>
cruft::rotate90 (cruft::region<2,T> obj, int steps) cruft::rotate90 (cruft::region<2,T> obj, int steps)
{ {
CHECK_LIMIT (steps, 0, 3); CHECK_INCLUSIVE (steps, 0, 3);
switch (steps) { switch (steps) {
case 0: case 0:

View File

@ -69,7 +69,7 @@ namespace cruft {
view<const char*> view<const char*>
get (component c) const& get (component c) const&
{ {
CHECK_LIMIT (c, 0, NUM_COMPONENTS); CHECK_INDEX (c, NUM_COMPONENTS);
return m_views[c]; return m_views[c];
} }

View File

@ -364,8 +364,8 @@ namespace cruft {
auto auto
slice (IndexA a, IndexB b) const slice (IndexA a, IndexB b) const
{ {
CHECK_LIMIT (cruft::abs (a), IndexA {0}, cruft::cast::lossless<IndexA> (size ())); CHECK_INCLUSIVE (cruft::abs (a), IndexA {0}, cruft::cast::lossless<IndexA> (size ()));
CHECK_LIMIT (cruft::abs (b), IndexB {0}, cruft::cast::lossless<IndexB> (size ())); CHECK_INCLUSIVE (cruft::abs (b), IndexB {0}, cruft::cast::lossless<IndexB> (size ()));
auto first = m_begin; auto first = m_begin;
auto last = m_begin; auto last = m_begin;
@ -485,7 +485,9 @@ namespace cruft {
constexpr auto&& constexpr auto&&
operator[] (size_t idx) noexcept operator[] (size_t idx) noexcept
{ {
CHECK_LIMIT (idx, 0u, size ()); CHECK_GE (idx, 0u);
CHECK_LT (idx, size ());
return *std::next (begin (), idx); return *std::next (begin (), idx);
} }
@ -493,7 +495,8 @@ namespace cruft {
constexpr auto&& constexpr auto&&
operator[] (size_t idx) const noexcept operator[] (size_t idx) const noexcept
{ {
CHECK_LIMIT (idx, 0u, size ()); CHECK_GE (idx, 0u);
CHECK_LT (idx, size ());
return *std::next (begin (), idx); return *std::next (begin (), idx);
} }