From 89d7db9d8128830975e1318d954db4ed4e484d13 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 19 Aug 2022 04:01:42 +0000 Subject: [PATCH] Revert "[tsan] Keep thread/stack for closed FD" Test is flaky. This reverts commit e9c5bde88ea2e35fadb15c5e5a1d2cb583fd0772. --- compiler-rt/lib/tsan/rtl/tsan_fd.cpp | 10 +++------- compiler-rt/lib/tsan/rtl/tsan_fd.h | 2 +- compiler-rt/lib/tsan/rtl/tsan_report.cpp | 5 ++--- compiler-rt/lib/tsan/rtl/tsan_report.h | 1 - compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp | 4 +--- compiler-rt/test/tsan/fd_location_closed.cpp | 29 ---------------------------- 6 files changed, 7 insertions(+), 44 deletions(-) delete mode 100644 compiler-rt/test/tsan/fd_location_closed.cpp diff --git a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp index ab295a6..cf8f491 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp @@ -34,7 +34,6 @@ struct FdDesc { atomic_uintptr_t aux_sync; // FdSync* Tid creation_tid; StackID creation_stack; - bool closed; }; struct FdContext { @@ -121,7 +120,6 @@ static void init(ThreadState *thr, uptr pc, int fd, FdSync *s, } d->creation_tid = thr->tid; d->creation_stack = CurrentStackId(thr, pc); - d->closed = false; // This prevents false positives on fd_close_norace3.cpp test. // The mechanics of the false positive are not completely clear, // but it happens only if global reset is enabled (flush_memory_ms=1) @@ -157,7 +155,7 @@ void FdOnFork(ThreadState *thr, uptr pc) { } } -bool FdLocation(uptr addr, int *fd, Tid *tid, StackID *stack, bool *closed) { +bool FdLocation(uptr addr, int *fd, Tid *tid, StackID *stack) { for (int l1 = 0; l1 < kTableSizeL1; l1++) { FdDesc *tab = (FdDesc*)atomic_load(&fdctx.tab[l1], memory_order_relaxed); if (tab == 0) @@ -168,7 +166,6 @@ bool FdLocation(uptr addr, int *fd, Tid *tid, StackID *stack, bool *closed) { *fd = l1 * kTableSizeL1 + l2; *tid = d->creation_tid; *stack = d->creation_stack; - *closed = d->closed; return true; } } @@ -245,9 +242,8 @@ void FdClose(ThreadState *thr, uptr pc, int fd, bool write) { reinterpret_cast( atomic_load(&d->aux_sync, memory_order_relaxed))); atomic_store(&d->aux_sync, 0, memory_order_relaxed); - d->closed = true; - d->creation_tid = thr->tid; - d->creation_stack = CurrentStackId(thr, pc); + d->creation_tid = kInvalidTid; + d->creation_stack = kInvalidStackID; } void FdFileCreate(ThreadState *thr, uptr pc, int fd) { diff --git a/compiler-rt/lib/tsan/rtl/tsan_fd.h b/compiler-rt/lib/tsan/rtl/tsan_fd.h index dddc1d2..92625dc 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_fd.h +++ b/compiler-rt/lib/tsan/rtl/tsan_fd.h @@ -54,7 +54,7 @@ void FdSocketCreate(ThreadState *thr, uptr pc, int fd); void FdSocketAccept(ThreadState *thr, uptr pc, int fd, int newfd); void FdSocketConnecting(ThreadState *thr, uptr pc, int fd); void FdSocketConnect(ThreadState *thr, uptr pc, int fd); -bool FdLocation(uptr addr, int *fd, Tid *tid, StackID *stack, bool *closed); +bool FdLocation(uptr addr, int *fd, Tid *tid, StackID *stack); void FdOnFork(ThreadState *thr, uptr pc); uptr File2addr(const char *path); diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_report.cpp index 9b03adc..3b0bddc 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_report.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_report.cpp @@ -200,9 +200,8 @@ static void PrintLocation(const ReportLocation *loc) { } else if (loc->type == ReportLocationTLS) { Printf(" Location is TLS of %s.\n\n", thread_name(thrbuf, loc->tid)); } else if (loc->type == ReportLocationFD) { - Printf(" Location is file descriptor %d %s by %s at:\n", loc->fd, - loc->fd_closed ? "destroyed" : "created", - thread_name(thrbuf, loc->tid)); + Printf(" Location is file descriptor %d created by %s at:\n", + loc->fd, thread_name(thrbuf, loc->tid)); print_stack = true; } Printf("%s", d.Default()); diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h index 3c88864..718eacd 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_report.h +++ b/compiler-rt/lib/tsan/rtl/tsan_report.h @@ -76,7 +76,6 @@ struct ReportLocation { uptr external_tag = 0; Tid tid = kInvalidTid; int fd = 0; - bool fd_closed = false; bool suppressable = false; ReportStack *stack = nullptr; }; diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp index c2cff60..713acb0 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cpp @@ -281,11 +281,9 @@ void ScopedReportBase::AddLocation(uptr addr, uptr size) { int fd = -1; Tid creat_tid = kInvalidTid; StackID creat_stack = 0; - bool closed = false; - if (FdLocation(addr, &fd, &creat_tid, &creat_stack, &closed)) { + if (FdLocation(addr, &fd, &creat_tid, &creat_stack)) { auto *loc = New(); loc->type = ReportLocationFD; - loc->fd_closed = closed; loc->fd = fd; loc->tid = creat_tid; loc->stack = SymbolizeStackId(creat_stack); diff --git a/compiler-rt/test/tsan/fd_location_closed.cpp b/compiler-rt/test/tsan/fd_location_closed.cpp deleted file mode 100644 index 55abc36..0000000 --- a/compiler-rt/test/tsan/fd_location_closed.cpp +++ /dev/null @@ -1,29 +0,0 @@ -// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t |& FileCheck %s -#include "test.h" - -#include - -void *Thread(void *x) { - int fd = (long)x; - char buf; - read(fd, &buf, 1); - barrier_wait(&barrier); - close(fd); - return NULL; -} - -int main() { - barrier_init(&barrier, 2); - int fd = open("/dev/random", O_RDONLY); - pthread_t t[2]; - pthread_create(&t[0], NULL, Thread, (void *)(long)fd); - pthread_create(&t[1], NULL, Thread, (void *)(long)fd); - - pthread_join(t[0], NULL); - pthread_join(t[1], NULL); -} - -// CHECK: WARNING: ThreadSanitizer: data race -// CHECK: Location is file descriptor {{[0-9]+}} destroyed by thread -// CHECK: #0 close -// CHECK: #1 Thread -- 2.7.4