Annotate QMutex with TSAN annotations

The Q(Basic)Mutex fast paths are entirely inline in the caller, which
means we need to annotate its operations directly or TSAN doesn't know
what's going on. Also annotate QRecursiveMutex.

The tryLock code could be in principle simplified via a QScopeGuard
but I didn't want to make a central class like QMutex depend on it.

[ChangeLog][QtCore][QMutex] QMutex now has annotations for
ThreadSanitizer.

Change-Id: Ibb130404e63a5ec9bcef9675f9addd16a2c38b7f
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/thread/qfutex_p.h

@@ -52,28 +52,7 @@
 //
 
 #include <private/qglobal_p.h>
-
-#if (__has_feature(thread_sanitizer) || defined(__SANITIZE_THREAD__)) && __has_include(<sanitizer/tsan_interface.h>)
-#  include <sanitizer/tsan_interface.h>
-inline void _q_tsan_acquire(void *addr, void *addr2 = nullptr)
-{
-    // A futex call ensures total ordering on the futex words
-    // (in either success or failure of the call). Instruct TSAN accordingly,
-    // as TSAN does not understand the futex(2) syscall (or equivalent).
-    __tsan_acquire(addr);
-    if (addr2)
-        __tsan_acquire(addr2);
-}
-inline void _q_tsan_release(void *addr, void *addr2 = nullptr)
-{
-    if (addr2)
-        __tsan_release(addr2);
-    __tsan_release(addr);
-}
-#else
-inline void _q_tsan_acquire(void *, void * = nullptr) {}
-inline void _q_tsan_release(void *, void * = nullptr) {}
-#endif // building for TSAN and __has_include(<sanitizer/tsan_interface.h>)
+#include <QtCore/qtsan_impl.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -114,13 +93,13 @@ namespace QtLinuxFutex {
     inline int _q_futex(int *addr, int op, int val, quintptr val2 = 0,
                         int *addr2 = nullptr, int val3 = 0) noexcept
     {
-        _q_tsan_release(addr, addr2);
+        QtTsan::futexRelease(addr, addr2);
 
         // we use __NR_futex because some libcs (like Android's bionic) don't
         // provide SYS_futex etc.
         int result = syscall(__NR_futex, addr, op | FUTEX_PRIVATE_FLAG, val, val2, addr2, val3);
 
-        _q_tsan_acquire(addr, addr2);
+        QtTsan::futexAcquire(addr, addr2);
 
         return result;
     }
@@ -176,9 +155,9 @@ constexpr inline bool futexAvailable() { return true; }
 template <typename Atomic>
 inline void futexWait(Atomic &futex, typename Atomic::Type expectedValue)
 {
-    _q_tsan_release(&futex);
+    QtTsan::futexRelease(&futex);
     WaitOnAddress(&futex, &expectedValue, sizeof(expectedValue), INFINITE);
-    _q_tsan_acquire(&futex);
+    QtTsan::futexAcquire(&futex);
 }
 template <typename Atomic>
 inline bool futexWait(Atomic &futex, typename Atomic::Type expectedValue, qint64 nstimeout)

src/corelib/thread/qmutex.cpp

@@ -334,10 +334,14 @@ QRecursiveMutex::~QRecursiveMutex()
 */
 bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 {
+    unsigned tsanFlags = QtTsan::MutexWriteReentrant | QtTsan::TryLock;
+    QtTsan::mutexPreLock(this, tsanFlags);
+
     Qt::HANDLE self = QThread::currentThreadId();
     if (owner.loadRelaxed() == self) {
         ++count;
         Q_ASSERT_X(count != 0, "QMutex::lock", "Overflow in recursion counter");
+        QtTsan::mutexPostLock(this, tsanFlags, 0);
         return true;
     }
     bool success = true;
@@ -349,6 +353,11 @@ bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 
     if (success)
         owner.storeRelaxed(self);
+    else
+        tsanFlags |= QtTsan::TryLockFailed;
+
+    QtTsan::mutexPostLock(this, tsanFlags, 0);
+
     return success;
 }
 
@@ -412,6 +421,7 @@ bool QRecursiveMutex::tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
 void QRecursiveMutex::unlock() noexcept
 {
     Q_ASSERT(owner.loadRelaxed() == QThread::currentThreadId());
+    QtTsan::mutexPreUnlock(this, 0u);
 
     if (count > 0) {
         count--;
@@ -419,6 +429,8 @@ void QRecursiveMutex::unlock() noexcept
         owner.storeRelaxed(nullptr);
         mutex.unlock();
     }
+
+    QtTsan::mutexPostUnlock(this, 0u);
 }
 
 

src/corelib/thread/qmutex.h

@@ -42,6 +42,7 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qatomic.h>
+#include <QtCore/qtsan_impl.h>
 #include <new>
 
 #if __has_include(<chrono>)
@@ -104,19 +105,37 @@ public:
 
     // BasicLockable concept
     inline void lock() QT_MUTEX_LOCK_NOEXCEPT {
+        QtTsan::mutexPreLock(this, 0u);
+
         if (!fastTryLock())
             lockInternal();
+
+        QtTsan::mutexPostLock(this, 0u, 0);
     }
 
     // BasicLockable concept
     inline void unlock() noexcept {
         Q_ASSERT(d_ptr.loadRelaxed()); //mutex must be locked
+
+        QtTsan::mutexPreUnlock(this, 0u);
+
         if (!fastTryUnlock())
             unlockInternal();
+
+        QtTsan::mutexPostUnlock(this, 0u);
     }
 
     bool tryLock() noexcept {
-        return fastTryLock();
+        unsigned tsanFlags = QtTsan::TryLock;
+        QtTsan::mutexPreLock(this, tsanFlags);
+
+        const bool success = fastTryLock();
+
+        if (!success)
+            tsanFlags |= QtTsan::TryLockFailed;
+        QtTsan::mutexPostLock(this, tsanFlags, 0);
+
+        return success;
     }
 
     // Lockable concept
@@ -168,9 +187,23 @@ public:
     using QBasicMutex::tryLock;
     bool tryLock(int timeout) QT_MUTEX_LOCK_NOEXCEPT
     {
-        if (fastTryLock())
-            return true;
-        return lockInternal(timeout);
+        unsigned tsanFlags = QtTsan::TryLock;
+        QtTsan::mutexPreLock(this, tsanFlags);
+
+        bool success = fastTryLock();
+
+        if (success) {
+            QtTsan::mutexPostLock(this, tsanFlags, 0);
+            return success;
+        }
+
+        success = lockInternal(timeout);
+
+        if (!success)
+            tsanFlags |= QtTsan::TryLockFailed;
+        QtTsan::mutexPostLock(this, tsanFlags, 0);
+
+        return success;
     }
 
     // TimedLockable concept

src/corelib/thread/qtsan_impl.h

@@ -0,0 +1,115 @@
+/****************************************************************************
+**
+** Copyright (C) 2017 Intel Corporation.
+** Copyright (C) 2022 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the QtCore module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QTSAN_IMPL_H
+#define QTSAN_IMPL_H
+
+#include <QtCore/qglobal.h>
+
+#if (__has_feature(thread_sanitizer) || defined(__SANITIZE_THREAD__)) && __has_include(<sanitizer/tsan_interface.h>)
+#  define QT_BUILDING_UNDER_TSAN
+#  include <sanitizer/tsan_interface.h>
+#endif
+
+QT_BEGIN_NAMESPACE
+
+namespace QtTsan {
+#ifdef QT_BUILDING_UNDER_TSAN
+inline void futexAcquire(void *addr, void *addr2 = nullptr)
+{
+    // A futex call ensures total ordering on the futex words
+    // (in either success or failure of the call). Instruct TSAN accordingly,
+    // as TSAN does not understand the futex(2) syscall (or equivalent).
+    ::__tsan_acquire(addr);
+    if (addr2)
+        ::__tsan_acquire(addr2);
+}
+
+inline void futexRelease(void *addr, void *addr2 = nullptr)
+{
+    if (addr2)
+        ::__tsan_release(addr2);
+    ::__tsan_release(addr);
+}
+
+inline void mutexPreLock(void *addr, unsigned flags)
+{
+    ::__tsan_mutex_pre_lock(addr, flags);
+}
+
+inline void mutexPostLock(void *addr, unsigned flags, int recursion)
+{
+    ::__tsan_mutex_post_lock(addr, flags, recursion);
+}
+
+inline void mutexPreUnlock(void *addr, unsigned flags)
+{
+    ::__tsan_mutex_pre_unlock(addr, flags);
+}
+
+inline void mutexPostUnlock(void *addr, unsigned flags)
+{
+    ::__tsan_mutex_post_unlock(addr, flags);
+}
+
+enum : unsigned {
+    MutexWriteReentrant = ::__tsan_mutex_write_reentrant,
+    TryLock = ::__tsan_mutex_try_lock,
+    TryLockFailed = ::__tsan_mutex_try_lock_failed,
+};
+#else
+inline void futexAcquire(void *, void * = nullptr) {}
+inline void futexRelease(void *, void * = nullptr) {}
+
+enum : unsigned {
+    MutexWriteReentrant,
+    TryLock,
+    TryLockFailed,
+};
+inline void mutexPreLock(void *, unsigned) {}
+inline void mutexPostLock(void *, unsigned, int) {}
+inline void mutexPreUnlock(void *, unsigned) {}
+inline void mutexPostUnlock(void *, unsigned) {}
+#endif // QT_BUILDING_UNDER_TSAN
+} // namespace QtTsan
+
+QT_END_NAMESPACE
+
+#endif // QTSAN_IMPL_H