[compiler-rt] r240687 - tsan: fix handling of dup2
Dmitry Vyukov
dvyukov at google.com
Thu Jun 25 13:32:04 PDT 2015
Author: dvyukov
Date: Thu Jun 25 15:32:04 2015
New Revision: 240687
URL: http://llvm.org/viewvc/llvm-project?rev=240687&view=rev
Log:
tsan: fix handling of dup2
Previously tsan modelled dup2(oldfd, newfd) as write on newfd.
We hit several cases where the write lead to false positives:
1. Some software dups a closed pipe in place of a socket before closing
the socket (to prevent races actually).
2. Some daemons dup /dev/null in place of stdin/stdout.
On the other hand we have not seen cases when write here catches real bugs.
So model dup2 as read on newfd instead.
Added:
compiler-rt/trunk/test/tsan/fd_dup_norace2.cc
compiler-rt/trunk/test/tsan/fd_dup_race.cc
Modified:
compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc
compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h
compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc?rev=240687&r1=240686&r2=240687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc Thu Jun 25 15:32:04 2015
@@ -91,7 +91,8 @@ static FdDesc *fddesc(ThreadState *thr,
}
// pd must be already ref'ed.
-static void init(ThreadState *thr, uptr pc, int fd, FdSync *s) {
+static void init(ThreadState *thr, uptr pc, int fd, FdSync *s,
+ bool write = true) {
FdDesc *d = fddesc(thr, pc, fd);
// As a matter of fact, we don't intercept all close calls.
// See e.g. libc __res_iclose().
@@ -109,8 +110,13 @@ static void init(ThreadState *thr, uptr
}
d->creation_tid = thr->tid;
d->creation_stack = CurrentStackId(thr, pc);
- // To catch races between fd usage and open.
- MemoryRangeImitateWrite(thr, pc, (uptr)d, 8);
+ if (write) {
+ // To catch races between fd usage and open.
+ MemoryRangeImitateWrite(thr, pc, (uptr)d, 8);
+ } else {
+ // See the dup-related comment in FdClose.
+ MemoryRead(thr, pc, (uptr)d, kSizeLog8);
+ }
}
void FdInit() {
@@ -181,13 +187,25 @@ void FdAccess(ThreadState *thr, uptr pc,
MemoryRead(thr, pc, (uptr)d, kSizeLog8);
}
-void FdClose(ThreadState *thr, uptr pc, int fd) {
+void FdClose(ThreadState *thr, uptr pc, int fd, bool write) {
DPrintf("#%d: FdClose(%d)\n", thr->tid, fd);
if (bogusfd(fd))
return;
FdDesc *d = fddesc(thr, pc, fd);
- // To catch races between fd usage and close.
- MemoryWrite(thr, pc, (uptr)d, kSizeLog8);
+ if (write) {
+ // To catch races between fd usage and close.
+ MemoryWrite(thr, pc, (uptr)d, kSizeLog8);
+ } else {
+ // This path is used only by dup2/dup3 calls.
+ // We do read instead of write because there is a number of legitimate
+ // cases where write would lead to false positives:
+ // 1. Some software dups a closed pipe in place of a socket before closing
+ // the socket (to prevent races actually).
+ // 2. Some daemons dup /dev/null in place of stdin/stdout.
+ // On the other hand we have not seen cases when write here catches real
+ // bugs.
+ MemoryRead(thr, pc, (uptr)d, kSizeLog8);
+ }
// We need to clear it, because if we do not intercept any call out there
// that creates fd, we will hit false postives.
MemoryResetRange(thr, pc, (uptr)d, 8);
@@ -204,15 +222,15 @@ void FdFileCreate(ThreadState *thr, uptr
init(thr, pc, fd, &fdctx.filesync);
}
-void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd) {
+void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd, bool write) {
DPrintf("#%d: FdDup(%d, %d)\n", thr->tid, oldfd, newfd);
if (bogusfd(oldfd) || bogusfd(newfd))
return;
// Ignore the case when user dups not yet connected socket.
FdDesc *od = fddesc(thr, pc, oldfd);
MemoryRead(thr, pc, (uptr)od, kSizeLog8);
- FdClose(thr, pc, newfd);
- init(thr, pc, newfd, ref(od->sync));
+ FdClose(thr, pc, newfd, write);
+ init(thr, pc, newfd, ref(od->sync), write);
}
void FdPipeCreate(ThreadState *thr, uptr pc, int rfd, int wfd) {
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h?rev=240687&r1=240686&r2=240687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h Thu Jun 25 15:32:04 2015
@@ -42,9 +42,9 @@ void FdInit();
void FdAcquire(ThreadState *thr, uptr pc, int fd);
void FdRelease(ThreadState *thr, uptr pc, int fd);
void FdAccess(ThreadState *thr, uptr pc, int fd);
-void FdClose(ThreadState *thr, uptr pc, int fd);
+void FdClose(ThreadState *thr, uptr pc, int fd, bool write = true);
void FdFileCreate(ThreadState *thr, uptr pc, int fd);
-void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd);
+void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd, bool write);
void FdPipeCreate(ThreadState *thr, uptr pc, int rfd, int wfd);
void FdEventCreate(ThreadState *thr, uptr pc, int fd);
void FdSignalCreate(ThreadState *thr, uptr pc, int fd);
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=240687&r1=240686&r2=240687&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Thu Jun 25 15:32:04 2015
@@ -1519,7 +1519,7 @@ TSAN_INTERCEPTOR(int, dup, int oldfd) {
SCOPED_TSAN_INTERCEPTOR(dup, oldfd);
int newfd = REAL(dup)(oldfd);
if (oldfd >= 0 && newfd >= 0 && newfd != oldfd)
- FdDup(thr, pc, oldfd, newfd);
+ FdDup(thr, pc, oldfd, newfd, true);
return newfd;
}
@@ -1527,7 +1527,7 @@ TSAN_INTERCEPTOR(int, dup2, int oldfd, i
SCOPED_TSAN_INTERCEPTOR(dup2, oldfd, newfd);
int newfd2 = REAL(dup2)(oldfd, newfd);
if (oldfd >= 0 && newfd2 >= 0 && newfd2 != oldfd)
- FdDup(thr, pc, oldfd, newfd2);
+ FdDup(thr, pc, oldfd, newfd2, false);
return newfd2;
}
@@ -1535,7 +1535,7 @@ TSAN_INTERCEPTOR(int, dup3, int oldfd, i
SCOPED_TSAN_INTERCEPTOR(dup3, oldfd, newfd, flags);
int newfd2 = REAL(dup3)(oldfd, newfd, flags);
if (oldfd >= 0 && newfd2 >= 0 && newfd2 != oldfd)
- FdDup(thr, pc, oldfd, newfd2);
+ FdDup(thr, pc, oldfd, newfd2, false);
return newfd2;
}
Added: compiler-rt/trunk/test/tsan/fd_dup_norace2.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/fd_dup_norace2.cc?rev=240687&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/fd_dup_norace2.cc (added)
+++ compiler-rt/trunk/test/tsan/fd_dup_norace2.cc Thu Jun 25 15:32:04 2015
@@ -0,0 +1,40 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+#include "test.h"
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+// dup2(oldfd, newfd) races with read(newfd).
+// This is not reported as race because:
+// 1. Some software dups a closed pipe in place of a socket before closing
+// the socket (to prevent races actually).
+// 2. Some daemons dup /dev/null in place of stdin/stdout.
+
+int fd;
+
+void *Thread(void *x) {
+ char buf;
+ if (read(fd, &buf, 1) != 1)
+ exit(printf("read failed\n"));
+ return 0;
+}
+
+int main() {
+ fd = open("/dev/random", O_RDONLY);
+ int fd2 = open("/dev/random", O_RDONLY);
+ if (fd == -1 || fd2 == -1)
+ exit(printf("open failed\n"));
+ pthread_t th;
+ pthread_create(&th, 0, Thread, 0);
+ if (dup2(fd2, fd) == -1)
+ exit(printf("dup2 failed\n"));
+ pthread_join(th, 0);
+ if (close(fd) == -1)
+ exit(printf("close failed\n"));
+ if (close(fd2) == -1)
+ exit(printf("close failed\n"));
+ printf("DONE\n");
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer: data race
+// CHECK: DONE
Added: compiler-rt/trunk/test/tsan/fd_dup_race.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/fd_dup_race.cc?rev=240687&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/fd_dup_race.cc (added)
+++ compiler-rt/trunk/test/tsan/fd_dup_race.cc Thu Jun 25 15:32:04 2015
@@ -0,0 +1,33 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t 2>&1 | FileCheck %s
+#include "test.h"
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+// dup2(oldfd, newfd) races with close(newfd).
+
+int fd;
+
+void *Thread(void *x) {
+ barrier_wait(&barrier);
+ if (close(fd) == -1)
+ exit(printf("close failed\n"));
+ return 0;
+}
+
+int main() {
+ barrier_init(&barrier, 2);
+ fd = open("/dev/random", O_RDONLY);
+ int fd2 = open("/dev/random", O_RDONLY);
+ if (fd == -1 || fd2 == -1)
+ exit(printf("open failed\n"));
+ pthread_t th;
+ pthread_create(&th, 0, Thread, 0);
+ if (dup2(fd2, fd) == -1)
+ exit(printf("dup2 failed\n"));
+ barrier_wait(&barrier);
+ pthread_join(th, 0);
+ printf("DONE\n");
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
More information about the llvm-commits
mailing list