[compiler-rt] r311768 - tsan: don't pass bogus PCs to __tsan_symbolize_external
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 25 07:53:41 PDT 2017
It looks like this commit broke the green dragon bot. Could you please revert and investigate?
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/34882/consoleFull#7286766538254eaf0-7326-4999-85b0-388101f2d404
thanks,
adrian
> On Aug 25, 2017, at 1:52 AM, Dmitry Vyukov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: dvyukov
> Date: Fri Aug 25 01:52:28 2017
> New Revision: 311768
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311768&view=rev
> Log:
> tsan: don't pass bogus PCs to __tsan_symbolize_external
>
> See the added comment for an explanation.
>
> Reviewed in https://reviews.llvm.org/D37107
>
>
> Added:
> compiler-rt/trunk/test/tsan/double_race.cc
> Modified:
> compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h
> compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
> compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h?rev=311768&r1=311767&r2=311768&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h Fri Aug 25 01:52:28 2017
> @@ -825,7 +825,7 @@ void ALWAYS_INLINE TraceAddEvent(ThreadS
> 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(ThreadS
> }
> 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;
> }
>
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc?rev=311768&r1=311767&r2=311768&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc Fri Aug 25 01:52:28 2017
> @@ -406,8 +406,8 @@ void RestoreStack(int tid, const u64 epo
> 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;
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h?rev=311768&r1=311767&r2=311768&view=diff
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_trace.h Fri Aug 25 01:52:28 2017
> @@ -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.
>
> Added: compiler-rt/trunk/test/tsan/double_race.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/double_race.cc?rev=311768&view=auto
> ==============================================================================
> --- compiler-rt/trunk/test/tsan/double_race.cc (added)
> +++ compiler-rt/trunk/test/tsan/double_race.cc Fri Aug 25 01:52:28 2017
> @@ -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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list