[compiler-rt] 66298e1 - tsan: fix another false positive related to open/close

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2022-03-10T17:02:51+01:00
New Revision: 66298e1c54fcf3f4643528c461f5cf376d27055b

URL: https://github.com/llvm/llvm-project/commit/66298e1c54fcf3f4643528c461f5cf376d27055b
DIFF: https://github.com/llvm/llvm-project/commit/66298e1c54fcf3f4643528c461f5cf376d27055b.diff

LOG: tsan: fix another false positive related to open/close

The false positive fixed by commit f831d6fc80
("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
302ec7b9bc ("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

Added: 
    compiler-rt/test/tsan/fd_close_norace3.cpp

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_fd.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
index 6c5835fbdc109..9f358eabd21c8 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
@@ -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);
   }
 }
 

diff  --git a/compiler-rt/test/tsan/fd_close_norace3.cpp b/compiler-rt/test/tsan/fd_close_norace3.cpp
new file mode 100644
index 0000000000000..eab8cd515ab3e
--- /dev/null
+++ b/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