[core] Added Clang TSA attributes. (#2000)

- Added CMake build option ENABLE_CLANG_TSA.
- srt::sync::UniqueLock now may throw an exception in unlock and lock.
- MSVC SAL attributes are partially added.

CMakeLists.txt

@@ -136,6 +136,8 @@ option(USE_BUSY_WAITING "Enable more accurate sending times at a cost of potenti
 option(USE_GNUSTL "Get c++ library/headers from the gnustl.pc" OFF)
 option(ENABLE_SOCK_CLOEXEC "Enable setting SOCK_CLOEXEC on a socket" ON)
 
+option(ENABLE_CLANG_TSA "Enable Clang Thread Safety Analysis" OFF)
+
 set(TARGET_srt "srt" CACHE STRING "The name for the SRT library")
 
 # Use application-defined group reader
@@ -634,6 +636,11 @@ if (ENABLE_THREAD_CHECK)
 	)
 endif()
 
+if (ENABLE_CLANG_TSA)
+	list(APPEND SRT_EXTRA_CFLAGS "-Wthread-safety")
+	message(STATUS "Clang TSA: Enabled")
+endif()
+
 if (ENABLE_PROFILE)
 	if (HAVE_COMPILER_GNU_COMPAT)
 		# They are actually cflags, not definitions, but CMake is stupid enough.

srtcore/logging.h

@@ -131,7 +131,10 @@ struct LogConfig
     {
     }
 
+    SRT_ATTR_ACQUIRE(mutex)
     void lock() { mutex.lock(); }
+
+    SRT_ATTR_RELEASE(mutex)
     void unlock() { mutex.unlock(); }
 };
 

srtcore/platform_sys.h

@@ -41,7 +41,7 @@
    #include <stdint.h>
    #include <inttypes.h>
    #if defined(_MSC_VER)
-      #pragma warning(disable:4251)
+      #pragma warning(disable: 4251 26812)
    #endif
 #else
 

srtcore/srt.h

@@ -963,7 +963,7 @@ typedef struct SRT_EPOLL_EVENT_STR
     int       events; // SRT_EPOLL_IN | SRT_EPOLL_OUT | SRT_EPOLL_ERR
 #ifdef __cplusplus
     SRT_EPOLL_EVENT_STR(SRTSOCKET s, int ev): fd(s), events(ev) {}
-    SRT_EPOLL_EVENT_STR() {} // NOTE: allows singular values, no init.
+    SRT_EPOLL_EVENT_STR(): fd(-1), events(0) {} // NOTE: allows singular values, no init.
 #endif
 } SRT_EPOLL_EVENT;
 SRT_API int srt_epoll_uwait(int eid, SRT_EPOLL_EVENT* fdsSet, int fdsSize, int64_t msTimeOut);

srtcore/srt_attr_defs.h

@@ -0,0 +1,119 @@
+/*
+ * SRT - Secure, Reliable, Transport
+ * Copyright (c) 2019 Haivision Systems Inc.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+/*****************************************************************************
+The file contains various planform and compiler dependent attribute definitions
+used by SRT library internally.
+
+1. Attributes for thread safety analysis
+   - Clang (https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader).
+   - Other compilers: none.
+
+ *****************************************************************************/
+
+#ifndef INC_SRT_ATTR_DEFS_H
+#define INC_SRT_ATTR_DEFS_H
+
+#if _MSC_VER >= 1920
+// In case of MSVC these attributes have to preceed the attributed objects (variable, function).
+// E.g. SRT_ATTR_GUARDED_BY(mtx) int object;
+// It is tricky to annotate e.g. the following function, as clang complaints it does not know 'm'.
+// SRT_ATTR_EXCLUDES(m) SRT_ATTR_ACQUIRE(m)
+// inline void enterCS(Mutex& m) { m.lock(); }
+#define SRT_ATTR_CAPABILITY(expr)
+#define SRT_ATTR_SCOPED_CAPABILITY
+#define SRT_ATTR_GUARDED_BY(expr) _Guarded_by_(expr)
+#define SRT_ATTR_PT_GUARDED_BY(expr)
+#define SRT_ATTR_ACQUIRED_BEFORE(...)
+#define SRT_ATTR_ACQUIRED_AFTER(...)
+#define SRT_ATTR_REQUIRES(expr) _Requires_lock_held_(expr)
+#define SRT_ATTR_REQUIRES_SHARED(...)
+#define SRT_ATTR_ACQUIRE(expr) _Acquires_nonreentrant_lock_(expr)
+#define SRT_ATTR_ACQUIRE_SHARED(...)
+#define SRT_ATTR_RELEASE(expr) _Releases_lock_(expr)
+#define SRT_ATTR_RELEASE_SHARED(...)
+#define SRT_ATTR_RELEASE_GENERIC(...)
+#define SRT_ATTR_TRY_ACQUIRE(...) _Acquires_nonreentrant_lock_(expr)
+#define SRT_ATTR_TRY_ACQUIRE_SHARED(...)
+#define SRT_ATTR_EXCLUDES(...)
+#define SRT_ATTR_ASSERT_CAPABILITY(expr)
+#define SRT_ATTR_ASSERT_SHARED_CAPABILITY(x)
+#define SRT_ATTR_RETURN_CAPABILITY(x)
+#define SRT_ATTR_NO_THREAD_SAFETY_ANALYSIS
+#else
+
+#if defined(__clang__)
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)   __attribute__((x))
+#else
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)   // no-op
+#endif
+
+#define SRT_ATTR_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
+
+#define SRT_ATTR_SCOPED_CAPABILITY \
+  THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+
+#define SRT_ATTR_GUARDED_BY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+
+#define SRT_ATTR_PT_GUARDED_BY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x))
+
+#define SRT_ATTR_ACQUIRED_BEFORE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__))
+
+#define SRT_ATTR_ACQUIRED_AFTER(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__))
+
+#define SRT_ATTR_REQUIRES(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__))
+
+#define SRT_ATTR_REQUIRES_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__))
+
+#define SRT_ATTR_ACQUIRE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__))
+
+#define SRT_ATTR_ACQUIRE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__))
+
+#define SRT_ATTR_RELEASE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__))
+
+#define SRT_ATTR_RELEASE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__))
+
+#define SRT_ATTR_RELEASE_GENERIC(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(release_generic_capability(__VA_ARGS__))
+
+#define SRT_ATTR_TRY_ACQUIRE(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__))
+
+#define SRT_ATTR_TRY_ACQUIRE_SHARED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__))
+
+#define SRT_ATTR_EXCLUDES(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__))
+
+#define SRT_ATTR_ASSERT_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x))
+
+#define SRT_ATTR_ASSERT_SHARED_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x))
+
+#define SRT_ATTR_RETURN_CAPABILITY(x) \
+  THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+
+#define SRT_ATTR_NO_THREAD_SAFETY_ANALYSIS \
+  THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+#endif // not _MSC_VER
+
+#endif // INC_SRT_ATTR_DEFS_H

srtcore/sync.h

@@ -53,6 +53,7 @@
 
 #include "srt.h"
 #include "utilities.h"
+#include "srt_attr_defs.h"
 
 class CUDTException;    // defined in common.h
 
@@ -379,7 +380,7 @@ using ScopedLock = lock_guard<mutex>;
 /// Mutex is a class wrapper, that should mimic the std::chrono::mutex class.
 /// At the moment the extra function ref() is temporally added to allow calls
 /// to pthread_cond_timedwait(). Will be removed by introducing CEvent.
-class Mutex
+class SRT_ATTR_CAPABILITY("mutex") Mutex
 {
     friend class SyncEvent;
 
@@ -388,11 +389,11 @@ class Mutex
     ~Mutex();
 
 public:
-    int lock();
-    int unlock();
+    int lock() SRT_ATTR_ACQUIRE();
+    int unlock() SRT_ATTR_RELEASE();
 
     /// @return     true if the lock was acquired successfully, otherwise false
-    bool try_lock();
+    bool try_lock() SRT_ATTR_TRY_ACQUIRE(true);
 
     // TODO: To be removed with introduction of the CEvent.
     pthread_mutex_t& ref() { return m_mutex; }
@@ -402,28 +403,39 @@ class Mutex
 };
 
 /// A pthread version of std::chrono::scoped_lock<mutex> (or lock_guard for C++11)
-class ScopedLock
+class SRT_ATTR_SCOPED_CAPABILITY ScopedLock
 {
 public:
+    SRT_ATTR_ACQUIRE(m)
     ScopedLock(Mutex& m);
+
+    SRT_ATTR_RELEASE()
     ~ScopedLock();
 
 private:
     Mutex& m_mutex;
 };
 
 /// A pthread version of std::chrono::unique_lock<mutex>
-class UniqueLock
+class SRT_ATTR_SCOPED_CAPABILITY UniqueLock
 {
     friend class SyncEvent;
 
 public:
+    SRT_ATTR_ACQUIRE(m_Mutex)
     UniqueLock(Mutex &m);
+
+    SRT_ATTR_RELEASE(m_Mutex)
     ~UniqueLock();
 
 public:
+    SRT_ATTR_ACQUIRE(m_Mutex)
     void lock();
+
+    SRT_ATTR_RELEASE(m_Mutex)
     void unlock();
+
+    SRT_ATTR_RETURN_CAPABILITY(m_Mutex)
     Mutex* mutex(); // reflects C++11 unique_lock::mutex()
 
 private:
@@ -432,26 +444,28 @@ class UniqueLock
 };
 #endif // ENABLE_STDCXX_SYNC
 
-inline void enterCS(Mutex& m) { m.lock(); }
-inline bool tryEnterCS(Mutex& m) { return m.try_lock(); }
-inline void leaveCS(Mutex& m) { m.unlock(); }
+inline void enterCS(Mutex& m) SRT_ATTR_EXCLUDES(m) SRT_ATTR_ACQUIRE(m) { m.lock(); }
+
+inline bool tryEnterCS(Mutex& m) SRT_ATTR_TRY_ACQUIRE(true, m) { return m.try_lock(); }
+
+inline void leaveCS(Mutex& m) SRT_ATTR_REQUIRES(m) SRT_ATTR_RELEASE(m) { m.unlock(); }
 
 class InvertedLock
 {
-    Mutex *m_pMutex;
+    Mutex& m_mtx;
 
-  public:
+public:
+    SRT_ATTR_REQUIRES(m) SRT_ATTR_RELEASE(m)
     InvertedLock(Mutex& m)
-        : m_pMutex(&m)
+        : m_mtx(m)
     {
-        leaveCS(*m_pMutex);
+        m_mtx.unlock();
     }
 
+    SRT_ATTR_ACQUIRE(m_mtx)
     ~InvertedLock()
     {
-        if (!m_pMutex)
-            return;
-        enterCS(*m_pMutex);
+        m_mtx.lock();
     }
 };
 

srtcore/sync_posix.cpp

@@ -244,22 +244,27 @@ srt::sync::UniqueLock::UniqueLock(Mutex& m)
 
 srt::sync::UniqueLock::~UniqueLock()
 {
-    unlock();
+    if (m_iLocked == 0)
+    {
+        unlock();
+    }
 }
 
 void srt::sync::UniqueLock::lock()
 {
-    if (m_iLocked == -1)
-        m_iLocked = m_Mutex.lock();
+    if (m_iLocked != -1)
+        throw CThreadException(MJ_SYSTEMRES, MN_THREAD, 0);
+
+    m_iLocked = m_Mutex.lock();
 }
 
 void srt::sync::UniqueLock::unlock()
 {
-    if (m_iLocked == 0)
-    {
-        m_Mutex.unlock();
-        m_iLocked = -1;
-    }
+    if (m_iLocked != 0)
+        throw CThreadException(MJ_SYSTEMRES, MN_THREAD, 0);
+
+    m_Mutex.unlock();
+    m_iLocked = -1;
 }
 
 srt::sync::Mutex* srt::sync::UniqueLock::mutex()

srtcore/udt.h

@@ -70,10 +70,10 @@ modified by
 #include "srt.h"
 
 /*
-* SRT_ENABLE_THREADCHECK (THIS IS SET IN MAKEFILE NOT HERE)
+* SRT_ENABLE_THREADCHECK IS SET IN MAKEFILE, NOT HERE
 */
 #if defined(SRT_ENABLE_THREADCHECK)
-#include <threadcheck.h>
+#include "threadcheck.h"
 #else
 #define THREAD_STATE_INIT(name)
 #define THREAD_EXIT()