diff --git a/libcxx/docs/ReleaseNotes/23.rst b/libcxx/docs/ReleaseNotes/23.rst index 312841c1de36..1213e8f43c8c 100644 --- a/libcxx/docs/ReleaseNotes/23.rst +++ b/libcxx/docs/ReleaseNotes/23.rst @@ -71,6 +71,9 @@ Improvements and New Features unspecified non-null string, so user code that does not match on the exact message remains correct. - ``ifstream::read`` has been optimized to pass through large reads to system calls directly instead of copying them in chunks. +- ``std::scoped_lock`` now carries Clang thread safety annotations when locking multiple mutexes, so + ``-Wthread-safety`` no longer incorrectly reports the guarded data accessed inside the block as + unprotected. This requires Clang 21 or later. Deprecations and Removals ------------------------- diff --git a/libcxx/include/__configuration/attributes.h b/libcxx/include/__configuration/attributes.h index c543ba9cb9e2..a49bedd1b874 100644 --- a/libcxx/include/__configuration/attributes.h +++ b/libcxx/include/__configuration/attributes.h @@ -342,10 +342,10 @@ # define _LIBCPP_TRY_ACQUIRE_SHARED_CAPABILITY(...) #endif -#if __has_cpp_attribute(_Clang::__release_capability__) -# define _LIBCPP_RELEASE_CAPABILITY [[_Clang::__release_capability__]] +#if __has_attribute(__release_capability__) +# define _LIBCPP_RELEASE_CAPABILITY(...) __attribute__((__release_capability__(__VA_ARGS__))) #else -# define _LIBCPP_RELEASE_CAPABILITY +# define _LIBCPP_RELEASE_CAPABILITY(...) #endif #if __has_cpp_attribute(_Clang::__release_shared_capability__) diff --git a/libcxx/include/__mutex/lock_guard.h b/libcxx/include/__mutex/lock_guard.h index 67a25ed2f8a8..29022e8c23ec 100644 --- a/libcxx/include/__mutex/lock_guard.h +++ b/libcxx/include/__mutex/lock_guard.h @@ -34,7 +34,7 @@ public: [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI lock_guard(mutex_type& __m, adopt_lock_t) _LIBCPP_REQUIRES_CAPABILITY(__m) : __m_(__m) {} - _LIBCPP_RELEASE_CAPABILITY _LIBCPP_HIDE_FROM_ABI ~lock_guard() { __m_.unlock(); } + _LIBCPP_RELEASE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI ~lock_guard() { __m_.unlock(); } lock_guard(lock_guard const&) = delete; lock_guard& operator=(lock_guard const&) = delete; diff --git a/libcxx/include/__mutex/mutex.h b/libcxx/include/__mutex/mutex.h index d0addacc2893..f71cba03cd4d 100644 --- a/libcxx/include/__mutex/mutex.h +++ b/libcxx/include/__mutex/mutex.h @@ -39,7 +39,7 @@ public: _LIBCPP_ACQUIRE_CAPABILITY() void lock(); [[__nodiscard__]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) bool try_lock() _NOEXCEPT; - _LIBCPP_RELEASE_CAPABILITY void unlock() _NOEXCEPT; + _LIBCPP_RELEASE_CAPABILITY() void unlock() _NOEXCEPT; typedef __libcpp_mutex_t* native_handle_type; [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI native_handle_type native_handle() { return &__m_; } diff --git a/libcxx/include/mutex b/libcxx/include/mutex index 75b080f8cd08..26ff343a21c1 100644 --- a/libcxx/include/mutex +++ b/libcxx/include/mutex @@ -458,7 +458,7 @@ public: __m_.lock(); } - _LIBCPP_RELEASE_CAPABILITY _LIBCPP_HIDE_FROM_ABI ~scoped_lock() { __m_.unlock(); } + _LIBCPP_RELEASE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI ~scoped_lock() { __m_.unlock(); } [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(adopt_lock_t, mutex_type& __m) _LIBCPP_REQUIRES_CAPABILITY(__m) @@ -469,25 +469,31 @@ public: }; template -class scoped_lock { +class _LIBCPP_SCOPED_LOCKABLE scoped_lock { static_assert(sizeof...(_MArgs) > 1, "At least 2 lock types required"); typedef tuple<_MArgs&...> _MutexTuple; public: - [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) : __t_(__margs...) { + [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit scoped_lock(_MArgs&... __margs) _LIBCPP_ACQUIRE_CAPABILITY(__margs...) + : __t_(__margs...) { std::lock(__margs...); } - [[nodiscard]] _LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) : __t_(__margs...) {} + [[nodiscard]] _LIBCPP_HIDE_FROM_ABI scoped_lock(adopt_lock_t, _MArgs&... __margs) + _LIBCPP_REQUIRES_CAPABILITY(__margs...) + : __t_(__margs...) {} - _LIBCPP_HIDE_FROM_ABI ~scoped_lock() { __unlock_unpack(make_index_sequence(), __t_); } + _LIBCPP_RELEASE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI ~scoped_lock() { + __unlock_unpack(make_index_sequence(), __t_); + } scoped_lock(scoped_lock const&) = delete; scoped_lock& operator=(scoped_lock const&) = delete; private: template - _LIBCPP_HIDE_FROM_ABI static void __unlock_unpack(index_sequence<_Indx...>, _MutexTuple& __mt) { + _LIBCPP_HIDE_FROM_ABI static void __unlock_unpack(index_sequence<_Indx...>, _MutexTuple& __mt) + _LIBCPP_RELEASE_CAPABILITY(std::get<_Indx>(__mt)...) { (std::get<_Indx>(__mt).unlock(), ...); } diff --git a/libcxx/include/shared_mutex b/libcxx/include/shared_mutex index 9479f0aa616f..8fc89fe1cb97 100644 --- a/libcxx/include/shared_mutex +++ b/libcxx/include/shared_mutex @@ -200,7 +200,7 @@ public: [[nodiscard]] _LIBCPP_TRY_ACQUIRE_CAPABILITY(true) _LIBCPP_HIDE_FROM_ABI bool try_lock() { return __base_.try_lock(); } - _LIBCPP_RELEASE_CAPABILITY _LIBCPP_HIDE_FROM_ABI void unlock() { return __base_.unlock(); } + _LIBCPP_RELEASE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI void unlock() { return __base_.unlock(); } // Shared ownership _LIBCPP_ACQUIRE_SHARED_CAPABILITY _LIBCPP_HIDE_FROM_ABI void lock_shared() { return __base_.lock_shared(); } @@ -262,7 +262,7 @@ public: return true; } - _LIBCPP_RELEASE_CAPABILITY void unlock(); + _LIBCPP_RELEASE_CAPABILITY() void unlock(); // Shared ownership _LIBCPP_ACQUIRE_SHARED_CAPABILITY void lock_shared(); diff --git a/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.pass.cpp b/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.pass.cpp new file mode 100644 index 000000000000..06751f9d4921 --- /dev/null +++ b/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.pass.cpp @@ -0,0 +1,69 @@ +//===----------------------------------------------------------------------===// +// +// 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: no-threads, c++03, c++11, c++14 + +// + +// GCC doesn't have thread safety attributes +// UNSUPPORTED: gcc + +// ADDITIONAL_COMPILE_FLAGS: -Wthread-safety + +#include + +std::mutex m1; +std::mutex m2; +int data1 __attribute__((guarded_by(m1))); +int data2 __attribute__((guarded_by(m2))); +int data3; + +static void no_mutex() { + std::scoped_lock<> lock; + data3++; +} + +static void one_mutex() { + std::scoped_lock lock(m1); + data1++; +} + +static void two_mutexes() { + std::scoped_lock lock(m1, m2); + data1++; + data2++; +} + +static void adopt_none() { + std::scoped_lock<> lock(std::adopt_lock); + data3++; +} + +static void adopt_one() { + m1.lock(); + std::scoped_lock lock(std::adopt_lock, m1); + data1++; +} + +static void adopt_two() { + m1.lock(); + m2.lock(); + std::scoped_lock lock(std::adopt_lock, m1, m2); + data1++; + data2++; +} + +int main(int, char**) { + no_mutex(); + one_mutex(); + two_mutexes(); + adopt_none(); + adopt_one(); + adopt_two(); + return 0; +} diff --git a/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.verify.cpp b/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.verify.cpp new file mode 100644 index 000000000000..3f9dd62c91f5 --- /dev/null +++ b/libcxx/test/extensions/clang/thread/thread.mutex/thread_safety_scoped_lock.verify.cpp @@ -0,0 +1,59 @@ +//===----------------------------------------------------------------------===// +// +// 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: no-threads, c++03, c++11, c++14 + +// + +// GCC doesn't have thread safety attributes +// UNSUPPORTED: gcc + +// ADDITIONAL_COMPILE_FLAGS: -Wthread-safety + +#include + +std::mutex m1; +std::mutex m2; +int data1 __attribute__((guarded_by(m1))); +int data2 __attribute__((guarded_by(m2))); + +void no_lock_write() { + data1++; // expected-warning {{writing variable 'data1' requires holding mutex 'm1' exclusively}} +} + +void no_lock_write2() { + std::scoped_lock<> lock; + data1++; // expected-warning {{writing variable 'data1' requires holding mutex 'm1' exclusively}} +} + +void wrong_lock_write() { + std::scoped_lock lock(m1); + data2++; // expected-warning {{writing variable 'data2' requires holding mutex 'm2' exclusively}} +} + +void adopt_without_lock() { + // expected-warning@+1 {{calling function 'scoped_lock' requires holding mutex 'm1' exclusively}} + std::scoped_lock lock(std::adopt_lock, m1); + data1++; // expected-warning {{writing variable 'data1' requires holding mutex 'm1' exclusively}} +} + +void adopt_two_only_one_locked() { + m1.lock(); + // expected-warning@+1 {{calling function 'scoped_lock' requires holding mutex 'm2' exclusively}} + std::scoped_lock lock(std::adopt_lock, m1, m2); + data1++; + data2++; // expected-warning {{writing variable 'data2' requires holding mutex 'm2' exclusively}} +} + +void after_scoped_lock_scope() { + { + std::scoped_lock lock(m1); + data1++; + } + data1++; // expected-warning {{writing variable 'data1' requires holding mutex 'm1' exclusively}} +}