tsan: more fd interceptors + bug fixes + tests

llvm-svn: 169621

diff --git a/compiler-rt/lib/tsan/lit_tests/fd_close_norace.cc b/compiler-rt/lib/tsan/lit_tests/fd_close_norace.cc
new file mode 100644 (file)
index 0000000..c000de4
--- /dev/null
+++ b/compiler-rt/lib/tsan/lit_tests/fd_close_norace.cc
@@ -0,0 +1,32 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
+#include <pthread.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+void *Thread1(void *x) {
+  int f = open("/dev/random", O_RDONLY);
+  close(f);
+  return NULL;
+}
+
+void *Thread2(void *x) {
+  sleep(1);
+  int f = open("/dev/random", O_RDONLY);
+  close(f);
+  return NULL;
+}
+
+int main() {
+  pthread_t t[2];
+  pthread_create(&t[0], NULL, Thread1, NULL);
+  pthread_create(&t[1], NULL, Thread2, NULL);
+  pthread_join(t[0], NULL);
+  pthread_join(t[1], NULL);
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer: data race
+
+

diff --git a/compiler-rt/lib/tsan/lit_tests/fd_pipe_race.cc b/compiler-rt/lib/tsan/lit_tests/fd_pipe_race.cc
index 21c2dd2..dfdb779 100644 (file)
--- a/compiler-rt/lib/tsan/lit_tests/fd_pipe_race.cc
+++ b/compiler-rt/lib/tsan/lit_tests/fd_pipe_race.cc
@@ -27,10 +27,10 @@ int main() {
 }
 
 // CHECK: WARNING: ThreadSanitizer: data race
-// CHECK:   Write of size 1
+// CHECK:   Write of size 8
 // CHECK:     #0 close
 // CHECK:     #1 Thread2
-// CHECK:   Previous read of size 1
+// CHECK:   Previous read of size 8
 // CHECK:     #0 write
 // CHECK:     #1 Thread1
 

diff --git a/compiler-rt/lib/tsan/lit_tests/fd_socket_norace.cc b/compiler-rt/lib/tsan/lit_tests/fd_socket_norace.cc
new file mode 100644 (file)
index 0000000..3a128f8
--- /dev/null
+++ b/compiler-rt/lib/tsan/lit_tests/fd_socket_norace.cc
@@ -0,0 +1,51 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
+struct sockaddr_in addr;
+int X;
+
+void *ClientThread(void *x) {
+  X = 42;
+  int c = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  if (connect(c, (struct sockaddr*)&addr, sizeof(addr))) {
+    perror("connect");
+    exit(1);
+  }
+  if (send(c, "a", 1, 0) != 1) {
+    perror("send");
+    exit(1);
+  }
+  close(c);
+  return NULL;
+}
+
+int main() {
+  int s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  addr.sin_family = AF_INET;
+  inet_pton(AF_INET, "127.0.0.1", &addr.sin_addr);
+  addr.sin_port = INADDR_ANY;
+  socklen_t len = sizeof(addr);
+  bind(s, (sockaddr*)&addr, len);
+  getsockname(s, (sockaddr*)&addr, &len);
+  listen(s, 10);
+  pthread_t t;
+  pthread_create(&t, 0, ClientThread, 0);
+  int c = accept(s, 0, 0);
+  char buf;
+  while (read(c, &buf, 1) != 1) {
+  }
+  X = 43;
+  close(c);
+  close(s);
+  pthread_join(t, 0);
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer: data race
+

diff --git a/compiler-rt/lib/tsan/lit_tests/fd_stdout_race.cc b/compiler-rt/lib/tsan/lit_tests/fd_stdout_race.cc
new file mode 100644 (file)
index 0000000..6581fc5
--- /dev/null
+++ b/compiler-rt/lib/tsan/lit_tests/fd_stdout_race.cc
@@ -0,0 +1,41 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
+#include <pthread.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+int X;
+
+void *Thread1(void *x) {
+  sleep(1);
+  int f = open("/dev/random", O_RDONLY);
+  char buf;
+  read(f, &buf, 1);
+  close(f);
+  X = 42;
+  return NULL;
+}
+
+void *Thread2(void *x) {
+  X = 43;
+  write(STDOUT_FILENO, "a", 1);
+  return NULL;
+}
+
+int main() {
+  pthread_t t[2];
+  pthread_create(&t[0], NULL, Thread1, NULL);
+  pthread_create(&t[1], NULL, Thread2, NULL);
+  pthread_join(t[0], NULL);
+  pthread_join(t[1], NULL);
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK:   Write of size 4
+// CHECK:     #0 Thread1
+// CHECK:   Previous write of size 4
+// CHECK:     #0 Thread2
+
+

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
index 90519f8..cd5e8da 100644 (file)
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
@@ -309,8 +309,8 @@ TSAN_INTERCEPTOR(void, siglongjmp, void *env, int val) {
 }
 
 enum FdType {
-  FdNone,  // Does not require any sync.
   FdGlobal,  // Something we don't know about, global sync.
+  FdNone,  // Does not require any sync.
   FdFile,
   FdSock,
   FdPipe,
@@ -338,6 +338,9 @@ struct FdContext {
 static FdContext fdctx;
 
 static void FdInit() {
+  fdctx.desc[0].type = FdNone;
+  fdctx.desc[1].type = FdNone;
+  fdctx.desc[2].type = FdNone;
 }
 
 static void *FdAddr(int fd) {
@@ -365,19 +368,19 @@ static void *FdAddr(int fd) {
 static void FdAcquire(ThreadState *thr, uptr pc, int fd) {
   void *addr = FdAddr(fd);
   DPrintf("#%d: FdAcquire(%d) -> %p\n", thr->tid, fd, addr);
-  if (addr) {
+  if (addr)
     Acquire(thr, pc, (uptr)addr);
-    MemoryRead1Byte(thr, pc, (uptr)addr);
-  }
+  if (fd < FdContext::kMaxFds)
+    MemoryRead8Byte(thr, pc, (uptr)&fdctx.desc[fd].sync);
 }
 
 static void FdRelease(ThreadState *thr, uptr pc, int fd) {
   void *addr = FdAddr(fd);
   DPrintf("#%d: FdRelease(%d) -> %p\n", thr->tid, fd, addr);
-  if (addr) {
+  if (addr)
     Release(thr, pc, (uptr)addr);
-    MemoryRead1Byte(thr, pc, (uptr)addr);
-  }
+  if (fd < FdContext::kMaxFds)
+    MemoryRead8Byte(thr, pc, (uptr)&fdctx.desc[fd].sync);
 }
 
 static void FdClose(ThreadState *thr, uptr pc, int fd) {
@@ -385,14 +388,47 @@ static void FdClose(ThreadState *thr, uptr pc, int fd) {
     return;
   void *addr = FdAddr(fd);
   if (addr) {
-    // To catch races between fd usage and close.
-    MemoryWrite1Byte(thr, pc, (uptr)addr);
     SyncVar *s = CTX()->synctab.GetAndRemove(thr, pc, (uptr)addr);
     if (s)
       DestroyAndFree(s);
   }
   FdDesc *desc = &fdctx.desc[fd];
-  desc->type = FdNone;
+  // FIXME(dvyukov): change to FdNone once we handle all fd operations.
+  desc->type = FdGlobal;
+  // To catch races between fd usage and close.
+  MemoryWrite8Byte(thr, pc, (uptr)&desc->sync);
+}
+
+static void FdCreateFile(ThreadState *thr, uptr pc, int fd) {
+  if (fd >= FdContext::kMaxFds)
+    return;
+  FdDesc *desc = &fdctx.desc[fd];
+  desc->type = FdFile;
+  // To catch races between fd usage and open.
+  MemoryRangeImitateWrite(thr, pc, (uptr)&desc->sync, sizeof(desc->sync));
+}
+
+static void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd) {
+  if (oldfd >= FdContext::kMaxFds || newfd >= FdContext::kMaxFds) {
+    if (oldfd < FdContext::kMaxFds) {
+      // FIXME(dvyukov): here we lose old sync object associated with the fd,
+      // this can lead to false positives.
+      FdDesc *odesc = &fdctx.desc[oldfd];
+      odesc->type = FdGlobal;
+    }
+    if (newfd < FdContext::kMaxFds) {
+      FdClose(thr, pc, newfd);
+      FdDesc *ndesc = &fdctx.desc[newfd];
+      ndesc->type = FdGlobal;
+    }
+    return;
+  }
+
+  FdClose(thr, pc, newfd);
+  FdDesc *ndesc = &fdctx.desc[newfd];
+  ndesc->type = FdFile;
+  // To catch races between fd usage and open.
+  MemoryRangeImitateWrite(thr, pc, (uptr)&ndesc->sync, sizeof(ndesc->sync));
 }
 
 static void FdCreatePipe(ThreadState *thr, uptr pc, int rfd, int wfd) {
@@ -405,21 +441,19 @@ static void FdCreatePipe(ThreadState *thr, uptr pc, int rfd, int wfd) {
       FdDesc *wdesc = &fdctx.desc[wfd];
       wdesc->type = FdGlobal;
     }
+    return;
   }
+
   FdDesc *rdesc = &fdctx.desc[rfd];
   rdesc->type = FdPipe;
-  void *raddr = FdAddr(rfd);
-  if (raddr) {
-    // To catch races between fd usage and open.
-    MemoryWrite1Byte(thr, pc, (uptr)raddr);
-  }
+  // To catch races between fd usage and open.
+  MemoryRangeImitateWrite(thr, pc, (uptr)&rdesc->sync, sizeof(rdesc->sync));
+
   FdDesc *wdesc = &fdctx.desc[wfd];
   wdesc->type = FdPipe;
-  void *waddr = FdAddr(wfd);
-  if (waddr) {
-    // To catch races between fd usage and open.
-    MemoryWrite1Byte(thr, pc, (uptr)waddr);
-  }
+  // To catch races between fd usage and open.
+  MemoryRangeImitateWrite(thr, pc, (uptr)&wdesc->sync, sizeof(rdesc->sync));
+
   DPrintf("#%d: FdCreatePipe(%d, %d) -> (%p, %p)\n",
       thr->tid, rfd, wfd, raddr, waddr);
 }
@@ -1186,6 +1220,46 @@ TSAN_INTERCEPTOR(int, sem_getvalue, void *s, int *sval) {
   return res;
 }
 
+TSAN_INTERCEPTOR(int, open, const char *name, int flags, int mode) {
+  SCOPED_TSAN_INTERCEPTOR(open, name, flags, mode);
+  int fd = REAL(open)(name, flags, mode);
+  if (fd >= 0)
+    FdCreateFile(thr, pc, fd);
+  return fd;
+}
+
+TSAN_INTERCEPTOR(int, creat, const char *name, int mode) {
+  SCOPED_TSAN_INTERCEPTOR(creat, name, mode);
+  int fd = REAL(creat)(name, mode);
+  if (fd >= 0)
+    FdCreateFile(thr, pc, fd);
+  return fd;
+}
+
+TSAN_INTERCEPTOR(int, dup, int oldfd) {
+  SCOPED_TSAN_INTERCEPTOR(dup, oldfd);
+  int newfd = REAL(dup)(oldfd);
+  if (newfd >= 0 && newfd != oldfd)
+    FdDup(thr, pc, oldfd, newfd);
+  return newfd;
+}
+
+TSAN_INTERCEPTOR(int, dup2, int oldfd, int newfd) {
+  SCOPED_TSAN_INTERCEPTOR(dup2, oldfd, newfd);
+  int newfd2 = REAL(dup2)(oldfd, newfd);
+  if (newfd2 >= 0 && newfd2 != oldfd)
+    FdDup(thr, pc, oldfd, newfd2);
+  return newfd2;
+}
+
+TSAN_INTERCEPTOR(int, dup3, int oldfd, int newfd, int flags) {
+  SCOPED_TSAN_INTERCEPTOR(dup3, oldfd, newfd, flags);
+  int newfd2 = REAL(dup3)(oldfd, newfd, flags);
+  if (newfd2 >= 0 && newfd2 != oldfd)
+    FdDup(thr, pc, oldfd, newfd2);
+  return newfd2;
+}
+
 TSAN_INTERCEPTOR(int, close, int fd) {
   SCOPED_TSAN_INTERCEPTOR(close, fd);
   FdClose(thr, pc, fd);
@@ -1694,6 +1768,11 @@ void InitializeInterceptors() {
   TSAN_INTERCEPT(sem_post);
   TSAN_INTERCEPT(sem_getvalue);
 
+  TSAN_INTERCEPT(open);
+  TSAN_INTERCEPT(creat);
+  TSAN_INTERCEPT(dup);
+  TSAN_INTERCEPT(dup2);
+  TSAN_INTERCEPT(dup3);
   TSAN_INTERCEPT(close);
   TSAN_INTERCEPT(pipe);
   TSAN_INTERCEPT(pipe2);

diff --git a/compiler-rt/lib/tsan/rtl/tsan_stat.cc b/compiler-rt/lib/tsan/rtl/tsan_stat.cc
index fb40f59..ba132b3 100644 (file)
--- a/compiler-rt/lib/tsan/rtl/tsan_stat.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_stat.cc
@@ -181,6 +181,11 @@ void StatOutput(u64 *stat) {
   name[StatInt_sem_timedwait]            = "  sem_timedwait                   ";
   name[StatInt_sem_post]                 = "  sem_post                        ";
   name[StatInt_sem_getvalue]             = "  sem_getvalue                    ";
+  name[StatInt_open]                     = "  open                            ";
+  name[StatInt_creat]                    = "  creat                           ";
+  name[StatInt_dup]                      = "  dup                             ";
+  name[StatInt_dup2]                     = "  dup2                            ";
+  name[StatInt_dup3]                     = "  dup3                            ";
   name[StatInt_close]                    = "  close                           ";
   name[StatInt_pipe]                     = "  pipe                            ";
   name[StatInt_pipe2]                    = "  pipe2                           ";

diff --git a/compiler-rt/lib/tsan/rtl/tsan_stat.h b/compiler-rt/lib/tsan/rtl/tsan_stat.h
index 70bed11..90dcaab 100644 (file)
--- a/compiler-rt/lib/tsan/rtl/tsan_stat.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_stat.h
@@ -176,6 +176,11 @@ enum StatType {
   StatInt_sem_timedwait,
   StatInt_sem_post,
   StatInt_sem_getvalue,
+  StatInt_open,
+  StatInt_creat,
+  StatInt_dup,
+  StatInt_dup2,
+  StatInt_dup3,
   StatInt_close,
   StatInt_pipe,
   StatInt_pipe2,