From fbea2b132e8112a285386630df65fe5693b60226 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Thu, 16 Jul 2026 15:12:19 +0200 Subject: [PATCH 1/3] Fix benign TSAN warning in TRUCO The race is benign functionally because threads only modify FOREGROUND (255) to VISITED_OUTER_RIGHT (100) or VISITED_ (200), all of which are NON-ZERO. findStartContourPoint only cares if a pixel is zero or non-zero, so its termination conditions are unchanged. However, concurrent read/write of different values is legally UB in C++ and triggers TSAN. --- modules/imgproc/src/contours_truco.cpp | 58 +++++++++++++++++++------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/modules/imgproc/src/contours_truco.cpp b/modules/imgproc/src/contours_truco.cpp index 94f1f332f6..f41bf42abe 100644 --- a/modules/imgproc/src/contours_truco.cpp +++ b/modules/imgproc/src/contours_truco.cpp @@ -2,10 +2,33 @@ #include "precomp.hpp" #include "contours_common.hpp" #include "opencv2/core/hal/intrin.hpp" -#include +#include namespace{ +// Atomic operations to avoid data race between findStartContourPoint and +// traceExternalContourMock. The race is benign functionally because threads +// only modify FOREGROUND (255) to VISITED_OUTER_RIGHT (100) or VISITED_ (200), +// all of which are NON-ZERO. findStartContourPoint only cares if a pixel is +// zero or non-zero, so its termination conditions are unchanged. However, +// concurrent read/write of different values is legally UB in C++ and triggers +// TSAN. +static inline uchar atomicLoad(const uchar* ptr) { + if constexpr (sizeof(std::atomic) == sizeof(uchar)) { + return reinterpret_cast*>(ptr)->load(std::memory_order_relaxed); + } else { + return *ptr; + } +} + +static inline void atomicStore(uchar* ptr, uchar val) { + if constexpr (sizeof(std::atomic) == sizeof(uchar)) { + reinterpret_cast*>(ptr)->store(val, std::memory_order_relaxed); + } else { + *ptr = val; + } +} + // Tunable block size. 1024 points = 8KB (Fits easily in L1 Cache) template class TRUCOPagedContour { @@ -186,10 +209,10 @@ public: int idx = search_idx + n; // Use offset cache uchar* neighbor = curr_ptr + offsets_[idx]; - if (*neighbor == BACKGROUND) continue; + if (atomicLoad(neighbor) == BACKGROUND) continue; dir = idx & 7; if (((search_idx <= 1) || (dir <= search_idx - 2)) && (curr_x!=c && curr_y!=r))//do nt apply to first pixel in the way back - *curr_ptr = VISITED_OUTER_RIGHT; + atomicStore(curr_ptr, VISITED_OUTER_RIGHT); // --- EXECUTE MOVE --- curr_y += dy_[dir]; curr_x += dx_[dir]; @@ -238,7 +261,7 @@ public: for ( n = 0; n < 8; ++n) { int idx = search_idx + n; - if ( *(curr_ptr + offsets_[idx]) == BACKGROUND) continue; + if (atomicLoad(curr_ptr + offsets_[idx]) == BACKGROUND) continue; if(curr_x+ dx_[ idx & 7]!=c+1 || curr_y+ dy_[ idx & 7]!=r-1) return false; break; } @@ -254,7 +277,7 @@ public: int idx = search_idx + n; // Use offset cache uchar* neighbor = curr_ptr + offsets_[idx]; - if (*neighbor == BACKGROUND) continue; + if (atomicLoad(neighbor) == BACKGROUND) continue; dir = idx & 7; // --- EXECUTE MOVE --- @@ -268,11 +291,11 @@ public: } if ((search_idx <= 1) || (dir <= search_idx - 2)) { - *curr_ptr = VISITED_OUTER_RIGHT; + atomicStore(curr_ptr, VISITED_OUTER_RIGHT); } - else if (*curr_ptr == FOREGROUND) + else if (atomicLoad(curr_ptr) == FOREGROUND) { - *curr_ptr = VISITED_; + atomicStore(curr_ptr, VISITED_); } // Short-circuit Jacob's Check @@ -291,7 +314,7 @@ public: } if (is_first_move) { if(dir==-1){//single pixel - *curr_ptr = VISITED_OUTER_RIGHT; + atomicStore(curr_ptr, VISITED_OUTER_RIGHT); break;//not moved } start_dir = dir; @@ -328,7 +351,7 @@ public: if ((c = findStartContourPoint(row_ptr, cols, c)) == cols) break; // 2. CHECK: Only process if actually FOREGROUND (redundancy check) - if (row_ptr[c] == FOREGROUND && r=cols)break;//end of row //internal contour - if(row_ptr[c-1]>VISITED_OUTER_RIGHT && r>rowRange.start){//inner contours of first line are handled by the thread above + if (atomicLoad(&row_ptr[c - 1]) > VISITED_OUTER_RIGHT && r>rowRange.start){//inner contours of first line are handled by the thread above if(traceContour(&buffer,r,c-1,row_ptr,rowRange,false)){ // Post-processing @@ -388,8 +411,10 @@ public: } } #endif - for (; j < width && !src_data[j]; ++j) - ; + for (; j < width; ++j) { + if (atomicLoad(&src_data[j])) break; + } + return j; } @@ -414,8 +439,9 @@ public: } } #endif - for (; j < width && src_data[j]; ++j) - ; + for (; j < width; ++j) { + if (atomicLoad(&src_data[j]) == 0) break; + } return j; } From d22128e265b3f2da6c8b52dfb6eb6a16f19d4a33 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Fri, 17 Jul 2026 13:57:44 +0200 Subject: [PATCH 2/3] Only use when thread sanitizer is on --- modules/imgproc/src/contours_truco.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/imgproc/src/contours_truco.cpp b/modules/imgproc/src/contours_truco.cpp index f41bf42abe..75f35877bb 100644 --- a/modules/imgproc/src/contours_truco.cpp +++ b/modules/imgproc/src/contours_truco.cpp @@ -13,20 +13,23 @@ namespace{ // zero or non-zero, so its termination conditions are unchanged. However, // concurrent read/write of different values is legally UB in C++ and triggers // TSAN. -static inline uchar atomicLoad(const uchar* ptr) { +inline uchar atomicLoad(const uchar* ptr) { +#ifdef CV_THREAD_SANITIZER if constexpr (sizeof(std::atomic) == sizeof(uchar)) { return reinterpret_cast*>(ptr)->load(std::memory_order_relaxed); - } else { - return *ptr; } +#endif + return *ptr; } -static inline void atomicStore(uchar* ptr, uchar val) { +inline void atomicStore(uchar* ptr, uchar val) { +#ifdef CV_THREAD_SANITIZER if constexpr (sizeof(std::atomic) == sizeof(uchar)) { reinterpret_cast*>(ptr)->store(val, std::memory_order_relaxed); - } else { - *ptr = val; + return; } +#endif + *ptr = val; } // Tunable block size. 1024 points = 8KB (Fits easily in L1 Cache) From 3924183f42a05cc853a7adffcf23a205245182c1 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Thu, 23 Jul 2026 15:16:18 +0200 Subject: [PATCH 3/3] Add more condition around SIMD code --- modules/imgproc/src/contours_truco.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/imgproc/src/contours_truco.cpp b/modules/imgproc/src/contours_truco.cpp index 75f35877bb..d2bd0c74ea 100644 --- a/modules/imgproc/src/contours_truco.cpp +++ b/modules/imgproc/src/contours_truco.cpp @@ -402,7 +402,7 @@ public: static inline int findStartContourPoint(uchar* src_data, int width, int j) { -#if (CV_SIMD || CV_SIMD_SCALABLE) +#if (CV_SIMD || CV_SIMD_SCALABLE) && !defined(CV_THREAD_SANITIZER) cv::v_uint8 v_zero = cv::vx_setzero_u8(); for (; j <= width - cv::VTraits::vlanes(); j += cv::VTraits::vlanes()) { @@ -423,7 +423,7 @@ public: inline static int findEndContourPoint(uchar* src_data,int width, int j) { -#if (CV_SIMD || CV_SIMD_SCALABLE) +#if (CV_SIMD || CV_SIMD_SCALABLE) && !defined(CV_THREAD_SANITIZER) if (j < width && !src_data[j]) { return j;