From 251e8d3e48240c6a7f7b966a711b470fbcfad3bc Mon Sep 17 00:00:00 2001 From: Michael Platings Date: Wed, 7 Feb 2024 16:38:28 +0000 Subject: [PATCH] Fix negative offsets stored as unsigned Running tests with UndefinedBehaviorSanitizer enabled emitted errors such as "addition of unsigned offset [...] overflowed" This change fixes these errors. --- intrinsiccv/include/types.h | 41 +++++++++++++------------ intrinsiccv/src/analysis/canny_neon.cpp | 4 +-- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/intrinsiccv/include/types.h b/intrinsiccv/include/types.h index 242f379f9..03b12d62e 100644 --- a/intrinsiccv/include/types.h +++ b/intrinsiccv/include/types.h @@ -137,19 +137,19 @@ class Columns final { // Subscript operator to return an arbitrary column at an index. To account // for channel count use at() method. - T &operator[](size_t index) INTRINSICCV_STREAMING_COMPATIBLE { + T &operator[](ptrdiff_t index) INTRINSICCV_STREAMING_COMPATIBLE { return ptr_[index]; } // Addition assignment operator to step across the columns. Columns &operator+=(ptrdiff_t diff) INTRINSICCV_STREAMING_COMPATIBLE { - ptr_ += channels() * diff; + ptr_ += static_cast(channels()) * diff; return *this; } // Subtraction assignment operator to step across the columns. Columns &operator-=(ptrdiff_t diff) INTRINSICCV_STREAMING_COMPATIBLE { - ptr_ -= channels() * diff; + ptr_ -= static_cast(channels()) * diff; return *this; } @@ -167,8 +167,10 @@ class Columns final { // NOLINTEND(hicpp-explicit-conversions) // Returns a new instance at a given column. - [[nodiscard]] Columns at(size_t column) INTRINSICCV_STREAMING_COMPATIBLE { - return Columns{&ptr_[column * channels()], channels()}; + [[nodiscard]] Columns at(ptrdiff_t column) + INTRINSICCV_STREAMING_COMPATIBLE { + return Columns{&ptr_[column * static_cast(channels())], + channels()}; } // Returns the number of channels in a row. @@ -257,7 +259,7 @@ class RowBase { // Adds a stride to a pointer, and returns the new pointer. template - [[nodiscard]] static P *add_stride(P *ptr, size_t stride) + [[nodiscard]] static P *add_stride(P *ptr, ptrdiff_t stride) INTRINSICCV_STREAMING_COMPATIBLE { uintptr_t intptr = reinterpret_cast(ptr); intptr += stride; @@ -268,7 +270,7 @@ class RowBase { // Subtracts a stride to a pointer, and returns the new pointer. template - [[nodiscard]] static P *subtract_stride(P *ptr, size_t stride) + [[nodiscard]] static P *subtract_stride(P *ptr, ptrdiff_t stride) INTRINSICCV_STREAMING_COMPATIBLE { uintptr_t intptr = reinterpret_cast(ptr); intptr -= stride; @@ -328,12 +330,12 @@ class Rows final : public RowBase { // Subscript operator to return an arbitrary position within the current row. // To account for stride and channel count use at() method. - T &operator[](size_t index) INTRINSICCV_STREAMING_COMPATIBLE { + T &operator[](ptrdiff_t index) INTRINSICCV_STREAMING_COMPATIBLE { return ptr_[index]; } // Addition assignment operator to navigate among rows. - Rows &operator+=(size_t diff) INTRINSICCV_STREAMING_COMPATIBLE { + Rows &operator+=(ptrdiff_t diff) INTRINSICCV_STREAMING_COMPATIBLE { ptr_ = get_pointer_at(diff); return *this; } @@ -351,8 +353,8 @@ class Rows final : public RowBase { // NOLINTEND(hicpp-explicit-conversions) // Returns a new instance at a given row and column. - [[nodiscard]] Rows at(size_t row, - size_t column = 0) INTRINSICCV_STREAMING_COMPATIBLE { + [[nodiscard]] Rows at(ptrdiff_t row, ptrdiff_t column = 0) + INTRINSICCV_STREAMING_COMPATIBLE { return Rows{get_pointer_at(row, column), stride(), channels()}; } @@ -373,10 +375,11 @@ class Rows final : public RowBase { private: // Returns a column in a row at a given index taking stride and channels into // account. - [[nodiscard]] T *get_pointer_at(size_t row, size_t column = 0) + [[nodiscard]] T *get_pointer_at(ptrdiff_t row, ptrdiff_t column = 0) INTRINSICCV_STREAMING_COMPATIBLE { - T *ptr = RowBase::add_stride(ptr_, row * stride()); - return &ptr[column * channels()]; + T *ptr = + RowBase::add_stride(ptr_, row * static_cast(stride())); + return &ptr[column * static_cast(channels())]; } // Pointer to the first row. @@ -410,12 +413,12 @@ class IndirectRows : public RowBase { // Subscript operator to return a position within the current row. To account // for stride and channel count use at() method. - T &operator[](size_t index) INTRINSICCV_STREAMING_COMPATIBLE { + T &operator[](ptrdiff_t index) INTRINSICCV_STREAMING_COMPATIBLE { return ptr_storage_[0][index]; } // Addition assignment operator to navigate among rows. - IndirectRows &operator+=(size_t diff) INTRINSICCV_STREAMING_COMPATIBLE { + IndirectRows &operator+=(ptrdiff_t diff) INTRINSICCV_STREAMING_COMPATIBLE { ptr_storage_ += diff; return *this; } @@ -426,8 +429,8 @@ class IndirectRows : public RowBase { } // Returns a new instance at a given row and column. - [[nodiscard]] Rows at(size_t row, - size_t column = 0) INTRINSICCV_STREAMING_COMPATIBLE { + [[nodiscard]] Rows at(ptrdiff_t row, ptrdiff_t column = 0) + INTRINSICCV_STREAMING_COMPATIBLE { auto rows = Rows{ptr_storage_[row], stride(), channels()}; return rows.at(0, column); } @@ -501,7 +504,7 @@ class ParallelRows final : public RowBase { : ParallelRows(ptr, stride, 1) {} // Addition assignment operator to navigate among rows. - ParallelRows &operator+=(size_t diff) INTRINSICCV_STREAMING_COMPATIBLE { + ParallelRows &operator+=(ptrdiff_t diff) INTRINSICCV_STREAMING_COMPATIBLE { ptrs_[0] = RowBase::add_stride(ptrs_[0], diff * stride()); ptrs_[1] = RowBase::add_stride(ptrs_[1], diff * stride()); return *this; diff --git a/intrinsiccv/src/analysis/canny_neon.cpp b/intrinsiccv/src/analysis/canny_neon.cpp index 733e7e08c..ec06a51a4 100644 --- a/intrinsiccv/src/analysis/canny_neon.cpp +++ b/intrinsiccv/src/analysis/canny_neon.cpp @@ -370,7 +370,7 @@ static void non_maxima_suppression_and_high_thresholding( Rows vertical_gradient_rows, Rows magnitude_rows, Rows dst_rows, double high_threshold, StrongEdgeStack &strong_edge_pixels) { - static constexpr size_t kNumLanes = VecTraits::num_lanes(); + static constexpr ptrdiff_t kNumLanes = VecTraits::num_lanes(); static const int16_t kPositionBits[8] = { 0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80, }; @@ -379,7 +379,7 @@ static void non_maxima_suppression_and_high_thresholding( const int16x8_t high_threshold_vec = vdupq_n_s16(static_cast(high_threshold)); - auto handle_row = [&](size_t index) { + auto handle_row = [&](ptrdiff_t index) { if (is_vect_len_memory_null(&magnitude_rows.at(1)[index + kNumLanes])) { memset(&dst_rows[index], 0, kNumLanes); return; -- GitLab