tsan: always handle closing of file descriptors

[dvyukov]

Summary

If we miss both close of a file descriptor and a subsequent open
if the same file descriptor number, we report false positives
between operations on the old and on the new descriptors.

There are lots of ways to create new file descriptors, but for closing
there is mostly close call. So we try to handle at least it.
However, if the close happens in an ignored library, we miss it
and start reporting false positives.

Handle closing of file descriptors always, even in ignored libraries
(as we do for malloc/free and other critical functions).
But don't imitate memory accesses on close for ignored libraries.

FdClose checks validity of the fd (fd >= 0) itself,
so remove the excessive checks in the callers.

[melver]

compiler-rt/lib/tsan/rtl/tsan_interceptors.h

39

Just 'inline'? (like in_symbolizer() above -- otherwise every TU that outlines it gets its own copy)

[dvyukov]

remove 'static'

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

 //===-- tsan_fd.cpp -------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of ThreadSanitizer (TSan), a race detector.
 //
 //===----------------------------------------------------------------------===//
 
 #include "tsan_fd.h"
-#include "tsan_rtl.h"
+
 #include <sanitizer_common/sanitizer_atomic.h>
 
+#include "tsan_interceptors.h"
+#include "tsan_rtl.h"
+
 namespace __tsan {
 
 const int kTableSizeL1 = 1024;
 const int kTableSizeL2 = 1024;
 const int kTableSize = kTableSizeL1 * kTableSizeL2;
 
 struct FdSync {
   atomic_uint64_t rc;
void FdAccess(ThreadState *thr, uptr pc, int fd) {
   MemoryAccess(thr, pc, (uptr)d, 8, kAccessRead);
 }
 
 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);
+  if (!MustIgnoreInterceptor(thr)) {
     if (write) {
       // To catch races between fd usage and close.
       MemoryAccess(thr, pc, (uptr)d, 8, kAccessWrite);
     } 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.
       MemoryAccess(thr, pc, (uptr)d, 8, kAccessRead);
     }
+  }
   // 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);
   unref(thr, pc, d->sync);
   d->sync = 0;
   d->creation_tid = kInvalidTid;
   d->creation_stack = kInvalidStackID;
 }

compiler-rt/lib/tsan/rtl/tsan_interceptors.h

 LibIgnore *libignore();
 
 #if !SANITIZER_GO
 inline bool in_symbolizer() {
   return UNLIKELY(cur_thread_init()->in_symbolizer);
 }
 #endif
 
+inline bool MustIgnoreInterceptor(ThreadState *thr) {
+  return !thr->is_inited || thr->ignore_interceptors || thr->in_ignored_lib;
+}
+
 }  // namespace __tsan
 
 #define SCOPED_INTERCEPTOR_RAW(func, ...)            \
   ThreadState *thr = cur_thread_init();              \
   ScopedInterceptor si(thr, #func, GET_CALLER_PC()); \
   UNUSED const uptr pc = GET_CURRENT_PC();
 
 #ifdef __powerpc64__
 // Debugging of crashes on powerpc after commit:
 // c80604f7a3 ("tsan: remove real func check from interceptors")
 // Somehow replacing if with DCHECK leads to strange failures in:
 // SanitizerCommon-tsan-powerpc64le-Linux :: Linux/ptrace.cpp
 // https://lab.llvm.org/buildbot/#/builders/105
 // https://lab.llvm.org/buildbot/#/builders/121
 // https://lab.llvm.org/buildbot/#/builders/57
 #  define CHECK_REAL_FUNC(func)                                          \
     if (REAL(func) == 0) {                                               \
       Report("FATAL: ThreadSanitizer: failed to intercept %s\n", #func); \
       Die();                                                             \
     }
 #else
 #  define CHECK_REAL_FUNC(func) DCHECK(REAL(func))
 #endif
 
-#define SCOPED_TSAN_INTERCEPTOR(func, ...)                                \
-  SCOPED_INTERCEPTOR_RAW(func, __VA_ARGS__);                              \
-  CHECK_REAL_FUNC(func);                                                  \
-  if (!thr->is_inited || thr->ignore_interceptors || thr->in_ignored_lib) \
+#define SCOPED_TSAN_INTERCEPTOR(func, ...)   \
+  SCOPED_INTERCEPTOR_RAW(func, __VA_ARGS__); \
+  CHECK_REAL_FUNC(func);                     \
+  if (MustIgnoreInterceptor(thr))            \
     return REAL(func)(__VA_ARGS__);
 
 #define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_START() \
     si.DisableIgnores();
 
 #define SCOPED_TSAN_INTERCEPTOR_USER_CALLBACK_END() \
     si.EnableIgnores();
 

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

 }
 #define TSAN_MAYBE_INTERCEPT_EVENTFD TSAN_INTERCEPT(eventfd)
 #else
 #define TSAN_MAYBE_INTERCEPT_EVENTFD
 #endif
 
 #if SANITIZER_LINUX
 TSAN_INTERCEPTOR(int, signalfd, int fd, void *mask, int flags) {
-  SCOPED_TSAN_INTERCEPTOR(signalfd, fd, mask, flags);
-  if (fd >= 0)
-    FdClose(thr, pc, fd);
+  SCOPED_INTERCEPTOR_RAW(signalfd, fd, mask, flags);
   fd = REAL(signalfd)(fd, mask, flags);
-  if (fd >= 0)
+  if (!MustIgnoreInterceptor(thr))
     FdSignalCreate(thr, pc, fd);
   return fd;
 }
 #define TSAN_MAYBE_INTERCEPT_SIGNALFD TSAN_INTERCEPT(signalfd)
 #else
 #define TSAN_MAYBE_INTERCEPT_SIGNALFD
 #endif
 
TSAN_INTERCEPTOR(int, listen, int fd, int backlog) {
   SCOPED_TSAN_INTERCEPTOR(listen, fd, backlog);
   int res = REAL(listen)(fd, backlog);
   if (fd > 0 && res == 0)
     FdAccess(thr, pc, fd);
   return res;
 }
 
 TSAN_INTERCEPTOR(int, close, int fd) {
-  SCOPED_TSAN_INTERCEPTOR(close, fd);
-  if (fd >= 0)
-    FdClose(thr, pc, fd);
+  SCOPED_INTERCEPTOR_RAW(close, fd);
   return REAL(close)(fd);
 }
 
 #if SANITIZER_LINUX
 TSAN_INTERCEPTOR(int, __close, int fd) {
-  SCOPED_TSAN_INTERCEPTOR(__close, fd);
-  if (fd >= 0)
-    FdClose(thr, pc, fd);
+  SCOPED_INTERCEPTOR_RAW(__close, fd);
   return REAL(__close)(fd);
 }
 #define TSAN_MAYBE_INTERCEPT___CLOSE TSAN_INTERCEPT(__close)
 #else
 #define TSAN_MAYBE_INTERCEPT___CLOSE
 #endif
 
 // glibc guts
 #if SANITIZER_LINUX && !SANITIZER_ANDROID
 TSAN_INTERCEPTOR(void, __res_iclose, void *state, bool free_addr) {
-  SCOPED_TSAN_INTERCEPTOR(__res_iclose, state, free_addr);
+  SCOPED_INTERCEPTOR_RAW(__res_iclose, state, free_addr);
   int fds[64];
   int cnt = ExtractResolvFDs(state, fds, ARRAY_SIZE(fds));
-  for (int i = 0; i < cnt; i++) {
-    if (fds[i] > 0)
-      FdClose(thr, pc, fds[i]);
-  }
+  for (int i = 0; i < cnt; i++) FdClose(thr, pc, fds[i]);
   REAL(__res_iclose)(state, free_addr);
 }
 #define TSAN_MAYBE_INTERCEPT___RES_ICLOSE TSAN_INTERCEPT(__res_iclose)
 #else
 #define TSAN_MAYBE_INTERCEPT___RES_ICLOSE
 #endif
 
 TSAN_INTERCEPTOR(int, pipe, int *pipefd) {

 TSAN_INTERCEPTOR(int, rmdir, char *path) {
   SCOPED_TSAN_INTERCEPTOR(rmdir, path);
   Release(thr, pc, Dir2addr(path));
   int res = REAL(rmdir)(path);
   return res;
 }
 
 TSAN_INTERCEPTOR(int, closedir, void *dirp) {
-  SCOPED_TSAN_INTERCEPTOR(closedir, dirp);
+  SCOPED_INTERCEPTOR_RAW(closedir, dirp);
   if (dirp) {
     int fd = dirfd(dirp);
     FdClose(thr, pc, fd);
   }
   return REAL(closedir)(dirp);
 }
 
 #if SANITIZER_LINUX
#define COMMON_INTERCEPTOR_FILE_OPEN(ctx, file, path) \
   if (file) {                                         \
     int fd = fileno_unlocked(file);                   \
     if (fd >= 0) FdFileCreate(thr, pc, fd);           \
   }
 
 #define COMMON_INTERCEPTOR_FILE_CLOSE(ctx, file) \
   if (file) {                                    \
     int fd = fileno_unlocked(file);              \
-    if (fd >= 0) FdClose(thr, pc, fd);           \
+    FdClose(thr, pc, fd);                        \
   }
 
 #define COMMON_INTERCEPTOR_DLOPEN(filename, flag) \
   ({                                              \
     CheckNoDeepBind(filename, flag);              \
     ThreadIgnoreBegin(thr, 0);                    \
     void *res = REAL(dlopen)(filename, flag);     \
     ThreadIgnoreEnd(thr);                         \

 
 static USED void syscall_release(uptr pc, uptr addr) {
   TSAN_SYSCALL();
   DPrintf("syscall_release(0x%zx)\n", addr);
   Release(thr, pc, addr);
 }
 
 static void syscall_fd_close(uptr pc, int fd) {
-  TSAN_SYSCALL();
+  auto *thr = cur_thread();
   FdClose(thr, pc, fd);
 }
 
 static USED void syscall_fd_acquire(uptr pc, int fd) {
   TSAN_SYSCALL();
   FdAcquire(thr, pc, fd);
   DPrintf("syscall_fd_acquire(%d)\n", fd);
 }