[libc++][ThreadSafety] Add thread safety annotations for variadic std::scoped_lock (#204462)
The thread safety annotations on std::scoped_lock were previously only applied to the empty and single-mutex specializations. The general variadic specialization carried no annotations, so -Wthread-safety considered none of the mutexes held inside a multi-mutex scoped_lock block and reported the guarded data accessed there as unprotected. Now that Clang supports pack expansion inside thread safety attributes (landed for Clang 21), annotate the variadic scoped_lock with acquire_capability/requires_capability on its constructors, release_capability on its destructor, and scoped_lockable on the class. Fixes #42000. Co-authored-by: Anthony Calandra <anthony@anthony-calandra.com>
This commit is contained in:
parent
29f1a71a8d
commit
72e1037c32
8 changed files with 150 additions and 13 deletions
|
|
@ -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
|
||||
-------------------------
|
||||
|
|
|
|||
|
|
@ -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__)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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_; }
|
||||
|
|
|
|||
|
|
@ -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... _MArgs>
|
||||
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<sizeof...(_MArgs)>(), __t_); }
|
||||
_LIBCPP_RELEASE_CAPABILITY() _LIBCPP_HIDE_FROM_ABI ~scoped_lock() {
|
||||
__unlock_unpack(make_index_sequence<sizeof...(_MArgs)>(), __t_);
|
||||
}
|
||||
|
||||
scoped_lock(scoped_lock const&) = delete;
|
||||
scoped_lock& operator=(scoped_lock const&) = delete;
|
||||
|
||||
private:
|
||||
template <size_t... _Indx>
|
||||
_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(), ...);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
// <mutex>
|
||||
|
||||
// GCC doesn't have thread safety attributes
|
||||
// UNSUPPORTED: gcc
|
||||
|
||||
// ADDITIONAL_COMPILE_FLAGS: -Wthread-safety
|
||||
|
||||
#include <mutex>
|
||||
|
||||
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<std::mutex> lock(m1);
|
||||
data1++;
|
||||
}
|
||||
|
||||
static void two_mutexes() {
|
||||
std::scoped_lock<std::mutex, std::mutex> 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<std::mutex> lock(std::adopt_lock, m1);
|
||||
data1++;
|
||||
}
|
||||
|
||||
static void adopt_two() {
|
||||
m1.lock();
|
||||
m2.lock();
|
||||
std::scoped_lock<std::mutex, std::mutex> 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;
|
||||
}
|
||||
|
|
@ -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
|
||||
|
||||
// <mutex>
|
||||
|
||||
// GCC doesn't have thread safety attributes
|
||||
// UNSUPPORTED: gcc
|
||||
|
||||
// ADDITIONAL_COMPILE_FLAGS: -Wthread-safety
|
||||
|
||||
#include <mutex>
|
||||
|
||||
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<std::mutex> 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<std::mutex> 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<std::mutex, std::mutex> 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<std::mutex> lock(m1);
|
||||
data1++;
|
||||
}
|
||||
data1++; // expected-warning {{writing variable 'data1' requires holding mutex 'm1' exclusively}}
|
||||
}
|
||||
Loading…
Reference in a new issue