[tsan] Detect uses of uninitialized, destroyed and invalid mutexes

[kubamracek]

Summary

Currently, calling pthread_mutex_lock on a destroyed (or uninitialized or completely invalid) mutex will silently continue, without any TSan reports. This is because the function will return EINVAL in these cases. However, it's extremely uncommon to correctly handle failure return values from pthread_mutex_lock. In fact, I don't remember ever seeing code that even checks the return value from pthread_mutex_lock.

This patch adds a new TSan report type, ReportTypeMutexInvalidAccess, which is triggered when pthread_mutex_lock or pthread_mutex_unlock returns EINVAL (this means the mutex is invalid, uninitialized or already destroyed).

[dvyukov]

LGTM

My only concern is that some pthread implementations will not detect this bug and the test will fail. But I guess the bots will tell.

We probably could do the same check in pthread_mutex_destroy. But FWIW my implementation does not detect double-destroy.

This show a flaw in the interceptors. If a mutex function fails, we don't execute COMMON_INTERCEPTOR_MUTEX_LOCK/DESTROY and don't catch use-after-free on mutex accesses. Always emulating the memory access is probably the right thing to do (esp for asan).

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

 #ifndef COMMON_INTERCEPTOR_MUTEX_UNLOCK
 #define COMMON_INTERCEPTOR_MUTEX_UNLOCK(ctx, m) {}
 #endif
 
 #ifndef COMMON_INTERCEPTOR_MUTEX_REPAIR
 #define COMMON_INTERCEPTOR_MUTEX_REPAIR(ctx, m) {}
 #endif
 
+#ifndef COMMON_INTERCEPTOR_MUTEX_INVALID
+#define COMMON_INTERCEPTOR_MUTEX_INVALID(ctx, m) {}
+#endif
+
 #ifndef COMMON_INTERCEPTOR_HANDLE_RECVMSG
 #define COMMON_INTERCEPTOR_HANDLE_RECVMSG(ctx, msg) ((void)(msg))
 #endif
 
 #ifndef COMMON_INTERCEPTOR_FILE_OPEN
 #define COMMON_INTERCEPTOR_FILE_OPEN(ctx, file, path) {}
 #endif
 

 INTERCEPTOR(int, pthread_mutex_lock, void *m) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_mutex_lock, m);
   int res = REAL(pthread_mutex_lock)(m);
   if (res == errno_EOWNERDEAD)
     COMMON_INTERCEPTOR_MUTEX_REPAIR(ctx, m);
   if (res == 0 || res == errno_EOWNERDEAD)
     COMMON_INTERCEPTOR_MUTEX_LOCK(ctx, m);
+  if (res == errno_EINVAL)
+    COMMON_INTERCEPTOR_MUTEX_INVALID(ctx, m);
   return res;
 }
 
 INTERCEPTOR(int, pthread_mutex_unlock, void *m) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_mutex_unlock, m);
   COMMON_INTERCEPTOR_MUTEX_UNLOCK(ctx, m);
-  return REAL(pthread_mutex_unlock)(m);
+  int res = REAL(pthread_mutex_unlock)(m);
+  if (res == errno_EINVAL)
+    COMMON_INTERCEPTOR_MUTEX_INVALID(ctx, m);
+  return res;
 }
 
 #define INIT_PTHREAD_MUTEX_LOCK COMMON_INTERCEPT_FUNCTION(pthread_mutex_lock)
 #define INIT_PTHREAD_MUTEX_UNLOCK \
   COMMON_INTERCEPT_FUNCTION(pthread_mutex_unlock)
 #else
 #define INIT_PTHREAD_MUTEX_LOCK
 #define INIT_PTHREAD_MUTEX_UNLOCK

compiler-rt/trunk/lib/tsan/rtl/tsan_debugging.cc

 static const char *ReportTypeDescription(ReportType typ) {
   if (typ == ReportTypeRace) return "data-race";
   if (typ == ReportTypeVptrRace) return "data-race-vptr";
   if (typ == ReportTypeUseAfterFree) return "heap-use-after-free";
   if (typ == ReportTypeVptrUseAfterFree) return "heap-use-after-free-vptr";
   if (typ == ReportTypeThreadLeak) return "thread-leak";
   if (typ == ReportTypeMutexDestroyLocked) return "locked-mutex-destroy";
   if (typ == ReportTypeMutexDoubleLock) return "mutex-double-lock";
+  if (typ == ReportTypeMutexInvalidAccess) return "mutex-invalid-access";
   if (typ == ReportTypeMutexBadUnlock) return "mutex-bad-unlock";
   if (typ == ReportTypeMutexBadReadLock) return "mutex-bad-read-lock";
   if (typ == ReportTypeMutexBadReadUnlock) return "mutex-bad-read-unlock";
   if (typ == ReportTypeSignalUnsafe) return "signal-unsafe-call";
   if (typ == ReportTypeErrnoInSignal) return "errno-in-signal-handler";
   if (typ == ReportTypeDeadlock) return "lock-order-inversion";
   return "";
 }

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc

 #define COMMON_INTERCEPTOR_MUTEX_UNLOCK(ctx, m) \
   MutexUnlock(((TsanInterceptorContext *)ctx)->thr, \
             ((TsanInterceptorContext *)ctx)->pc, (uptr)m)
 
 #define COMMON_INTERCEPTOR_MUTEX_REPAIR(ctx, m) \
   MutexRepair(((TsanInterceptorContext *)ctx)->thr, \
             ((TsanInterceptorContext *)ctx)->pc, (uptr)m)
 
+#define COMMON_INTERCEPTOR_MUTEX_INVALID(ctx, m) \
+  MutexInvalidAccess(((TsanInterceptorContext *)ctx)->thr, \
+                     ((TsanInterceptorContext *)ctx)->pc, (uptr)m)
+
 #if !SANITIZER_MAC
 #define COMMON_INTERCEPTOR_HANDLE_RECVMSG(ctx, msg) \
   HandleRecvmsg(((TsanInterceptorContext *)ctx)->thr, \
       ((TsanInterceptorContext *)ctx)->pc, msg)
 #endif
 
 #define COMMON_INTERCEPTOR_GET_TLS_RANGE(begin, end)                           \
   if (TsanThread *t = GetCurrentThread()) {                                    \

compiler-rt/trunk/lib/tsan/rtl/tsan_report.h

 enum ReportType {
   ReportTypeRace,
   ReportTypeVptrRace,
   ReportTypeUseAfterFree,
   ReportTypeVptrUseAfterFree,
   ReportTypeThreadLeak,
   ReportTypeMutexDestroyLocked,
   ReportTypeMutexDoubleLock,
+  ReportTypeMutexInvalidAccess,
   ReportTypeMutexBadUnlock,
   ReportTypeMutexBadReadLock,
   ReportTypeMutexBadReadUnlock,
   ReportTypeSignalUnsafe,
   ReportTypeErrnoInSignal,
   ReportTypeDeadlock
 };
 

compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc

static const char *ReportTypeString(ReportType typ) {
   if (typ == ReportTypeVptrUseAfterFree)
     return "heap-use-after-free (virtual call vs free)";
   if (typ == ReportTypeThreadLeak)
     return "thread leak";
   if (typ == ReportTypeMutexDestroyLocked)
     return "destroy of a locked mutex";
   if (typ == ReportTypeMutexDoubleLock)
     return "double lock of a mutex";
+  if (typ == ReportTypeMutexInvalidAccess)
+    return "use of an invalid mutex (e.g. uninitialized or destroyed)";
   if (typ == ReportTypeMutexBadUnlock)
     return "unlock of an unlocked mutex (or by a wrong thread)";
   if (typ == ReportTypeMutexBadReadLock)
     return "read lock of a write locked mutex";
   if (typ == ReportTypeMutexBadReadUnlock)
     return "read unlock of a write locked mutex";
   if (typ == ReportTypeSignalUnsafe)
     return "signal-unsafe call inside of a signal";

compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.h

 void MutexDestroy(ThreadState *thr, uptr pc, uptr addr);
 void MutexLock(ThreadState *thr, uptr pc, uptr addr, int rec = 1,
                bool try_lock = false);
 int  MutexUnlock(ThreadState *thr, uptr pc, uptr addr, bool all = false);
 void MutexReadLock(ThreadState *thr, uptr pc, uptr addr, bool try_lock = false);
 void MutexReadUnlock(ThreadState *thr, uptr pc, uptr addr);
 void MutexReadOrWriteUnlock(ThreadState *thr, uptr pc, uptr addr);
 void MutexRepair(ThreadState *thr, uptr pc, uptr addr);  // call on EOWNERDEAD
+void MutexInvalidAccess(ThreadState *thr, uptr pc, uptr addr);
 
 void Acquire(ThreadState *thr, uptr pc, uptr addr);
 // AcquireGlobal synchronizes the current thread with all other threads.
 // In terms of happens-before relation, it draws a HB edge from all threads
 // (where they happen to execute right now) to the current thread. We use it to
 // handle Go finalizers. Namely, finalizer goroutine executes AcquireGlobal
 // right before executing finalizers. This provides a coarse, but simple
 // approximation of the actual required synchronization.

compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc

 void MutexRepair(ThreadState *thr, uptr pc, uptr addr) {
   DPrintf("#%d: MutexRepair %zx\n", thr->tid, addr);
   SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, addr, true);
   s->owner_tid = SyncVar::kInvalidTid;
   s->recursion = 0;
   s->mtx.Unlock();
 }
 
+void MutexInvalidAccess(ThreadState *thr, uptr pc, uptr addr) {
+  DPrintf("#%d: MutexInvalidAccess %zx\n", thr->tid, addr);
+  SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, addr, true);
+  u64 mid = s->GetId();
+  s->mtx.Unlock();
+  ReportMutexMisuse(thr, pc, ReportTypeMutexInvalidAccess, addr, mid);
+}
+
 void Acquire(ThreadState *thr, uptr pc, uptr addr) {
   DPrintf("#%d: Acquire %zx\n", thr->tid, addr);
   if (thr->ignore_sync)
     return;
   SyncVar *s = ctx->metamap.GetOrCreateAndLock(thr, pc, addr, false);
   AcquireImpl(thr, pc, &s->clock);
   s->mtx.ReadUnlock();
 }

compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc

static const char *conv(ReportType typ) {
   else if (typ == ReportTypeVptrUseAfterFree)
     return kSuppressionRace;
   else if (typ == ReportTypeThreadLeak)
     return kSuppressionThread;
   else if (typ == ReportTypeMutexDestroyLocked)
     return kSuppressionMutex;
   else if (typ == ReportTypeMutexDoubleLock)
     return kSuppressionMutex;
+  else if (typ == ReportTypeMutexInvalidAccess)
+    return kSuppressionMutex;
   else if (typ == ReportTypeMutexBadUnlock)
     return kSuppressionMutex;
   else if (typ == ReportTypeMutexBadReadLock)
     return kSuppressionMutex;
   else if (typ == ReportTypeMutexBadReadUnlock)
     return kSuppressionMutex;
   else if (typ == ReportTypeSignalUnsafe)
     return kSuppressionSignal;

compiler-rt/trunk/test/tsan/mutex_lock_destroyed.cc

+// RUN: %clangxx_tsan %s -o %t
+// RUN: %deflake %run %t | FileCheck %s
+// RUN: %deflake %run %t 1 | FileCheck %s
+
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int main(int argc, char *argv[]) {
+  pthread_mutex_t *m = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t));
+  pthread_mutex_init(m, 0);
+  pthread_mutex_lock(m);
+  pthread_mutex_unlock(m);
+  pthread_mutex_destroy(m);
+
+  if (argc > 1 && argv[1][0] == '1')
+    free(m);
+
+  pthread_mutex_lock(m);
+  // CHECK: WARNING: ThreadSanitizer: use of an invalid mutex (e.g. uninitialized or destroyed)
+  // CHECK:   #0 pthread_mutex_lock
+  // CHECK:   #1 main {{.*}}mutex_lock_destroyed.cc:[[@LINE-3]]
+
+  return 0;
+}