[llvm-branch-commits] [compiler-rt] 96313d2 - [TSan] Optimize handling of racy address
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Jul 20 06:06:32 PDT 2020
Author: Joachim Protze
Date: 2020-07-20T15:02:27+02:00
New Revision: 96313d2de45ace49d40606dda71f03396f13ddef
URL: https://github.com/llvm/llvm-project/commit/96313d2de45ace49d40606dda71f03396f13ddef
DIFF: https://github.com/llvm/llvm-project/commit/96313d2de45ace49d40606dda71f03396f13ddef.diff
LOG: [TSan] Optimize handling of racy address
This patch splits the handling of racy address and racy stack into separate
functions. If a race was already reported for the address, we can avoid the
cost for collecting the involved stacks.
This patch also removes the race condition in storing the racy address / racy
stack. This race condition allowed all threads to report the race.
This patch changes the transitive suppression of reports. Previously
suppression could transitively chain memory location and racy stacks.
Now racy memory and racy stack are separate suppressions.
Commit again, now with fixed tests.
Reviewed by: dvyukov
Differential Revision: https://reviews.llvm.org/D83625
(cherry picked from commit 7358a1104a02d5f5e645ebff0530787453ae98da)
Added:
Modified:
compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
compiler-rt/lib/tsan/tests/rtl/tsan_test_util_posix.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
index 949beac1c551..3354546c2a10 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp
@@ -439,65 +439,61 @@ void RestoreStack(int tid, const u64 epoch, VarSizeStackTrace *stk,
ExtractTagFromStack(stk, tag);
}
-static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2],
- uptr addr_min, uptr addr_max) {
- bool equal_stack = false;
- RacyStacks hash;
- bool equal_address = false;
- RacyAddress ra0 = {addr_min, addr_max};
- {
- ReadLock lock(&ctx->racy_mtx);
- if (flags()->suppress_equal_stacks) {
- hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
- hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
- for (uptr i = 0; i < ctx->racy_stacks.Size(); i++) {
- if (hash == ctx->racy_stacks[i]) {
- VPrintf(2,
- "ThreadSanitizer: suppressing report as doubled (stack)\n");
- equal_stack = true;
- break;
- }
- }
- }
- if (flags()->suppress_equal_addresses) {
- for (uptr i = 0; i < ctx->racy_addresses.Size(); i++) {
- RacyAddress ra2 = ctx->racy_addresses[i];
- uptr maxbeg = max(ra0.addr_min, ra2.addr_min);
- uptr minend = min(ra0.addr_max, ra2.addr_max);
- if (maxbeg < minend) {
- VPrintf(2, "ThreadSanitizer: suppressing report as doubled (addr)\n");
- equal_address = true;
- break;
- }
- }
+static bool FindRacyStacks(const RacyStacks &hash) {
+ for (uptr i = 0; i < ctx->racy_stacks.Size(); i++) {
+ if (hash == ctx->racy_stacks[i]) {
+ VPrintf(2, "ThreadSanitizer: suppressing report as doubled (stack)\n");
+ return true;
}
}
- if (!equal_stack && !equal_address)
+ return false;
+}
+
+static bool HandleRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2]) {
+ if (!flags()->suppress_equal_stacks)
return false;
- if (!equal_stack) {
- Lock lock(&ctx->racy_mtx);
- ctx->racy_stacks.PushBack(hash);
- }
- if (!equal_address) {
- Lock lock(&ctx->racy_mtx);
- ctx->racy_addresses.PushBack(ra0);
+ RacyStacks hash;
+ hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
+ hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
+ {
+ ReadLock lock(&ctx->racy_mtx);
+ if (FindRacyStacks(hash))
+ return true;
}
- return true;
+ Lock lock(&ctx->racy_mtx);
+ if (FindRacyStacks(hash))
+ return true;
+ ctx->racy_stacks.PushBack(hash);
+ return false;
}
-static void AddRacyStacks(ThreadState *thr, VarSizeStackTrace traces[2],
- uptr addr_min, uptr addr_max) {
- Lock lock(&ctx->racy_mtx);
- if (flags()->suppress_equal_stacks) {
- RacyStacks hash;
- hash.hash[0] = md5_hash(traces[0].trace, traces[0].size * sizeof(uptr));
- hash.hash[1] = md5_hash(traces[1].trace, traces[1].size * sizeof(uptr));
- ctx->racy_stacks.PushBack(hash);
+static bool FindRacyAddress(const RacyAddress &ra0) {
+ for (uptr i = 0; i < ctx->racy_addresses.Size(); i++) {
+ RacyAddress ra2 = ctx->racy_addresses[i];
+ uptr maxbeg = max(ra0.addr_min, ra2.addr_min);
+ uptr minend = min(ra0.addr_max, ra2.addr_max);
+ if (maxbeg < minend) {
+ VPrintf(2, "ThreadSanitizer: suppressing report as doubled (addr)\n");
+ return true;
+ }
}
- if (flags()->suppress_equal_addresses) {
- RacyAddress ra0 = {addr_min, addr_max};
- ctx->racy_addresses.PushBack(ra0);
+ return false;
+}
+
+static bool HandleRacyAddress(ThreadState *thr, uptr addr_min, uptr addr_max) {
+ if (!flags()->suppress_equal_addresses)
+ return false;
+ RacyAddress ra0 = {addr_min, addr_max};
+ {
+ ReadLock lock(&ctx->racy_mtx);
+ if (FindRacyAddress(ra0))
+ return true;
}
+ Lock lock(&ctx->racy_mtx);
+ if (FindRacyAddress(ra0))
+ return true;
+ ctx->racy_addresses.PushBack(ra0);
+ return false;
}
bool OutputReport(ThreadState *thr, const ScopedReport &srep) {
@@ -618,6 +614,8 @@ void ReportRace(ThreadState *thr) {
if (IsExpectedReport(addr_min, addr_max - addr_min))
return;
}
+ if (HandleRacyAddress(thr, addr_min, addr_max))
+ return;
ReportType typ = ReportTypeRace;
if (thr->is_vptr_access && freed)
@@ -668,7 +666,7 @@ void ReportRace(ThreadState *thr) {
if (IsFiredSuppression(ctx, typ, traces[1]))
return;
- if (HandleRacyStacks(thr, traces, addr_min, addr_max))
+ if (HandleRacyStacks(thr, traces))
return;
// If any of the accesses has a tag, treat this as an "external" race.
@@ -711,7 +709,6 @@ void ReportRace(ThreadState *thr) {
if (!OutputReport(thr, rep))
return;
- AddRacyStacks(thr, traces, addr_min, addr_max);
}
void PrintCurrentStack(ThreadState *thr, uptr pc) {
diff --git a/compiler-rt/lib/tsan/tests/rtl/tsan_test_util_posix.cpp b/compiler-rt/lib/tsan/tests/rtl/tsan_test_util_posix.cpp
index a24d04f47007..733e5d282a37 100644
--- a/compiler-rt/lib/tsan/tests/rtl/tsan_test_util_posix.cpp
+++ b/compiler-rt/lib/tsan/tests/rtl/tsan_test_util_posix.cpp
@@ -27,6 +27,8 @@
#include <unistd.h>
#include <errno.h>
+#define CALLERPC (__builtin_return_address(0))
+
using namespace __tsan;
static __thread bool expect_report;
@@ -249,22 +251,42 @@ void ScopedThread::Impl::HandleEvent(Event *ev) {
switch (ev->type) {
case Event::READ:
case Event::WRITE: {
- void (*tsan_mop)(void *addr) = 0;
+ void (*tsan_mop)(void *addr, void *pc) = 0;
if (ev->type == Event::READ) {
switch (ev->arg /*size*/) {
- case 1: tsan_mop = __tsan_read1; break;
- case 2: tsan_mop = __tsan_read2; break;
- case 4: tsan_mop = __tsan_read4; break;
- case 8: tsan_mop = __tsan_read8; break;
- case 16: tsan_mop = __tsan_read16; break;
+ case 1:
+ tsan_mop = __tsan_read1_pc;
+ break;
+ case 2:
+ tsan_mop = __tsan_read2_pc;
+ break;
+ case 4:
+ tsan_mop = __tsan_read4_pc;
+ break;
+ case 8:
+ tsan_mop = __tsan_read8_pc;
+ break;
+ case 16:
+ tsan_mop = __tsan_read16_pc;
+ break;
}
} else {
switch (ev->arg /*size*/) {
- case 1: tsan_mop = __tsan_write1; break;
- case 2: tsan_mop = __tsan_write2; break;
- case 4: tsan_mop = __tsan_write4; break;
- case 8: tsan_mop = __tsan_write8; break;
- case 16: tsan_mop = __tsan_write16; break;
+ case 1:
+ tsan_mop = __tsan_write1_pc;
+ break;
+ case 2:
+ tsan_mop = __tsan_write2_pc;
+ break;
+ case 4:
+ tsan_mop = __tsan_write4_pc;
+ break;
+ case 8:
+ tsan_mop = __tsan_write8_pc;
+ break;
+ case 16:
+ tsan_mop = __tsan_write16_pc;
+ break;
}
}
CHECK_NE(tsan_mop, 0);
@@ -274,7 +296,7 @@ void ScopedThread::Impl::HandleEvent(Event *ev) {
const int ErrCode = ECHRNG;
#endif
errno = ErrCode;
- tsan_mop(ev->ptr);
+ tsan_mop(ev->ptr, (void *)ev->arg2);
CHECK_EQ(ErrCode, errno); // In no case must errno be changed.
break;
}
@@ -327,7 +349,7 @@ void ScopedThread::Impl::HandleEvent(Event *ev) {
}
void *ScopedThread::Impl::ScopedThreadCallback(void *arg) {
- __tsan_func_entry(__builtin_return_address(0));
+ __tsan_func_entry(CALLERPC);
Impl *impl = (Impl*)arg;
for (;;) {
Event* ev = (Event*)atomic_load(&impl->event, memory_order_acquire);
@@ -392,7 +414,8 @@ void ScopedThread::Detach() {
void ScopedThread::Access(void *addr, bool is_write,
int size, bool expect_race) {
- Event event(is_write ? Event::WRITE : Event::READ, addr, size);
+ Event event(is_write ? Event::WRITE : Event::READ, addr, size,
+ (uptr)CALLERPC);
if (expect_race)
event.ExpectReport(ReportTypeRace);
impl_->send(&event);
More information about the llvm-branch-commits
mailing list