ThreadSanitizer: false report about data race

[Fedr]

The program as follows is valid:

#include <atomic>
#include <iostream>
#include <future>
    
int state = 0;
std::atomic_int a = 0;

void foo(int from, int to) 
{
    for (int i = 0; i < 10; i++)
    {
        while (a.load(std::memory_order_relaxed) != from) {}
        std::atomic_thread_fence(std::memory_order_acquire);
        state++;
        a.store(to, std::memory_order_release);
    }
}

int main()
{    
    auto x = std::async(std::launch::async, foo, 0, 1);
    auto y = std::async(std::launch::async, foo, 1, 0);
}

The access to state variable is not a data race, because each thread before the modification executes atomic_thread_fence to see the results of the other thread, and after the modification executes atomic store with memory_order_release. But the sanitizer erroneously reports data race. Demo: https://gcc.godbolt.org/z/9cY3aM3cz

Related discussion: https://stackoverflow.com/q/70542993/7325599

[dr-m]

I have a related question. How can I tell ThreadSanitizer that some std::atomic based synchronization primitives are similar to std::mutex and friends? I did not find any mention of an interface in the documentation, nor anything obvious in the header files in my system. I suspect that there is some magic in clang itself, but I did not find it in the source code.

To demonstrate this, you can apply the following patch to dr-m/atomic_sync@aea7471:

diff --git a/test/test_atomic_sync.cc b/test/test_atomic_sync.cc
index 6b53259..67c7ee6 100644
--- a/test/test_atomic_sync.cc
+++ b/test/test_atomic_sync.cc
@@ -8,7 +8,7 @@
 #include "transactional_lock_guard.h"
 #include "transactional_mutex_storage.h"
 
-static std::atomic<bool> critical;
+bool critical;
 
 constexpr unsigned N_THREADS = 30;
 constexpr unsigned N_ROUNDS = 100;
@@ -22,7 +22,7 @@ constexpr unsigned M_ROUNDS = 100;
 # define atomic_spin_shared_mutex atomic_shared_mutex
 # define atomic_spin_recursive_shared_mutex atomic_recursive_shared_mutex
 #endif
-static atomic_spin_mutex<transactional_mutex_storage> m;
+static std::mutex m;
 
 #if !defined WITH_ELISION || defined NDEBUG
 # define transactional_assert(x) assert(x)
@@ -143,12 +143,10 @@ int main(int, char **)
   fputs(ATOMIC_MUTEX_NAME(mutex), stderr);
 #endif
 
-  assert(!m.is_locked_or_waiting());
   for (auto i = N_THREADS; i--; )
     t[i]= std::thread(test_atomic_mutex);
   for (auto i = N_THREADS; i--; )
     t[i].join();
-  assert(!m.is_locked_or_waiting());
 
   fputs(", " ATOMIC_MUTEX_NAME(shared_mutex), stderr);
 

Compile and run as:

mkdir build
cd build
cmake -DCMAKE_CXX_COMPILER=clang++-15 -DCMAKE_CXX_FLAGS='-stdlib=libc++ -fsanitize=thread' ..
cmake --build .
test/test_atomic_sync

If you omit the second hunk, ThreadSanitizer will start to complain.

I can’t use std::mutex, because I need something as small as possible, and something whose ownership can be passed between threads (one thread reserves it and another releases it).

I assume that specially declaring a std::atomic based data type as a mutex (and some member functions as lock and unlock operations) would additionally allow ThreadSanitizer to report lock-order-inversion warnings (ReportTypeDeadlock).

[dr-m]

Sorry for the noise. I should have looked for <sanitizer/tsan_interface.h>. It seems to allow the instrumentation of homebrew mutexes and rw-locks, as well as working around the missing support for std::atomic_thread_fence().