[libcxx-commits] [libcxx] 5b666cf - [libc++] Fix thread annotations on shared_mutex and shared_timed_mutex

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 6 05:37:14 PDT 2023


Author: Louis Dionne
Date: 2023-07-06T08:37:10-04:00
New Revision: 5b666cf11e7a913fdfd0a65975ff587452635659

URL: https://github.com/llvm/llvm-project/commit/5b666cf11e7a913fdfd0a65975ff587452635659
DIFF: https://github.com/llvm/llvm-project/commit/5b666cf11e7a913fdfd0a65975ff587452635659.diff

LOG: [libc++] Fix thread annotations on shared_mutex and shared_timed_mutex

Based on the comment in https://reviews.llvm.org/D54290#4418958, these
attributes need to be on the top-level functions in order to work
properly. Also, add tests.

Fixes http://llvm.org/PR57035.

Differential Revision: https://reviews.llvm.org/D154354

Added: 
    libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety.verify.cpp
    libcxx/test/libcxx/thread/thread.shared_timed_mutex/thread_safety.verify.cpp

Modified: 
    libcxx/.clang-format
    libcxx/include/shared_mutex

Removed: 
    


################################################################################
diff  --git a/libcxx/.clang-format b/libcxx/.clang-format
index f7cf25b6986b17..4389acecf89c7d 100644
--- a/libcxx/.clang-format
+++ b/libcxx/.clang-format
@@ -52,6 +52,7 @@ AttributeMacros: [
                   '_LIBCPP_STANDALONE_DEBUG',
                   '_LIBCPP_TEMPLATE_DATA_VIS',
                   '_LIBCPP_TEMPLATE_VIS',
+                  '_LIBCPP_THREAD_SAFETY_ANNOTATION',
                   '_LIBCPP_USING_IF_EXISTS',
                   '_LIBCPP_WEAK',
                  ]

diff  --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex
index 6b6291e5f68600..b627e85885c6a7 100644
--- a/libcxx/include/shared_mutex
+++ b/libcxx/include/shared_mutex
@@ -153,8 +153,7 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-struct _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_SHARED_MUTEX
-_LIBCPP_THREAD_SAFETY_ANNOTATION(capability("shared_mutex")) __shared_mutex_base {
+struct _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_SHARED_MUTEX __shared_mutex_base {
   mutex __mut_;
   condition_variable __gate1_;
   condition_variable __gate2_;
@@ -170,21 +169,22 @@ _LIBCPP_THREAD_SAFETY_ANNOTATION(capability("shared_mutex")) __shared_mutex_base
   __shared_mutex_base& operator=(const __shared_mutex_base&) = delete;
 
   // Exclusive ownership
-  void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability()); // blocking
-  bool try_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
-  void unlock() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability());
+  void lock(); // blocking
+  bool try_lock();
+  void unlock();
 
   // Shared ownership
-  void lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_shared_capability()); // blocking
-  bool try_lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_shared_capability(true));
-  void unlock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(release_shared_capability());
+  void lock_shared(); // blocking
+  bool try_lock_shared();
+  void unlock_shared();
 
   //     typedef implementation-defined native_handle_type; // See 30.2.3
   //     native_handle_type native_handle(); // See 30.2.3
 };
 
 #  if _LIBCPP_STD_VER >= 17
-class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_mutex {
+class _LIBCPP_EXPORTED_FROM_ABI
+    _LIBCPP_AVAILABILITY_SHARED_MUTEX _LIBCPP_THREAD_SAFETY_ANNOTATION(__capability__("shared_mutex")) shared_mutex {
   __shared_mutex_base __base_;
 
 public:
@@ -195,21 +195,35 @@ public:
   shared_mutex& operator=(const shared_mutex&) = delete;
 
   // Exclusive ownership
-  _LIBCPP_HIDE_FROM_ABI void lock() { return __base_.lock(); }
-  _LIBCPP_HIDE_FROM_ABI bool try_lock() { return __base_.try_lock(); }
-  _LIBCPP_HIDE_FROM_ABI void unlock() { return __base_.unlock(); }
+  _LIBCPP_HIDE_FROM_ABI void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(__acquire_capability__()) {
+    return __base_.lock();
+  }
+  _LIBCPP_HIDE_FROM_ABI bool try_lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_capability__(true)) {
+    return __base_.try_lock();
+  }
+  _LIBCPP_HIDE_FROM_ABI void unlock() _LIBCPP_THREAD_SAFETY_ANNOTATION(__release_capability__()) {
+    return __base_.unlock();
+  }
 
   // Shared ownership
-  _LIBCPP_HIDE_FROM_ABI void lock_shared() { return __base_.lock_shared(); }
-  _LIBCPP_HIDE_FROM_ABI bool try_lock_shared() { return __base_.try_lock_shared(); }
-  _LIBCPP_HIDE_FROM_ABI void unlock_shared() { return __base_.unlock_shared(); }
+  _LIBCPP_HIDE_FROM_ABI void lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(__acquire_shared_capability__()) {
+    return __base_.lock_shared();
+  }
+  _LIBCPP_HIDE_FROM_ABI bool try_lock_shared()
+      _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_shared_capability__(true)) {
+    return __base_.try_lock_shared();
+  }
+  _LIBCPP_HIDE_FROM_ABI 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_HIDE_FROM_ABI native_handle_type native_handle() { return __base::unlock_shared(); }
 };
 #  endif
 
-class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_timed_mutex {
+class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_SHARED_MUTEX _LIBCPP_THREAD_SAFETY_ANNOTATION(
+    __capability__("shared_timed_mutex")) shared_timed_mutex {
   __shared_mutex_base __base_;
 
 public:
@@ -220,28 +234,32 @@ public:
   shared_timed_mutex& operator=(const shared_timed_mutex&) = delete;
 
   // Exclusive ownership
-  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>
-  _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time) {
+  _LIBCPP_HIDE_FROM_ABI bool try_lock_for(const chrono::duration<_Rep, _Period>& __rel_time)
+      _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_capability__(true)) {
     return try_lock_until(chrono::steady_clock::now() + __rel_time);
   }
   template <class _Clock, class _Duration>
   _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS bool
-  try_lock_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
-  void unlock();
+  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_capability__());
 
   // Shared ownership
-  void lock_shared();
-  bool try_lock_shared();
+  void lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(__acquire_shared_capability__());
+  bool try_lock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_shared_capability__(true));
   template <class _Rep, class _Period>
-  _LIBCPP_HIDE_FROM_ABI bool try_lock_shared_for(const chrono::duration<_Rep, _Period>& __rel_time) {
+  _LIBCPP_HIDE_FROM_ABI bool try_lock_shared_for(const chrono::duration<_Rep, _Period>& __rel_time)
+      _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_shared_capability__(true)) {
     return try_lock_shared_until(chrono::steady_clock::now() + __rel_time);
   }
   template <class _Clock, class _Duration>
   _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS bool
-  try_lock_shared_until(const chrono::time_point<_Clock, _Duration>& __abs_time);
-  void unlock_shared();
+  try_lock_shared_until(const chrono::time_point<_Clock, _Duration>& __abs_time)
+      _LIBCPP_THREAD_SAFETY_ANNOTATION(__try_acquire_shared_capability__(true));
+  void unlock_shared() _LIBCPP_THREAD_SAFETY_ANNOTATION(__release_shared_capability__());
 };
 
 template <class _Clock, class _Duration>

diff  --git a/libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety.verify.cpp b/libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety.verify.cpp
new file mode 100644
index 00000000000000..19d8aab292b87a
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.shared_mutex/thread_safety.verify.cpp
@@ -0,0 +1,65 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+// UNSUPPORTED: no-threads
+// UNSUPPORTED: availability-shared_mutex-missing
+// REQUIRES: thread-safety
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+// On Windows Clang bugs out when both __declspec and __attribute__ are present,
+// the processing goes awry preventing the definition of the types.
+// XFAIL: msvc
+
+// <shared_mutex>
+//
+// class shared_mutex;
+//
+// void lock();
+// bool try_lock();
+// void unlock();
+//
+// void lock_shared();
+// bool try_lock_shared();
+// void unlock_shared();
+
+#include <shared_mutex>
+
+std::shared_mutex m;
+int data __attribute__((guarded_by(m))) = 0;
+void read(int);
+
+void f() {
+  // Exclusive locking
+  {
+    m.lock();
+    ++data; // ok
+    m.unlock();
+  }
+  {
+    if (m.try_lock()) {
+      ++data; // ok
+      m.unlock();
+    }
+  }
+
+  // Shared locking
+  {
+    m.lock_shared();
+    read(data); // ok
+    ++data; // expected-error {{writing variable 'data' requires holding shared_mutex 'm' exclusively}}
+    m.unlock_shared();
+  }
+  {
+    if (m.try_lock_shared()) {
+      read(data); // ok
+      ++data; // expected-error {{writing variable 'data' requires holding shared_mutex 'm' exclusively}}
+      m.unlock_shared();
+    }
+  }
+}

diff  --git a/libcxx/test/libcxx/thread/thread.shared_timed_mutex/thread_safety.verify.cpp b/libcxx/test/libcxx/thread/thread.shared_timed_mutex/thread_safety.verify.cpp
new file mode 100644
index 00000000000000..9fa1ef7d9996e8
--- /dev/null
+++ b/libcxx/test/libcxx/thread/thread.shared_timed_mutex/thread_safety.verify.cpp
@@ -0,0 +1,96 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11
+// UNSUPPORTED: no-threads
+// UNSUPPORTED: availability-shared_mutex-missing
+// REQUIRES: thread-safety
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+
+// On Windows Clang bugs out when both __declspec and __attribute__ are present,
+// the processing goes awry preventing the definition of the types.
+// XFAIL: msvc
+
+// <shared_mutex>
+//
+// class shared_timed_mutex;
+//
+// void lock();
+// bool try_lock();
+// bool try_lock_for(const std::chrono::duration<Rep, Period>&);
+// bool try_lock_until(const std::chrono::time_point<Clock, Duration>&);
+// void unlock();
+//
+// void lock_shared();
+// bool try_lock_shared();
+// bool try_lock_shared_for(const std::chrono::duration<Rep, Period>&);
+// bool try_lock_shared_until(const std::chrono::time_point<Clock, Duration>&);
+// void unlock_shared();
+
+#include <chrono>
+#include <shared_mutex>
+
+std::shared_timed_mutex m;
+int data __attribute__((guarded_by(m))) = 0;
+void read(int);
+
+void f(std::chrono::time_point<std::chrono::steady_clock> tp, std::chrono::milliseconds d) {
+  // Exclusive locking
+  {
+    m.lock();
+    ++data; // ok
+    m.unlock();
+  }
+  {
+    if (m.try_lock()) {
+      ++data; // ok
+      m.unlock();
+    }
+  }
+  {
+    if (m.try_lock_for(d)) {
+      ++data; // ok
+      m.unlock();
+    }
+  }
+  {
+    if (m.try_lock_until(tp)) {
+      ++data; // ok
+      m.unlock();
+    }
+  }
+
+  // Shared locking
+  {
+    m.lock_shared();
+    read(data); // ok
+    ++data; // expected-error {{writing variable 'data' requires holding shared_timed_mutex 'm' exclusively}}
+    m.unlock_shared();
+  }
+  {
+    if (m.try_lock_shared()) {
+      read(data); // ok
+      ++data; // expected-error {{writing variable 'data' requires holding shared_timed_mutex 'm' exclusively}}
+      m.unlock_shared();
+    }
+  }
+  {
+    if (m.try_lock_shared_for(d)) {
+      read(data); // ok
+      ++data; // expected-error {{writing variable 'data' requires holding shared_timed_mutex 'm' exclusively}}
+      m.unlock_shared();
+    }
+  }
+  {
+    if (m.try_lock_shared_until(tp)) {
+      read(data); // ok
+      ++data; // expected-error {{writing variable 'data' requires holding shared_timed_mutex 'm' exclusively}}
+      m.unlock_shared();
+    }
+  }
+}


        


More information about the libcxx-commits mailing list