tsan: support for linux aio

[dvyukov]

Summary

tsan: support for linux aio

[eugenis]

These are functions from a non-standard library (libaio). I'm not sure we can intercept them without forcing -laio on every *san program - which is obviously unacceptable.

Note a hack in io_setup syscall handler - please reproduce it in function-level interceptors.

[dvyukov]

remove the interceptors, just add tsan instrumentation to syscall interceptors

[dvyukov]

remove debug printf

[eugenis]

LGTM with comments

sanitizer_common/sanitizer_common_syscalls.inc

1284

This is not __sanitizer_iocb. It's struct io_event.

1323

sizeof(*iocb)

[dvyukov]

addressed the comments, added io_event struct definition

[dvyukov]

add support for preadv/pwritev ops

[eugenis]

sanitizer_common/sanitizer_common_syscalls.inc

1309

Are you sure len (which is aio_nbytes) is the number of iovec entries, and not the size of the array in bytes?

sanitizer_common/sanitizer_platform_limits_linux.cc

73 ↗

(On Diff #5794)

Use CHECK_TYPE_SIZE.

sanitizer_common/sanitizer_platform_limits_posix.h

120 ↗

(On Diff #5794)

Add compile-time checks for these values.

[Eugene.Zelenko]

Committed in r195830.

msan/lit_tests/Linux/syscalls.cc

int main(int argc, char *argv[]) {
   struct iocb *iocbp[2] = { &iocb[0], &iocb[1] };
   memset(iocb, 0, sizeof(iocb));
   iocb[0].aio_lio_opcode = IOCB_CMD_PREAD;
   iocb[0].aio_buf = (__u64)buf;
   iocb[0].aio_nbytes = kFortyTwo;
   iocb[1].aio_lio_opcode = IOCB_CMD_PREAD;
   iocb[1].aio_buf = (__u64)(&buf[kFortyTwo]);
   iocb[1].aio_nbytes = kFortyTwo;
-  __sanitizer_syscall_post_io_submit(1, 0, 2, &iocbp);
-  assert(__msan_test_shadow(buf, sizeof(buf)) == kFortyTwo);
+  __sanitizer_syscall_pre_io_submit(0, 2, &iocbp);
+  assert(__msan_test_shadow(buf, sizeof(buf)) == 2 * kFortyTwo);
 
   __msan_poison(buf, sizeof(buf));
   char *p = buf;
   __msan_poison(&p, sizeof(p));
   __sanitizer_syscall_post_io_setup(0, 1, &p);
   assert(__msan_test_shadow(&p, sizeof(p)) == -1);
   assert(__msan_test_shadow(buf, sizeof(buf)) >= 32);
 
   return 0;
 }

sanitizer_common/sanitizer_common_syscalls.inc

 //   COMMON_SYSCALL_POST_READ_RANGE
 //          Called in posthook for regions that were read by the kernel. Does
 //          not make much sense.
 //   COMMON_SYSCALL_POST_WRITE_RANGE
 //          Called in posthook for regions that were written to by the kernel
 //          and are now initialized.
 //   COMMON_SYSCALL_FD_CLOSE(fd)
 //          Called before closing file descriptor fd.
+//   COMMON_SYSCALL_FD_ACQUIRE(fd)
+//          Acquire memory visibility from fd.
+//   COMMON_SYSCALL_FD_RELEASE(fd)
+//          Release memory visibility to fd.
 //   COMMON_SYSCALL_PRE_FORK()
 //          Called before fork syscall.
 //   COMMON_SYSCALL_POST_FORK(long res)
 //          Called after fork syscall.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
 #if SANITIZER_LINUX
 
 #include "sanitizer_libc.h"
 
 #define PRE_SYSCALL(name)                                                      \
   SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_syscall_pre_impl_##name
 #define PRE_READ(p, s) COMMON_SYSCALL_PRE_READ_RANGE(p, s)
 #define PRE_WRITE(p, s) COMMON_SYSCALL_PRE_WRITE_RANGE(p, s)
 
 #define POST_SYSCALL(name)                                                     \
   SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_syscall_post_impl_##name
 #define POST_READ(p, s) COMMON_SYSCALL_POST_READ_RANGE(p, s)
 #define POST_WRITE(p, s) COMMON_SYSCALL_POST_WRITE_RANGE(p, s)
 
 #ifndef COMMON_SYSCALL_FD_CLOSE
-# define COMMON_SYSCALL_FD_CLOSE(fd)
+# define COMMON_SYSCALL_FD_CLOSE(fd) ((void)(fd))
+#endif
+
+#ifndef COMMON_SYSCALL_FD_ACQUIRE
+# define COMMON_SYSCALL_FD_ACQUIRE(fd) ((void)(fd))
+#endif
+
+#ifndef COMMON_SYSCALL_FD_RELEASE
+# define COMMON_SYSCALL_FD_RELEASE(fd) ((void)(fd))
 #endif
 
 #ifndef COMMON_SYSCALL_PRE_FORK
-# define COMMON_SYSCALL_PRE_FORK()
+# define COMMON_SYSCALL_PRE_FORK() {}
 #endif
 
 #ifndef COMMON_SYSCALL_POST_FORK
-# define COMMON_SYSCALL_POST_FORK(res)
+# define COMMON_SYSCALL_POST_FORK(res) {}
 #endif
 
 // FIXME: do some kind of PRE_READ for all syscall arguments (int(s) and such).
 
 extern "C" {
 struct sanitizer_kernel_iovec {
   void *iov_base;
   unsigned long iov_len;
if (res >= 0) {
     if (*ctx) POST_WRITE(*ctx, 32);
   }
 }
 
 PRE_SYSCALL(io_destroy)(long ctx) {}
 
 POST_SYSCALL(io_destroy)(long res, long ctx) {}
 
-PRE_SYSCALL(io_getevents)(long ctx_id, long min_nr, long nr, void *events,
-                          void *timeout) {
+PRE_SYSCALL(io_getevents)(long ctx_id, long min_nr, long nr,
+                          __sanitizer_iocb **iocbpp, void *timeout) {
   if (timeout) PRE_READ(timeout, struct_timespec_sz);
 }
 
 POST_SYSCALL(io_getevents)(long res, long ctx_id, long min_nr, long nr,
-                           void *events, void *timeout) {
+                           __sanitizer_iocb **iocbpp, void *timeout) {
   if (res >= 0) {
-    if (events) POST_WRITE(events, res * struct_io_event_sz);
+    if (iocbpp) POST_WRITE(iocbpp, res * struct_io_event_sz);
     if (timeout) POST_WRITE(timeout, struct_timespec_sz);
   }
+  for (long i = 0; i < res; i++) {
+    if (iocbpp[i]->aio_lio_opcode == iocb_cmd_pwrite ||
+        iocbpp[i]->aio_lio_opcode == iocb_cmd_pread)
+      COMMON_SYSCALL_FD_ACQUIRE(iocbpp[i]->aio_fildes);
+  }
 }
 
 PRE_SYSCALL(io_submit)(long ctx_id, long nr, __sanitizer_iocb **iocbpp) {
   for (long i = 0; i < nr; ++i) {
-    if (iocbpp[i]->aio_lio_opcode == iocb_cmd_pwrite && iocbpp[i]->aio_buf &&
-        iocbpp[i]->aio_nbytes)
-      PRE_READ((void *)iocbpp[i]->aio_buf, iocbpp[i]->aio_nbytes);
+    uptr op = iocbpp[i]->aio_lio_opcode;
+    int fd = iocbpp[i]->aio_fildes;
+    void *buf = (void*)iocbpp[i]->aio_buf;
+    uptr len = (uptr)iocbpp[i]->aio_nbytes;
+    if (buf && len) {
+      if (op == iocb_cmd_pwrite)
+        PRE_READ(buf, len);
+      if (op == iocb_cmd_pread)
+        POST_WRITE(buf, len);
+    }
+    if (op == iocb_cmd_pwrite || op == iocb_cmd_pread)
+      COMMON_SYSCALL_FD_RELEASE(fd);
   }
 }
 
 POST_SYSCALL(io_submit)(long res, long ctx_id, long nr,
-                        __sanitizer_iocb **iocbpp) {
-  if (res > 0 && iocbpp) {
-    for (long i = 0; i < res; ++i) {
-      if (iocbpp[i]->aio_lio_opcode == iocb_cmd_pread && iocbpp[i]->aio_buf &&
-          iocbpp[i]->aio_nbytes)
-        POST_WRITE((void *)iocbpp[i]->aio_buf, iocbpp[i]->aio_nbytes);
-    }
-  }
-}
+    __sanitizer_iocb **iocbpp) {}
 
-PRE_SYSCALL(io_cancel)(long ctx_id, void *iocb, void *result) {}
+PRE_SYSCALL(io_cancel)(long ctx_id, __sanitizer_iocb *iocb, void *result) {}
 
-POST_SYSCALL(io_cancel)(long res, long ctx_id, void *iocb, void *result) {
-  if (res >= 0) {
-    if (iocb) POST_WRITE(iocb, sizeof(__sanitizer_iocb));
+POST_SYSCALL(io_cancel)(long res, long ctx_id, __sanitizer_iocb *iocb,
+    void *result) {
+  if (res == 0) {
+    if (iocb) {
+      COMMON_SYSCALL_FD_ACQUIRE(iocb->aio_fildes);
+      POST_WRITE(iocb, sizeof(__sanitizer_iocb));
+    }
     if (result) POST_WRITE(result, struct_io_event_sz);
   }
 }
 
 PRE_SYSCALL(sendfile)(long out_fd, long in_fd, void *offset, long count) {}
 
 POST_SYSCALL(sendfile)(long res, long out_fd, long in_fd,
                        __sanitizer___kernel_off_t *offset, long count) {

tsan/rtl/tsan_interceptors.cc

 
 static void syscall_access_range(uptr pc, uptr p, uptr s, bool write) {
   TSAN_SYSCALL();
   MemoryAccessRange(thr, pc, p, s, write);
 }
 
 static void syscall_fd_close(uptr pc, int fd) {
   TSAN_SYSCALL();
-  if (fd >= 0)
-    FdClose(thr, pc, fd);
 }
 
+static void syscall_fd_acquire(uptr pc, int fd) {
+  TSAN_SYSCALL();
+  FdAcquire(thr, pc, fd);
+}
+
+static void syscall_fd_release(uptr pc, int fd) {
+  TSAN_SYSCALL();
+  FdRelease(thr, pc, fd);
+}
+
 static void syscall_pre_fork(uptr pc) {
   TSAN_SYSCALL();
 }
 
 static void syscall_post_fork(uptr pc, int res) {
   TSAN_SYSCALL();
   if (res == 0) {
     // child
     FdOnFork(thr, pc);
   } else if (res > 0) {
     // parent
   }
 }
 
 #define COMMON_SYSCALL_PRE_READ_RANGE(p, s) \
   syscall_access_range(GET_CALLER_PC(), (uptr)(p), (uptr)(s), false)
+
 #define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
   syscall_access_range(GET_CALLER_PC(), (uptr)(p), (uptr)(s), true)
+
 #define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
   do {                                       \
     (void)(p);                               \
     (void)(s);                               \
   } while (false)
+
 #define COMMON_SYSCALL_POST_WRITE_RANGE(p, s) \
   do {                                        \
     (void)(p);                                \
     (void)(s);                                \
   } while (false)
+
 #define COMMON_SYSCALL_FD_CLOSE(fd) syscall_fd_close(GET_CALLER_PC(), fd)
+
+#define COMMON_SYSCALL_FD_ACQUIRE(fd) syscall_fd_acquire(GET_CALLER_PC(), fd)
+
+#define COMMON_SYSCALL_FD_RELEASE(fd) syscall_fd_release(GET_CALLER_PC(), fd)
+
 #define COMMON_SYSCALL_PRE_FORK() \
   syscall_pre_fork(GET_CALLER_PC())
+
 #define COMMON_SYSCALL_POST_FORK(res) \
   syscall_post_fork(GET_CALLER_PC(), res)
+
 #include "sanitizer_common/sanitizer_common_syscalls.inc"
 
 namespace __tsan {
 
 static void finalize(void *arg) {
   ThreadState *thr = cur_thread();
   uptr pc = 0;
   atexit_ctx->exit(thr, pc);