From 87c6bb97162aaa9a9124f2af7dda411bac6f75fd Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 1 Feb 2013 14:41:58 +0000 Subject: [PATCH] tsan: even if races between atomic and plain memory accesses are turned off (report_atomic_races=0), still report races between atomic accesses and free(). llvm-svn: 174175 --- compiler-rt/lib/tsan/lit_tests/atomic_free.cc | 19 ++++++++++++++++++ compiler-rt/lib/tsan/lit_tests/atomic_free2.cc | 19 ++++++++++++++++++ compiler-rt/lib/tsan/lit_tests/race_on_mutex2.c | 24 +++++++++++++++++++++++ compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cc | 15 +++++--------- compiler-rt/lib/tsan/rtl/tsan_rtl.cc | 3 +++ compiler-rt/lib/tsan/rtl/tsan_rtl.h | 5 +++++ compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc | 12 ++++++++++-- compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc | 16 +++++++++++++++ 8 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 compiler-rt/lib/tsan/lit_tests/atomic_free.cc create mode 100644 compiler-rt/lib/tsan/lit_tests/atomic_free2.cc create mode 100644 compiler-rt/lib/tsan/lit_tests/race_on_mutex2.c diff --git a/compiler-rt/lib/tsan/lit_tests/atomic_free.cc b/compiler-rt/lib/tsan/lit_tests/atomic_free.cc new file mode 100644 index 0000000..ba9bd5a --- /dev/null +++ b/compiler-rt/lib/tsan/lit_tests/atomic_free.cc @@ -0,0 +1,19 @@ +// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s +#include +#include + +void *Thread(void *a) { + __atomic_fetch_add((int*)a, 1, __ATOMIC_SEQ_CST); + return 0; +} + +int main() { + int *a = new int(0); + pthread_t t; + pthread_create(&t, 0, Thread, a); + sleep(1); + delete a; + pthread_join(t, 0); +} + +// CHECK: WARNING: ThreadSanitizer: data race diff --git a/compiler-rt/lib/tsan/lit_tests/atomic_free2.cc b/compiler-rt/lib/tsan/lit_tests/atomic_free2.cc new file mode 100644 index 0000000..5517bf7 --- /dev/null +++ b/compiler-rt/lib/tsan/lit_tests/atomic_free2.cc @@ -0,0 +1,19 @@ +// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s +#include +#include + +void *Thread(void *a) { + sleep(1); + __atomic_fetch_add((int*)a, 1, __ATOMIC_SEQ_CST); + return 0; +} + +int main() { + int *a = new int(0); + pthread_t t; + pthread_create(&t, 0, Thread, a); + delete a; + pthread_join(t, 0); +} + +// CHECK: WARNING: ThreadSanitizer: heap-use-after-free diff --git a/compiler-rt/lib/tsan/lit_tests/race_on_mutex2.c b/compiler-rt/lib/tsan/lit_tests/race_on_mutex2.c new file mode 100644 index 0000000..84bef75 --- /dev/null +++ b/compiler-rt/lib/tsan/lit_tests/race_on_mutex2.c @@ -0,0 +1,24 @@ +// RUN: %clang_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s +#include +#include +#include +#include + +void *Thread(void *x) { + pthread_mutex_lock((pthread_mutex_t*)x); + pthread_mutex_unlock((pthread_mutex_t*)x); + return 0; +} + +int main() { + pthread_mutex_t Mtx; + pthread_mutex_init(&Mtx, 0); + pthread_t t; + pthread_create(&t, 0, Thread, &Mtx); + sleep(1); + pthread_mutex_destroy(&Mtx); + pthread_join(t, 0); + return 0; +} + +// CHECK: WARNING: ThreadSanitizer: data race diff --git a/compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cc b/compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cc index e39160b..a2f7ff4 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cc @@ -250,8 +250,7 @@ static T AtomicLoad(ThreadState *thr, uptr pc, const volatile T *a, // This fast-path is critical for performance. // Assume the access is atomic. if (!IsAcquireOrder(mo) && sizeof(T) <= sizeof(a)) { - if (flags()->report_atomic_races) - MemoryReadAtomic(thr, pc, (uptr)a, SizeLog()); + MemoryReadAtomic(thr, pc, (uptr)a, SizeLog()); return *a; } SyncVar *s = CTX()->synctab.GetOrCreateAndLock(thr, pc, (uptr)a, false); @@ -260,8 +259,7 @@ static T AtomicLoad(ThreadState *thr, uptr pc, const volatile T *a, T v = *a; s->mtx.ReadUnlock(); __sync_synchronize(); - if (flags()->report_atomic_races) - MemoryReadAtomic(thr, pc, (uptr)a, SizeLog()); + MemoryReadAtomic(thr, pc, (uptr)a, SizeLog()); return v; } @@ -269,8 +267,7 @@ template static void AtomicStore(ThreadState *thr, uptr pc, volatile T *a, T v, morder mo) { CHECK(IsStoreOrder(mo)); - if (flags()->report_atomic_races) - MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); + MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); // This fast-path is critical for performance. // Assume the access is atomic. // Strictly saying even relaxed store cuts off release sequence, @@ -292,8 +289,7 @@ static void AtomicStore(ThreadState *thr, uptr pc, volatile T *a, T v, template static T AtomicRMW(ThreadState *thr, uptr pc, volatile T *a, T v, morder mo) { - if (flags()->report_atomic_races) - MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); + MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); SyncVar *s = CTX()->synctab.GetOrCreateAndLock(thr, pc, (uptr)a, true); thr->clock.set(thr->tid, thr->fast_state.epoch()); if (IsAcqRelOrder(mo)) @@ -353,8 +349,7 @@ template static bool AtomicCAS(ThreadState *thr, uptr pc, volatile T *a, T *c, T v, morder mo, morder fmo) { (void)fmo; // Unused because llvm does not pass it yet. - if (flags()->report_atomic_races) - MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); + MemoryWriteAtomic(thr, pc, (uptr)a, SizeLog()); SyncVar *s = CTX()->synctab.GetOrCreateAndLock(thr, pc, (uptr)a, true); thr->clock.set(thr->tid, thr->fast_state.epoch()); if (IsAcqRelOrder(mo)) diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc index 855d645..cb44fba 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc @@ -535,7 +535,10 @@ void MemoryResetRange(ThreadState *thr, uptr pc, uptr addr, uptr size) { } void MemoryRangeFreed(ThreadState *thr, uptr pc, uptr addr, uptr size) { + CHECK_EQ(thr->is_freeing, false); + thr->is_freeing = true; MemoryAccessRange(thr, pc, addr, size, true); + thr->is_freeing = false; Shadow s(thr->fast_state); s.ClearIgnoreBit(); s.MarkAsFreed(); diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h index 40743fa..54d1671 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h @@ -280,6 +280,10 @@ class Shadow : public FastState { x_ |= kFreedBit; } + bool IsFreed() const { + return x_ & kFreedBit; + } + bool GetFreedAndReset() { bool res = x_ & kFreedBit; x_ &= ~kFreedBit; @@ -372,6 +376,7 @@ struct ThreadState { int in_rtl; bool in_symbolizer; bool is_alive; + bool is_freeing; const uptr stk_addr; const uptr stk_size; const uptr tls_addr; diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc index 6c86b06..a07f6a2 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc @@ -26,8 +26,12 @@ void MutexCreate(ThreadState *thr, uptr pc, uptr addr, CHECK_GT(thr->in_rtl, 0); DPrintf("#%d: MutexCreate %zx\n", thr->tid, addr); StatInc(thr, StatMutexCreate); - if (!linker_init && IsAppMem(addr)) + if (!linker_init && IsAppMem(addr)) { + CHECK(!thr->is_freeing); + thr->is_freeing = true; MemoryWrite(thr, pc, addr, kSizeLog1); + thr->is_freeing = false; + } SyncVar *s = ctx->synctab.GetOrCreateAndLock(thr, pc, addr, true); s->is_rw = rw; s->is_recursive = recursive; @@ -49,8 +53,12 @@ void MutexDestroy(ThreadState *thr, uptr pc, uptr addr) { SyncVar *s = ctx->synctab.GetAndRemove(thr, pc, addr); if (s == 0) return; - if (IsAppMem(addr)) + if (IsAppMem(addr)) { + CHECK(!thr->is_freeing); + thr->is_freeing = true; MemoryWrite(thr, pc, addr, kSizeLog1); + thr->is_freeing = false; + } if (flags()->report_destroy_locked && s->owner_tid != SyncVar::kInvalidTid && !s->is_broken) { diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc index ba9b0ad..5fb8a6e 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc @@ -531,11 +531,27 @@ static bool IsJavaNonsense(const ReportDesc *rep) { return false; } +static bool RaceBetweenAtomicAndFree(ThreadState *thr) { + Shadow s0(thr->racy_state[0]); + Shadow s1(thr->racy_state[1]); + CHECK(!(s0.IsAtomic() && s1.IsAtomic())); + if (!s0.IsAtomic() && !s1.IsAtomic()) + return true; + if (s0.IsAtomic() && s1.IsFreed()) + return true; + if (s1.IsAtomic() && thr->is_freeing) + return true; + return false; +} + void ReportRace(ThreadState *thr) { if (!flags()->report_bugs) return; ScopedInRtl in_rtl; + if (!flags()->report_atomic_races && !RaceBetweenAtomicAndFree(thr)) + return; + if (thr->in_signal_handler) Printf("ThreadSanitizer: printing report from signal handler." " Can crash or hang.\n"); -- 2.7.4