diff options
| -rw-r--r-- | lib/tsan/rtl/tsan_rtl.h | 4 | ||||
| -rw-r--r-- | lib/tsan/rtl/tsan_rtl_report.cc | 26 | ||||
| -rw-r--r-- | lib/tsan/rtl/tsan_trace.h | 2 | ||||
| -rw-r--r-- | test/tsan/double_race.cc | 52 |
4 files changed, 79 insertions, 5 deletions
diff --git a/lib/tsan/rtl/tsan_rtl.h b/lib/tsan/rtl/tsan_rtl.h index 2cf2e1684..99c4d2529 100644 --- a/lib/tsan/rtl/tsan_rtl.h +++ b/lib/tsan/rtl/tsan_rtl.h @@ -825,7 +825,7 @@ void ALWAYS_INLINE TraceAddEvent(ThreadState *thr, FastState fs, return; DCHECK_GE((int)typ, 0); DCHECK_LE((int)typ, 7); - DCHECK_EQ(GetLsb(addr, 61), addr); + DCHECK_EQ(GetLsb(addr, kEventPCBits), addr); StatInc(thr, StatEvents); u64 pos = fs.GetTracePos(); if (UNLIKELY((pos % kTracePartSize) == 0)) { @@ -837,7 +837,7 @@ void ALWAYS_INLINE TraceAddEvent(ThreadState *thr, FastState fs, } Event *trace = (Event*)GetThreadTrace(fs.tid()); Event *evp = &trace[pos]; - Event ev = (u64)addr | ((u64)typ << 61); + Event ev = (u64)addr | ((u64)typ << kEventPCBits); *evp = ev; } diff --git a/lib/tsan/rtl/tsan_rtl_report.cc b/lib/tsan/rtl/tsan_rtl_report.cc index 85a982941..c1d2cc4b5 100644 --- a/lib/tsan/rtl/tsan_rtl_report.cc +++ b/lib/tsan/rtl/tsan_rtl_report.cc @@ -406,8 +406,8 @@ void RestoreStack(int tid, const u64 epoch, VarSizeStackTrace *stk, Event *events = (Event*)GetThreadTrace(tid); for (uptr i = ebegin; i <= eend; i++) { Event ev = events[i]; - EventType typ = (EventType)(ev >> 61); - uptr pc = (uptr)(ev & ((1ull << 61) - 1)); + EventType typ = (EventType)(ev >> kEventPCBits); + uptr pc = (uptr)(ev & ((1ull << kEventPCBits) - 1)); DPrintf2(" %zu typ=%d pc=%zx\n", i, typ, pc); if (typ == EventTypeMop) { stack[pos] = pc; @@ -634,7 +634,27 @@ void ReportRace(ThreadState *thr) { const uptr kMop = 2; VarSizeStackTrace traces[kMop]; uptr tags[kMop] = {kExternalTagNone}; - const uptr toppc = TraceTopPC(thr); + uptr toppc = TraceTopPC(thr); + if (toppc >> kEventPCBits) { + // This is a work-around for a known issue. + // The scenario where this happens is rather elaborate and requires + // an instrumented __sanitizer_report_error_summary callback and + // a __tsan_symbolize_external callback and a race during a range memory + // access larger than 8 bytes. MemoryAccessRange adds the current PC to + // the trace and starts processing memory accesses. A first memory access + // triggers a race, we report it and call the instrumented + // __sanitizer_report_error_summary, which adds more stuff to the trace + // since it is intrumented. Then a second memory access in MemoryAccessRange + // also triggers a race and we get here and call TraceTopPC to get the + // current PC, however now it contains some unrelated events from the + // callback. Most likely, TraceTopPC will now return a EventTypeFuncExit + // event. Later we subtract -1 from it (in GetPreviousInstructionPc) + // and the resulting PC has kExternalPCBit set, so we pass it to + // __tsan_symbolize_external. __tsan_symbolize_external is within its rights + // to crash since the PC is completely bogus. + // test/tsan/double_race.cc contains a test case for this. + toppc = 0; + } ObtainCurrentStack(thr, toppc, &traces[0], &tags[0]); if (IsFiredSuppression(ctx, typ, traces[0])) return; diff --git a/lib/tsan/rtl/tsan_trace.h b/lib/tsan/rtl/tsan_trace.h index 96a18ac41..9aef375cb 100644 --- a/lib/tsan/rtl/tsan_trace.h +++ b/lib/tsan/rtl/tsan_trace.h @@ -41,6 +41,8 @@ enum EventType { // u64 addr : 61; // Associated pc. typedef u64 Event; +const uptr kEventPCBits = 61; + struct TraceHeader { #if !SANITIZER_GO BufferedStackTrace stack0; // Start stack for the trace. diff --git a/test/tsan/double_race.cc b/test/tsan/double_race.cc new file mode 100644 index 000000000..76b017428 --- /dev/null +++ b/test/tsan/double_race.cc @@ -0,0 +1,52 @@ +// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t 2>&1 | FileCheck %s +#include "test.h" +#include <memory.h> + +// A reproducer for a known issue. +// See reference to double_race.cc in tsan_rtl_report.cc for an explanation. + +char buf[16]; +volatile int nreport; + +void __sanitizer_report_error_summary(const char *summary) { + nreport++; +} + +const int kEventPCBits = 61; + +extern "C" bool __tsan_symbolize_external(unsigned long pc, char *func_buf, + unsigned long func_siz, + char *file_buf, + unsigned long file_siz, int *line, + int *col) { + if (pc >> kEventPCBits) { + printf("bad PC passed to __tsan_symbolize_external: %lx\n", pc); + _exit(1); + } + return true; +} + +void *Thread(void *arg) { + barrier_wait(&barrier); + memset(buf, 2, sizeof(buf)); + return 0; +} + +int main() { + barrier_init(&barrier, 2); + pthread_t t; + pthread_create(&t, 0, Thread, 0); + memset(buf, 1, sizeof(buf)); + barrier_wait(&barrier); + pthread_join(t, 0); + return 0; +} + +// CHECK: WARNING: ThreadSanitizer: data race +// CHECK: Write of size 8 at {{.*}} by thread T1: +// CHECK: #0 memset +// CHECK: #1 Thread +// CHECK-NOT: bad PC passed to __tsan_symbolize_external +// CHECK: WARNING: ThreadSanitizer: data race +// CHECK: Write of size 8 at {{.*}} by thread T1: +// CHECK: #0 Thread |