Skip to content

Commit

Permalink
tsan: fix another false positive related to open/close
Browse files Browse the repository at this point in the history
The false positive fixed by commit f831d6f
("tsan: fix false positive during fd close") still happens episodically
on the added more stressful test which does just open/close.

I don't have a coherent explanation as to what exactly happens
but the fix fixes the false positive on this test as well.
The issue may be related to lost writes during asynchronous MADV_DONTNEED.
I've debugged similar unexplainable false positive related to freed and
reused memory and at the time the only possible explanation I found is that
an asynchronous MADV_DONTNEED may lead to lost writes. That's why commit
302ec7b ("tsan: add memory_limit_mb flag") added StopTheWorld around
the memory flush, but unfortunately the commit does not capture these findings.

Reviewed By: melver

Differential Revision: https://reviews.llvm.org/D121363
  • Loading branch information
dvyukov committed Mar 10, 2022
1 parent a1ac771 commit 66298e1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
7 changes: 6 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_fd.cpp
Expand Up @@ -113,12 +113,17 @@ static void init(ThreadState *thr, uptr pc, int fd, FdSync *s,
}
d->creation_tid = thr->tid;
d->creation_stack = CurrentStackId(thr, pc);
// 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)
// and may be related to lost writes during asynchronous MADV_DONTNEED.
SlotLocker locker(thr);
if (write) {
// To catch races between fd usage and open.
MemoryRangeImitateWrite(thr, pc, (uptr)d, 8);
} else {
// See the dup-related comment in FdClose.
MemoryAccess(thr, pc, (uptr)d, 8, kAccessRead);
MemoryAccess(thr, pc, (uptr)d, 8, kAccessRead | kAccessSlotLocked);
}
}

Expand Down
28 changes: 28 additions & 0 deletions compiler-rt/test/tsan/fd_close_norace3.cpp
@@ -0,0 +1,28 @@
// RUN: %clangxx_tsan -O1 %s -o %t && %env_tsan_opts=flush_memory_ms=1 %run %t 2>&1 | FileCheck %s
#include "test.h"
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

void *Thread(void *stop) {
while (!__atomic_load_n((int *)stop, __ATOMIC_RELAXED))
close(open("/dev/null", O_RDONLY));
return 0;
}

int main() {
int stop = 0;
const int kThreads = 10;
pthread_t th[kThreads];
for (int i = 0; i < kThreads; i++)
pthread_create(&th[i], 0, Thread, &stop);
sleep(5);
__atomic_store_n(&stop, 1, __ATOMIC_RELAXED);
for (int i = 0; i < kThreads; i++)
pthread_join(th[i], 0);
fprintf(stderr, "DONE\n");
}

// CHECK-NOT: WARNING: ThreadSanitizer: data race
// CHECK: DONE

0 comments on commit 66298e1

Please sign in to comment.