Updated TSan suppression

Annotated some code against TSan warning

diff --git a/cds/compiler/feature_tsan.h b/cds/compiler/feature_tsan.h
index af780b979cd15e1c6b4ede740f4ac6c805536dbe..9852773e8f249b3fd33c21c538ecd6dac66bfc70 100644 (file)
--- a/cds/compiler/feature_tsan.h
+++ b/cds/compiler/feature_tsan.h
@@ -32,7 +32,7 @@
 #define CDSLIB_COMPILER_FEATURE_TSAN_H
 
 // Thread Sanitizer annotations.
 #define CDSLIB_COMPILER_FEATURE_TSAN_H
 
 // Thread Sanitizer annotations.

-// From https://groups.google.com/d/msg/thread-sanitizer/SsrHB7FTnTk/mNTGNLQj-9cJ
+// From http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/annotate_happens_before.cc?view=markup

 
 //@cond
 
 
 //@cond
 


@@ -52,6 +52,9 @@
                                                     CDS_TSAN_ANNOTATE_IGNORE_READS_END
 #   define CDS_TSAN_ANNOTATE_NEW_MEMORY( addr, sz ) AnnotateNewMemory( (char *) __FILE__, __LINE__, reinterpret_cast<void *>(addr), sz )
 
                                                     CDS_TSAN_ANNOTATE_IGNORE_READS_END
 #   define CDS_TSAN_ANNOTATE_NEW_MEMORY( addr, sz ) AnnotateNewMemory( (char *) __FILE__, __LINE__, reinterpret_cast<void *>(addr), sz )
 

+#   define CDS_TSAN_ANNOTATE_MUTEX_ACQUIRED( addr )  AnnotateRWLockAcquired( __FILE__, __LINE__, reinterpret_cast<void *>(addr), 1 )
+#   define CDS_TSAN_ANNOTATE_MUTEX_RELEASED( addr )  AnnotateRWLockReleased( __FILE__, __LINE__, reinterpret_cast<void *>(addr), 1 )
+

     // provided by TSan
     extern "C" {
         void AnnotateHappensBefore(const char *f, int l, void *addr);
     // provided by TSan
     extern "C" {
         void AnnotateHappensBefore(const char *f, int l, void *addr);


@@ -64,6 +67,8 @@
 
         void AnnotateNewMemory(char *f, int l, void * mem, size_t size);
 
 
         void AnnotateNewMemory(char *f, int l, void * mem, size_t size);
 

+        void AnnotateRWLockAcquired( const char *f, int l, void *m, long is_w );
+        void AnnotateRWLockReleased( const char *f, int l, void *m, long is_w );

     }
 
 #else // CDS_THREAD_SANITIZER_ENABLED
     }
 
 #else // CDS_THREAD_SANITIZER_ENABLED


@@ -80,6 +85,9 @@
 
 #   define CDS_TSAN_ANNOTATE_NEW_MEMORY( addr, sz )
 
 
 #   define CDS_TSAN_ANNOTATE_NEW_MEMORY( addr, sz )
 

+#   define CDS_TSAN_ANNOTATE_MUTEX_ACQUIRED( addr )
+#   define CDS_TSAN_ANNOTATE_MUTEX_RELEASED( addr )
+

 #endif
 
 //@endcond
 #endif
 
 //@endcond

diff --git a/cds/container/basket_queue.h b/cds/container/basket_queue.h
index 0192f483fe19a056508e2d008969e87c41bfda07..0a09a2c9ef8fc5fd51c44ad9cd309eafdc4fdd9e 100644 (file)
--- a/cds/container/basket_queue.h
+++ b/cds/container/basket_queue.h
@@ -397,7 +397,13 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
         */
         bool dequeue( value_type& dest )
         {

-            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
+            return dequeue_with( [&dest]( value_type& src ) { 
+                // TSan finds a race between this read of \p src and node_type constructor
+                // I think, it is wrong
+                CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN;
+                dest = std::move( src );
+                CDS_TSAN_ANNOTATE_IGNORE_READS_END;
+            });

         }
 
         /// Dequeues a value using a functor
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/moir_queue.h b/cds/container/moir_queue.h
index bf2d7729f135ebe8abb1d251eba20edec5982e41..5ed1222335e928cffbb49103c8f8bd70f5da2b2f 100644 (file)
--- a/cds/container/moir_queue.h
+++ b/cds/container/moir_queue.h
@@ -240,7 +240,13 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
         */
         bool dequeue( value_type& dest )
         {

-            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src ); });
+            return dequeue_with( [&dest]( value_type& src ) {
+                // TSan finds a race between this read of \p src and node_type constructor
+                // I think, it is wrong
+                CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN;
+                dest = std::move( src );
+                CDS_TSAN_ANNOTATE_IGNORE_READS_END;
+            });

         }
 
         /// Dequeues a value using a functor
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/container/msqueue.h b/cds/container/msqueue.h
index b79cff66732d9acb5915b51f3cf620976ef1140c..874a5cd038a23ed4cce1be3e92fdee11b0baa70f 100644 (file)
--- a/cds/container/msqueue.h
+++ b/cds/container/msqueue.h
@@ -355,7 +355,13 @@ namespace cds { namespace container {
         */
         bool dequeue( value_type& dest )
         {
         */
         bool dequeue( value_type& dest )
         {

-            return dequeue_with( [&dest]( value_type& src ) { dest = std::move( src );});
+            return dequeue_with( [&dest]( value_type& src ) { 
+                // TSan finds a race between this read of \p src and node_type constructor
+                // I think, it is wrong
+                CDS_TSAN_ANNOTATE_IGNORE_READS_BEGIN;
+                dest = std::move( src );
+                CDS_TSAN_ANNOTATE_IGNORE_READS_END;
+            });

         }
 
         /// Dequeues a value using a functor
         }
 
         /// Dequeues a value using a functor

diff --git a/cds/sync/spinlock.h b/cds/sync/spinlock.h
index ec0ee5cfbb6055f8d93d35af09c8e6a9924d6b05..32b75ea17831cb66412b8f481f63695d3ace230c 100644 (file)
--- a/cds/sync/spinlock.h
+++ b/cds/sync/spinlock.h
@@ -149,8 +149,10 @@ namespace cds {
             {
                 backoff_strategy backoff;
                 while ( nTryCount-- ) {
             {
                 backoff_strategy backoff;
                 while ( nTryCount-- ) {

-                    if ( try_lock())
+                    if ( try_lock() ) {
+                        CDS_TSAN_ANNOTATE_MUTEX_ACQUIRED( &m_spin );

                         return true;
                         return true;

+                    }

                     backoff();
                 }
                 return false;
                     backoff();
                 }
                 return false;


@@ -182,6 +184,7 @@ namespace cds {
                 CDS_DEBUG_ONLY( m_dbgOwnerId = OS::c_NullThreadId; )
 
                 m_spin.store( false, atomics::memory_order_release );
                 CDS_DEBUG_ONLY( m_dbgOwnerId = OS::c_NullThreadId; )
 
                 m_spin.store( false, atomics::memory_order_release );

+                CDS_TSAN_ANNOTATE_MUTEX_RELEASED( &m_spin );

             }
         };
 
             }
         };
 


@@ -246,8 +249,10 @@ namespace cds {
                 backoff_strategy bkoff;
 
                 while ( nTryCount-- ) {
                 backoff_strategy bkoff;
 
                 while ( nTryCount-- ) {

-                    if ( try_acquire())
+                    if ( try_acquire() ) {
+                        CDS_TSAN_ANNOTATE_MUTEX_ACQUIRED( &m_spin );

                         return true;
                         return true;

+                    }

                     bkoff();
                 }
                 return false;
                     bkoff();
                 }
                 return false;


@@ -347,6 +352,7 @@ namespace cds {
                     else {
                         free();
                         m_spin.store( 0, atomics::memory_order_release );
                     else {
                         free();
                         m_spin.store( 0, atomics::memory_order_release );

+                        CDS_TSAN_ANNOTATE_MUTEX_RELEASED( &m_spin );

                     }
                     return true;
                 }
                     }
                     return true;
                 }

diff --git a/cds/urcu/details/base.h b/cds/urcu/details/base.h
index 234526810cbec282f21899188898498131e9fea6..e05abed548ae46ea419ebeec7798f0e9a2bfca37 100644 (file)
--- a/cds/urcu/details/base.h
+++ b/cds/urcu/details/base.h
@@ -375,6 +375,7 @@ namespace cds {
                     thread_record * pOldHead = m_pHead.load( atomics::memory_order_acquire );
                     do {
                         pRec->m_list.m_pNext = pOldHead;
                     thread_record * pOldHead = m_pHead.load( atomics::memory_order_acquire );
                     do {
                         pRec->m_list.m_pNext = pOldHead;

+                        CDS_TSAN_ANNOTATE_HAPPENS_BEFORE( &( pRec->m_list.m_pNext ));

                     } while ( !m_pHead.compare_exchange_weak( pOldHead, pRec, atomics::memory_order_release, atomics::memory_order_relaxed ));
 
                     return pRec;
                     } while ( !m_pHead.compare_exchange_weak( pOldHead, pRec, atomics::memory_order_release, atomics::memory_order_relaxed ));
 
                     return pRec;

diff --git a/src/hp_gc.cpp b/src/hp_gc.cpp
index eb667c2fec5eb9dc6e0bbfa0f0cbeebb106e9b7c..15c675ca6fd2b8fc5ae3941a1378b60328dcf39d 100644 (file)
--- a/src/hp_gc.cpp
+++ b/src/hp_gc.cpp
@@ -151,6 +151,7 @@ namespace cds { namespace gc {
             hplist_node * pOldHead = m_pListHead.load( atomics::memory_order_acquire );
             do {
                 hprec->m_pNextNode = pOldHead;
             hplist_node * pOldHead = m_pListHead.load( atomics::memory_order_acquire );
             do {
                 hprec->m_pNextNode = pOldHead;

+                CDS_TSAN_ANNOTATE_HAPPENS_BEFORE( &( hprec->m_pNextNode ));

             } while ( !m_pListHead.compare_exchange_weak( pOldHead, hprec, atomics::memory_order_release, atomics::memory_order_relaxed ));
 
             return hprec;
             } while ( !m_pListHead.compare_exchange_weak( pOldHead, hprec, atomics::memory_order_release, atomics::memory_order_relaxed ));
 
             return hprec;

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 7cf4875906b2a089c42d2f164877e31c1e6de9f5..d27c6f51a60e0f99e59d06a1a08901a3beb375a0 100644 (file)
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -13,3 +13,6 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DGTEST_LANG_CXX11")
  
 add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/unit)
 add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/stress)
  
 add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/unit)
 add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/stress)

+
+file(GLOB SANITIZER_OPTION_FILES ${PROJECT_SOURCE_DIR}/tools/tsan-suppression)
+file(COPY ${SANITIZER_OPTION_FILES} DESTINATION ${EXECUTABLE_OUTPUT_PATH})

diff --git a/test/stress/freelist/put_get.cpp b/test/stress/freelist/put_get.cpp
index 6a51563eec1cfac8e69ab1d835af8cc133cdcd93..59f6878a27e4c57d52dbc84297721b6886dc20ed 100644 (file)
--- a/test/stress/freelist/put_get.cpp
+++ b/test/stress/freelist/put_get.cpp
@@ -88,7 +88,9 @@ namespace {
                     item_type* p;
 
                     while ( (p = static_cast<item_type*>( m_FreeList.get())) != nullptr ) {
                     item_type* p;
 
                     while ( (p = static_cast<item_type*>( m_FreeList.get())) != nullptr ) {

+                        CDS_TSAN_ANNOTATE_IGNORE_RW_BEGIN;

                         p->counter++;
                         p->counter++;

+                        CDS_TSAN_ANNOTATE_IGNORE_RW_END;

                         arr[n] = p;
                         ++m_nSuccess;
                         ++n;
                         arr[n] = p;
                         ++m_nSuccess;
                         ++n;

diff --git a/tools/tsan-suppression b/tools/tsan-suppression
index 8d8c46d5ffda50aea958344af680382b1289db03..eb2c54211ce40a826711bc691ad4c53bb392ef1a 100644 (file)
--- a/tools/tsan-suppression
+++ b/tools/tsan-suppression
@@ -4,14 +4,21 @@
 #   verosity=n Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).
 #   history_size=[0..7], default 2
 
 #   verosity=n Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).
 #   history_size=[0..7], default 2
 

-# DHP 
-#race:cds::gc::details::retired_ptr::free
+# false: LazyList potential deadlock
+deadlock:cds/intrusive/impl/lazy_list.h

 
 

-# uRCU false positive
-#race:cds::urcu::gc*::batch_retire*
+# false: BronsonAVLTree potential deadlock
+deadlock:cds/container/impl/bronson_avltree_map_rcu.h

 
 

-# EllenBinTree false positive
-#race:ellen_bintree_pool::internal_node_allocator*::allocate
+#TODO: temporary suppressed. Must be researched later
+race:cds/container/impl/bronson_avltree_map_rcu.h

 
 

-# TODO: TSan false positive or library issues?
-#race:cds::container::OptimisticQueue*::alloc_node
+#TODO: MSPriorityQueue - temporary suppressed. Must be researched later
+# Seems, TSan don't see spinlock blocking. How to learn TSan to see non-traditional locking algo?..
+race:cds::intrusive::MSPriorityQueue
+
+#TODO: gc::DHP must be reimplemented ASAP
+race:cds::gc::dhp::GarbageCollector::scan
+
+#TODO: temporary suppressed. Must be researched later
+race:cds::memory::michael::Heap
\ No newline at end of file