From c9d435989f96d1a1f2826adf2c66169c3ca6865e Mon Sep 17 00:00:00 2001 From: Dave Clausen Date: Mon, 11 Mar 2024 17:45:48 -0400 Subject: [PATCH 1/2] Fix race in the implementation of __tsan_acquire() which can cause false positive data race warnings when the atomic variable was previously written to using compare_exchange_strong/weak. --- compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp | 2 +- .../tsan/compare_exchange_acquire_fence.cpp | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp index 2e978852ea7d3..2a8aa1915c9ae 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cpp @@ -446,9 +446,9 @@ void Acquire(ThreadState *thr, uptr pc, uptr addr) { if (!s) return; SlotLocker locker(thr); + ReadLock lock(&s->mtx); if (!s->clock) return; - ReadLock lock(&s->mtx); thr->clock.Acquire(s->clock); } diff --git a/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp b/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp new file mode 100644 index 0000000000000..12bab80f14bfb --- /dev/null +++ b/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp @@ -0,0 +1,46 @@ +// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s +// This is a correct program and tsan should not report a race. +// +// Verify that there is a happens-before relationship between a +// memory_order_release store that happens as part of a successful +// compare_exchange_strong(), and an atomic_thread_fence(memory_order_acquire) +// that happens after a relaxed load. + +#include +#include +#include +#include +#include + +std::atomic a; +unsigned int b; +constexpr int loops = 100000; + +void Thread1() { + for (int i = 0; i < loops; ++i) { + while (a.load(std::memory_order_acquire)) { + } + b = i; + bool expected = false; + a.compare_exchange_strong(expected, true, std::memory_order_acq_rel); + } +} + +int main() { + std::thread t(Thread1); + unsigned int sum = 0; + for (int i = 0; i < loops; ++i) { + while (!a.load(std::memory_order_relaxed)) { + } + std::atomic_thread_fence(std::memory_order_acquire); + __tsan_acquire(&a); + sum += b; + a.store(false, std::memory_order_release); + } + t.join(); + fprintf(stderr, "DONE: %u\n", sum); + return 0; +} + +// CHECK: DONE +// CHECK-NOT: ThreadSanitizer: data race From c6f74e0e9b1d504045a26e56fada5f4b9b6f0d2b Mon Sep 17 00:00:00 2001 From: Dave Clausen Date: Tue, 12 Mar 2024 14:04:48 -0400 Subject: [PATCH 2/2] Remove FileCheck from the test commandline --- compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp b/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp index 12bab80f14bfb..b9fd0c5ad21f2 100644 --- a/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp +++ b/compiler-rt/test/tsan/compare_exchange_acquire_fence.cpp @@ -1,4 +1,4 @@ -// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s +// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 // This is a correct program and tsan should not report a race. // // Verify that there is a happens-before relationship between a @@ -41,6 +41,3 @@ int main() { fprintf(stderr, "DONE: %u\n", sum); return 0; } - -// CHECK: DONE -// CHECK-NOT: ThreadSanitizer: data race