[libcxx] Annotate shared_mutex with thread annotations

[phosek]

Summary

This change moves the thread annotations from __shared_mutex_base
to shared_mutex and introduces tests to ensure their functionality.

[ldionne]

When are those annotations enabled? By default they're turned off, would it make sense to turn them on by default? It seems like we're missing out on something interesting here.

[ldionne]

But I don't have any objection with the patch itself.

[mclow.lists]

It would be nice to have a test (in test/libcxx) to check that these annotations are in the expected places.

[ldionne]

Ping @phosek . Are we doing this?

[ldionne]

Is there still interest in doing this? If so, I actually think we should test those annotations, perhaps by enabling them in some CI configuration (TSAN?). How can we do that? Will request changes so it shows up in the queue properly.

[phosek]

In D67466#1666937, @ldionne wrote:

When are those annotations enabled? By default they're turned off, would it make sense to turn them on by default? It seems like we're missing out on something interesting here.

These are statically checked by the compiler when _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS is defined. I've included tests to cover the newly added annotations similarly to how we already test annotations on std::mutex.

[ldionne]

libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety_missing_unlock.fail.cpp

1 ↗

(On Diff #363556)

Can we make this a .verify.cpp instead?

libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety_missing_unlock_shared.fail.cpp

1 ↗

(On Diff #363556)

.verify.cpp

libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety_requires_capability.fail.cpp

1 ↗

(On Diff #363556)

.verify.cpp

38–46 ↗

(On Diff #363556)

Is this actually required to trigger the error? If not, let's not have a main function at all. If so, I don't understand how this works!

libcxx/include/__mutex_base

public:
     ~lock_guard() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability()) {__m_.unlock();}
 
 private:
     lock_guard(lock_guard const&) _LIBCPP_EQUAL_DELETE;
     lock_guard& operator=(lock_guard const&) _LIBCPP_EQUAL_DELETE;
 };
 
 template <class _Mutex>
-class _LIBCPP_TEMPLATE_VIS unique_lock
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_THREAD_SAFETY_ANNOTATION(scoped_lockable) unique_lock
 {
 public:
     typedef _Mutex mutex_type;
 
 private:
     mutex_type* __m_;
     bool __owns_;
 
 public:
     _LIBCPP_INLINE_VISIBILITY
     unique_lock() _NOEXCEPT : __m_(nullptr), __owns_(false) {}
     _LIBCPP_INLINE_VISIBILITY
-    explicit unique_lock(mutex_type& __m)
+    explicit unique_lock(mutex_type& __m) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability(__m))
         : __m_(_VSTD::addressof(__m)), __owns_(true) {__m_->lock();}
     _LIBCPP_INLINE_VISIBILITY
     unique_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
         : __m_(_VSTD::addressof(__m)), __owns_(false) {}
     _LIBCPP_INLINE_VISIBILITY
     unique_lock(mutex_type& __m, try_to_lock_t)
         : __m_(_VSTD::addressof(__m)), __owns_(__m.try_lock()) {}
     _LIBCPP_INLINE_VISIBILITY
-    unique_lock(mutex_type& __m, adopt_lock_t)
+    unique_lock(mutex_type& __m, adopt_lock_t) _LIBCPP_THREAD_SAFETY_ANNOTATION(requires_capability(__m))
         : __m_(_VSTD::addressof(__m)), __owns_(true) {}
     template <class _Clock, class _Duration>
     _LIBCPP_INLINE_VISIBILITY
         unique_lock(mutex_type& __m, const chrono::time_point<_Clock, _Duration>& __t)
             : __m_(_VSTD::addressof(__m)), __owns_(__m.try_lock_until(__t)) {}
     template <class _Rep, class _Period>
     _LIBCPP_INLINE_VISIBILITY
         unique_lock(mutex_type& __m, const chrono::duration<_Rep, _Period>& __d)
             : __m_(_VSTD::addressof(__m)), __owns_(__m.try_lock_for(__d)) {}
     _LIBCPP_INLINE_VISIBILITY
-    ~unique_lock()
+    ~unique_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability())
     {
         if (__owns_)
             __m_->unlock();
     }
 
 private:
     unique_lock(unique_lock const&); // = delete;
     unique_lock& operator=(unique_lock const&); // = delete;
unique_lock& operator=(unique_lock&& __u) _NOEXCEPT
             __owns_ = __u.__owns_;
             __u.__m_ = nullptr;
             __u.__owns_ = false;
             return *this;
         }
 
 #endif  // _LIBCPP_CXX03_LANG
 
-    void lock();
-    bool try_lock();
+    void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability());
+    bool try_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
 
     template <class _Rep, class _Period>
-        bool try_lock_for(const chrono::duration<_Rep, _Period>& __d);
+        bool try_lock_for(const chrono::duration<_Rep, _Period>& __d)
+        _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
     template <class _Clock, class _Duration>
-        bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t);
+        bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __t)
+        _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
 
-    void unlock();
+    void unlock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability());
 
     _LIBCPP_INLINE_VISIBILITY
     void swap(unique_lock& __u) _NOEXCEPT
     {
         _VSTD::swap(__m_, __u.__m_);
         _VSTD::swap(__owns_, __u.__owns_);
     }
     _LIBCPP_INLINE_VISIBILITY
     mutex_type* release() _NOEXCEPT
     {
         mutex_type* __m = __m_;
         __m_ = nullptr;
         __owns_ = false;
         return __m;
     }
 
     _LIBCPP_INLINE_VISIBILITY
     bool owns_lock() const _NOEXCEPT {return __owns_;}
     _LIBCPP_INLINE_VISIBILITY
     _LIBCPP_EXPLICIT
         operator bool () const _NOEXCEPT {return __owns_;}
-    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(return_capability(__m_))
     mutex_type* mutex() const _NOEXCEPT {return __m_;}
 };
 
 template <class _Mutex>
 void
 unique_lock<_Mutex>::lock()
 {
     if (__m_ == nullptr)

libcxx/include/shared_mutex

__shared_mutex_base
     void unlock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_shared_capability());
 
 //     typedef implementation-defined native_handle_type; // See 30.2.3
 //     native_handle_type native_handle(); // See 30.2.3
 };
 
 
 #if _LIBCPP_STD_VER > 14
-class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_mutex
+class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX _LIBCPP_THREAD_SAFETY_ANNOTATION(capability("shared_mutex")) shared_mutex
 {
     __shared_mutex_base __base;
 public:
     _LIBCPP_INLINE_VISIBILITY shared_mutex() : __base() {}
     _LIBCPP_INLINE_VISIBILITY ~shared_mutex() = default;
 
     shared_mutex(const shared_mutex&) = delete;
     shared_mutex& operator=(const shared_mutex&) = delete;
 
     // Exclusive ownership
-    _LIBCPP_INLINE_VISIBILITY void lock()     { return __base.lock(); }
-    _LIBCPP_INLINE_VISIBILITY bool try_lock() { return __base.try_lock(); }
-    _LIBCPP_INLINE_VISIBILITY void unlock()   { return __base.unlock(); }
+    _LIBCPP_INLINE_VISIBILITY void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability())
+        { return __base.lock(); }
+    _LIBCPP_INLINE_VISIBILITY bool try_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true))
+        { return __base.try_lock(); }
+    _LIBCPP_INLINE_VISIBILITY void unlock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability())
+        { return __base.unlock(); }
 
     // Shared ownership
-    _LIBCPP_INLINE_VISIBILITY void lock_shared()     { return __base.lock_shared(); }
-    _LIBCPP_INLINE_VISIBILITY bool try_lock_shared() { return __base.try_lock_shared(); }
-    _LIBCPP_INLINE_VISIBILITY void unlock_shared()   { return __base.unlock_shared(); }
+    _LIBCPP_INLINE_VISIBILITY void lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_shared_capability())
+        { return __base.lock_shared(); }
+    _LIBCPP_INLINE_VISIBILITY bool try_lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_shared_capability(true))
+        { return __base.try_lock_shared(); }
+    _LIBCPP_INLINE_VISIBILITY void unlock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_shared_capability())
+        { return __base.unlock_shared(); }
 
 //     typedef __shared_mutex_base::native_handle_type native_handle_type;
 //     _LIBCPP_INLINE_VISIBILITY native_handle_type native_handle() { return __base::unlock_shared(); }
 };
 #endif
 
 
 class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_timed_mutex
shared_timed_mutex::try_lock_shared_until(
     }
     unsigned __num_readers = (__base.__state_ & __base.__n_readers_) + 1;
     __base.__state_ &= ~__base.__n_readers_;
     __base.__state_ |= __num_readers;
     return true;
 }
 
 template <class _Mutex>
-class shared_lock
+class _LIBCPP_THREAD_SAFETY_ANNOTATION(scoped_lockable) shared_lock
 {
 public:
     typedef _Mutex mutex_type;
 
 private:
     mutex_type* __m_;
     bool __owns_;
 
 public:
     _LIBCPP_INLINE_VISIBILITY
     shared_lock() _NOEXCEPT
         : __m_(nullptr),
           __owns_(false)
         {}
 
     _LIBCPP_INLINE_VISIBILITY
-    explicit shared_lock(mutex_type& __m)
+    explicit shared_lock(mutex_type& __m) _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_shared_capability(__m))
         : __m_(_VSTD::addressof(__m)),
           __owns_(true)
         {__m_->lock_shared();}
 
     _LIBCPP_INLINE_VISIBILITY
     shared_lock(mutex_type& __m, defer_lock_t) _NOEXCEPT
         : __m_(_VSTD::addressof(__m)),
           __owns_(false)
         {}
 
     _LIBCPP_INLINE_VISIBILITY
     shared_lock(mutex_type& __m, try_to_lock_t)
         : __m_(_VSTD::addressof(__m)),
           __owns_(__m.try_lock_shared())
         {}
 
     _LIBCPP_INLINE_VISIBILITY
-    shared_lock(mutex_type& __m, adopt_lock_t)
+    shared_lock(mutex_type& __m, adopt_lock_t) _LIBCPP_THREAD_SAFETY_ANNOTATION(requires_shared_capability(__m))
         : __m_(_VSTD::addressof(__m)),
           __owns_(true)
         {}
 
     template <class _Clock, class _Duration>
         _LIBCPP_INLINE_VISIBILITY
         shared_lock(mutex_type& __m,
                     const chrono::time_point<_Clock, _Duration>& __abs_time)
             : __m_(_VSTD::addressof(__m)),
               __owns_(__m.try_lock_shared_until(__abs_time))
             {}
 
     template <class _Rep, class _Period>
         _LIBCPP_INLINE_VISIBILITY
         shared_lock(mutex_type& __m,
                     const chrono::duration<_Rep, _Period>& __rel_time)
             : __m_(_VSTD::addressof(__m)),
               __owns_(__m.try_lock_shared_for(__rel_time))
             {}
 
     _LIBCPP_INLINE_VISIBILITY
-    ~shared_lock()
+    ~shared_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability(__m))
     {
         if (__owns_)
             __m_->unlock_shared();
     }
 
     shared_lock(shared_lock const&) = delete;
     shared_lock& operator=(shared_lock const&) = delete;
 
shared_lock& operator=(shared_lock&& __u) _NOEXCEPT
         __owns_ = false;
         __m_ = __u.__m_;
         __owns_ = __u.__owns_;
         __u.__m_ = nullptr;
         __u.__owns_ = false;
         return *this;
     }
 
-    void lock();
-    bool try_lock();
+    void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_shared_capability());
+    bool try_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_shared_capability(true));
     template <class Rep, class Period>
-        bool try_lock_for(const chrono::duration<Rep, Period>& rel_time);
+        bool try_lock_for(const chrono::duration<Rep, Period>& rel_time)
+        _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
     template <class Clock, class Duration>
-        bool try_lock_until(const chrono::time_point<Clock, Duration>& abs_time);
-    void unlock();
+        bool try_lock_until(const chrono::time_point<Clock, Duration>& abs_time)
+        _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
+    void unlock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_shared_capability());
 
     // Setters
     _LIBCPP_INLINE_VISIBILITY
     void swap(shared_lock& __u) _NOEXCEPT
     {
         _VSTD::swap(__m_, __u.__m_);
         _VSTD::swap(__owns_, __u.__owns_);
     }
public:
 
     // Getters
     _LIBCPP_INLINE_VISIBILITY
     bool owns_lock() const _NOEXCEPT {return __owns_;}
 
     _LIBCPP_INLINE_VISIBILITY
     explicit operator bool () const _NOEXCEPT {return __owns_;}
 
-    _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_THREAD_SAFETY_ANNOTATION(return_capability(__m_))
     mutex_type* mutex() const _NOEXCEPT {return __m_;}
 };
 
 template <class _Mutex>
 void
 shared_lock<_Mutex>::lock()
 {
     if (__m_ == nullptr)