[clang] 4b1cd29 - Thread Safety Analysis: Don't treat function pointer parameters as scoped capabilities (#211885)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 28 08:53:18 PDT 2026
Author: Jameson Nash
Date: 2026-07-28T11:53:13-04:00
New Revision: 4b1cd29ffb39ea94590869657e9170230155a979
URL: https://github.com/llvm/llvm-project/commit/4b1cd29ffb39ea94590869657e9170230155a979
DIFF: https://github.com/llvm/llvm-project/commit/4b1cd29ffb39ea94590869657e9170230155a979.diff
LOG: Thread Safety Analysis: Don't treat function pointer parameters as scoped capabilities (#211885)
Capability attributes on a parameter mean one of two unrelated things:
on a scoped-lockable parameter they describe the locks the passed scope
object holds, while on a function pointer parameter they describe the
requirements of the function called through the pointer. Since #191187
allowed the latter, both of the scoped-lockable code paths have been
misreading function pointer parameters as scope objects.
At a call site, the argument bound to the parameter was translated into
a capability and required to be held, so passing a callback that
requires a capability was reported as a missing lock named after the
callback:
```
void apply(fn_t fn EXCLUSIVE_LOCKS_REQUIRED(mu));
static int my_fn() EXCLUSIVE_LOCKS_REQUIRED(mu);
...
apply(my_fn); // warning: requires holding mutex 'fn'
```
In the callee, the same confusion seeded the parameter's capabilities
into the function's entry lockset, so they were considered held
throughout the body and the calls made through the pointer went
unchecked -- the opposite of what the annotation asks for.
Skip function pointer parameters in both places; their attributes are
already checked at the indirect call sites (by #191187), the same way as
for annotated function pointer variables and fields.
Assisted-by: Claude Opus 5
Added:
Modified:
clang/docs/ThreadSafetyAnalysis.md
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/warn-thread-safety-analysis.c
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
clang/test/SemaCXX/warn-thread-safety-parsing.cpp
Removed:
################################################################################
diff --git a/clang/docs/ThreadSafetyAnalysis.md b/clang/docs/ThreadSafetyAnalysis.md
index 012c8a4c41373..f29b5bbc55e1e 100644
--- a/clang/docs/ThreadSafetyAnalysis.md
+++ b/clang/docs/ThreadSafetyAnalysis.md
@@ -510,9 +510,10 @@ Use of these attributes has been deprecated.
### Function Pointers
-Thread safety attributes may also be applied to function pointer variables and
-fields. The attributes describe the locking behavior of calling through that
-pointer, and the analysis will check calls through the pointer accordingly.
+Thread safety attributes may also be applied to variables, fields, and
+parameters of function pointer (or, in C++, function reference) type. The
+attributes describe the locking behavior of calling through that pointer, and
+the analysis will check calls through the pointer accordingly.
```c++
Mutex mu;
@@ -531,16 +532,22 @@ void test(Ops *ops) {
ops->read();
unlock_fn();
}
+
+void visit_all(void (*visit)(int) REQUIRES(mu), int n) {
+ lock_fn();
+ visit(n); // OK: 'mu' is held here
+ unlock_fn();
+}
```
-Note that the attributes are on the *variable* (or field), not on the function
-pointer type. Assigning a function with
diff erent (or no) attributes to an
-annotated function pointer variable is not diagnosed. The analysis trusts the
-annotations on the variable at the call site.
+Note that the attributes are on the *variable* (or field, or parameter), not on
+the function pointer type. Assigning a function with
diff erent (or no)
+attributes to an annotated function pointer variable is not diagnosed. The
+analysis trusts the annotations on the variable at the call site.
-This support is limited to plain function pointers. Pointers-to-member
-functions, blocks, and wrapper types such as `std::function` are not
-supported yet.
+This support is limited to plain function pointers and function references.
+Pointers-to-member functions, blocks, and wrapper types such as `std::function`
+are not supported yet.
### Warning flags
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index cce6f70a58893..d9d0d485f16ac 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4350,7 +4350,7 @@ def warn_thread_attribute_decl_not_pointer : Warning<
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
def warn_thread_attribute_not_on_scoped_lockable_param : Warning<
"%0 attribute applies to function parameters only if their type is a "
- "reference to a 'scoped_lockable'-annotated type">,
+ "function pointer or a reference to a 'scoped_lockable'-annotated type">,
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
def warn_thread_attribute_requires_preceded : Warning<
"%0 attribute on %1 must be preceded by %2 attribute">,
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index b3d7cb238bb46..5e656a3dbc1cb 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -62,6 +62,19 @@ using namespace threadSafety;
// Key method definition
ThreadSafetyHandler::~ThreadSafetyHandler() = default;
+/// True if capability attributes on \p Param describe the function reached
+/// through it rather than the argument bound to it.
+///
+/// Sema accepts capability attributes on a parameter for two unrelated
+/// purposes: a scoped-lockable parameter, where the attributes describe the
+/// locks the passed scope object holds, and a parameter naming a function to
+/// call -- a function pointer or a function reference -- where they describe
+/// the requirements of the function called through it.
+static bool isCallbackParam(const ParmVarDecl *Param) {
+ QualType T = Param->getType().getNonReferenceType();
+ return T->isFunctionPointerType() || T->isFunctionType();
+}
+
/// Issue a warning about an invalid lock expression
static void warnInvalidLock(ThreadSafetyHandler &Handler,
const Expr *MutexExp, const NamedDecl *D,
@@ -2332,6 +2345,8 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
const auto *CalledFunction = dyn_cast<FunctionDecl>(D);
if (CalledFunction && Args.has_value()) {
for (auto [Param, Arg] : zip(CalledFunction->parameters(), *Args)) {
+ if (isCallbackParam(Param))
+ continue;
CapExprSet DeclaredLocks;
for (const Attr *At : Param->attrs()) {
switch (At->getKind()) {
@@ -2904,6 +2919,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
else
llvm_unreachable("Unknown function kind");
for (const ParmVarDecl *Param : Params) {
+ if (isCallbackParam(Param))
+ continue;
CapExprSet UnderlyingLocks;
for (const auto *Attr : Param->attrs()) {
Loc = Attr->getLocation();
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 1b272b5416860..492b125587344 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -436,18 +436,20 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
}
}
-/// True if T (or its pointee, after stripping a top-level reference) is a
-/// function pointer or dependent.
-static bool isFunctionPointerOrDependent(QualType T) {
+/// True if T names a function to call: a function pointer, a function
+/// reference, or a reference to a function pointer. Dependent types are also
+/// accepted, and re-checked after instantiation.
+static bool isCallbackOrDependent(QualType T) {
T = T.getNonReferenceType();
- return T->isDependentType() || T->isFunctionPointerType();
+ return T->isDependentType() || T->isFunctionPointerType() ||
+ T->isFunctionType();
}
/// Checks that thread-safety attributes on variables or fields apply only to
-/// function pointer types.
+/// function pointer or function reference types.
static bool checkThreadSafetyValueDeclIsFunPtr(Sema &S, const ValueDecl *VD,
const AttributeCommonInfo &A) {
- if (isFunctionPointerOrDependent(VD->getType()))
+ if (isCallbackOrDependent(VD->getType()))
return true;
S.Diag(A.getLoc(), diag::warn_thread_attribute_not_on_fun_ptr)
<< A << (isa<FieldDecl>(VD) ? 1 : 0);
@@ -477,8 +479,8 @@ static bool checkThreadSafetyAttrSubject(Sema &S, Decl *D, const ParsedAttr &AL,
if (CheckParmVar) {
if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
- // A function-pointer parameter is also valid here.
- if (isFunctionPointerOrDependent(PVD->getType()))
+ // A function-pointer or function-reference parameter is also valid here.
+ if (isCallbackOrDependent(PVD->getType()))
return true;
return checkFunParamsAreScopedLockable(S, PVD, AL);
}
@@ -500,7 +502,7 @@ bool Sema::checkInstantiatedThreadSafetyAttrs(const Decl *D, const Attr *A) {
// Parameters of template functions need to be re-checked during
// instantiation because their types might have been dependent.
if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
- if (isFunctionPointerOrDependent(PVD->getType()))
+ if (isCallbackOrDependent(PVD->getType()))
return true;
return checkFunParamsAreScopedLockable(*this, PVD, *A);
}
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index d9ebc2a91dadc..c61152d59a7dd 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -319,6 +319,42 @@ void test_fp_ops_fail(struct FPOps *ops) {
ops->requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex '&FPOps::mu' exclusively}}
}
+// Function pointer parameters. The attributes constrain the function reached
+// through the pointer, so they are checked where the pointer is called, and
+// must not be mistaken for requirements of the enclosing function's callers
+// (nor for scoped-lockable parameter annotations). SemaCXX's
+// warn-thread-safety-analysis.cpp covers the C++ spellings of this.
+typedef void (*visit_fn)(int);
+
+void visit_cb(int x) EXCLUSIVE_LOCKS_REQUIRED(mu1);
+void visit_all(visit_fn visit EXCLUSIVE_LOCKS_REQUIRED(mu1), int n);
+void visit_all_locked_fp(visit_fn visit EXCLUSIVE_LOCKS_REQUIRED(mu1), int n)
+ EXCLUSIVE_LOCKS_REQUIRED(mu1) {
+ visit(n);
+}
+void visit_all_unlocked_fp(visit_fn visit EXCLUSIVE_LOCKS_REQUIRED(mu1), int n) {
+ visit(n); // expected-warning {{calling function 'visit' requires holding mutex 'mu1' exclusively}}
+}
+
+// Passing an annotated callee is not itself a use of the capability, so these
+// calls do not require mu1 to be held here.
+void test_fp_param(int n) {
+ visit_all(visit_cb, n);
+ visit_all(&visit_cb, n);
+ visit_all_unlocked_fp(visit_cb, n);
+ // Only visit_all_locked_fp's own attribute requires mu1.
+ visit_all_locked_fp(visit_cb, n); // expected-warning {{calling function 'visit_all_locked_fp' requires holding mutex 'mu1' exclusively}}
+}
+
+// Acquire/release on a function pointer parameter likewise describe the pointee,
+// so calling the enclosing function neither acquires nor releases mu1.
+void call_locker(void (*lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1));
+void test_fp_param_acquire(void) {
+ call_locker(0);
+ mutex_exclusive_lock(&mu1);
+ mutex_exclusive_unlock(&mu1);
+}
+
// Function pointer attributes referring to parameters.
struct BDev {
struct Mutex lock;
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 5fc4bb4d7cd0a..54b2c70940c24 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -8230,4 +8230,89 @@ void test_attr_refers_to_param(Mutex *m) {
m->Unlock();
}
+// Function references name a function to call just like function pointers do.
+void lock_impl(void) EXCLUSIVE_LOCK_FUNCTION(mu);
+void unlock_impl(void) UNLOCK_FUNCTION(mu);
+void requires_impl(void) EXCLUSIVE_LOCKS_REQUIRED(mu);
+
+void (&lock_ref)(void) EXCLUSIVE_LOCK_FUNCTION(mu) = lock_impl;
+void (&unlock_ref)(void) UNLOCK_FUNCTION(mu) = unlock_impl;
+void (&requires_ref)(void) EXCLUSIVE_LOCKS_REQUIRED(mu) = requires_impl;
+
+void testReferenceAcquireRelease() {
+ lock_ref();
+ x = 1;
+ requires_ref();
+ unlock_ref();
+}
+
+void testReferenceRequiresFail() {
+ requires_ref(); // expected-warning {{calling function 'requires_ref' requires holding mutex 'mu' exclusively}}
+}
+
+// Capability attributes on a parameter that names a function to call -- a
+// function pointer, a function reference, or a reference to either -- describe
+// the function reached through the parameter, not a capability that the bound
+// argument stands for. They are checked where the parameter is called, and are
+// neither requirements on nor effects for callers of the enclosing function.
+
+void callback(int) EXCLUSIVE_LOCKS_REQUIRED(mu);
+
+void takes_ptr(void (*cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n);
+void takes_ref(void (&cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n);
+void takes_ptr_ref(void (*&cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n);
+
+// Passing an annotated callee is not itself a use of the capability.
+void testPassCallback(void (*&pcb)(int), int n) {
+ takes_ptr(callback, n);
+ takes_ptr(&callback, n);
+ takes_ref(callback, n);
+ takes_ptr_ref(pcb, n);
+}
+
+// The attributes are checked at the indirect call instead ...
+void testCallPtr(void (*cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n) {
+ cb(n); // expected-warning {{calling function 'cb' requires holding mutex 'mu' exclusively}}
+}
+
+void testCallRef(void (&cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n) {
+ cb(n); // expected-warning {{calling function 'cb' requires holding mutex 'mu' exclusively}}
+}
+
+void testCallPtrRef(void (*&cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n) {
+ cb(n); // expected-warning {{calling function 'cb' requires holding mutex 'mu' exclusively}}
+}
+
+// ... where the enclosing function's own requirements can satisfy them.
+void testCallRefLocked(void (&cb)(int) EXCLUSIVE_LOCKS_REQUIRED(mu), int n)
+ EXCLUSIVE_LOCKS_REQUIRED(mu) {
+ cb(n);
+}
+
+// Acquire and release likewise describe the function called through the
+// parameter, so calling the enclosing function neither acquires nor releases.
+void takes_locker(void (&lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu));
+
+void testAcquireNotTransferred() {
+ takes_locker(lock_impl);
+ x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
+}
+
+void testCallAcquires(void (&lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu)) {
+ lock();
+ x = 1;
+ mu.Unlock();
+}
+
+// A dependent parameter type is classified after instantiation.
+template <typename F>
+void callDependent(F cb EXCLUSIVE_LOCKS_REQUIRED(mu), int n) {
+ cb(n); // expected-warning 2 {{calling function 'cb' requires holding mutex 'mu' exclusively}}
+}
+
+void testDependent(int n) {
+ callDependent<void (*)(int)>(callback, n); // expected-note {{in instantiation of function template specialization 'FunctionPointers::callDependent<void (*)(int)>' requested here}}
+ callDependent<void (&)(int)>(callback, n); // expected-note {{in instantiation of function template specialization 'FunctionPointers::callDependent<void (&)(int)>' requested here}}
+}
+
} // namespace FunctionPointers
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 368c20cb45209..8ceacb5a83418 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -617,7 +617,7 @@ class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
void elf_fun_params1(MutexLock& scope EXCLUSIVE_LOCK_FUNCTION(mu1));
void elf_fun_params2(int lvar EXCLUSIVE_LOCK_FUNCTION(mu1)); // \
- // expected-warning{{'exclusive_lock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning{{'exclusive_lock_function' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
void elf_fun_params3(MutexLock& scope EXCLUSIVE_LOCK_FUNCTION()); // \
// expected-warning{{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
@@ -699,7 +699,7 @@ int slf_test_var SHARED_LOCK_FUNCTION(); // \
void slf_fun_params1(MutexLock& scope SHARED_LOCK_FUNCTION(mu1));
void slf_fun_params2(int lvar SHARED_LOCK_FUNCTION(mu1)); // \
- // expected-warning {{'shared_lock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning {{'shared_lock_function' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
void slf_fun_params3(MutexLock& scope SHARED_LOCK_FUNCTION()); // \
// expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
@@ -999,7 +999,7 @@ class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \
void uf_fun_params1(MutexLock& scope UNLOCK_FUNCTION(mu1));
void uf_fun_params2(int lvar UNLOCK_FUNCTION(mu1)); // \
- // expected-warning {{'unlock_function' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning {{'unlock_function' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
void uf_fun_params3(MutexLock& scope UNLOCK_FUNCTION()); // \
// expected-warning {{'unlock_function' attribute without capability arguments can only be applied to non-static methods of a class}}
@@ -1152,11 +1152,11 @@ int le_test_var LOCKS_EXCLUDED(mu1); // \
void le_fun_params1(MutexLock& scope LOCKS_EXCLUDED(mu1));
void le_fun_params2(int lvar LOCKS_EXCLUDED(mu1)); // \
- // expected-warning{{'locks_excluded' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning{{'locks_excluded' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
template <typename T>
void le_fun_params3(T& lvar LOCKS_EXCLUDED(mu1)) {} // \
- // expected-warning{{'locks_excluded' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning{{'locks_excluded' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
void call_le_fun_params3(int i) {
MutexLock scope(&mu1);
le_fun_params3(i); // expected-note {{while substituting deduced template arguments into function template 'le_fun_params3' [with T = int]}}
@@ -1246,7 +1246,7 @@ int elr_test_var EXCLUSIVE_LOCKS_REQUIRED(mu1); // \
void elr_fun_params1(MutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu1));
void elr_fun_params2(int lvar EXCLUSIVE_LOCKS_REQUIRED(mu1)); // \
- // expected-warning {{'exclusive_locks_required' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning {{'exclusive_locks_required' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
class ElrFoo {
private:
@@ -1333,7 +1333,7 @@ int slr_test_var SHARED_LOCKS_REQUIRED(mu1); // \
void slr_fun_params1(MutexLock& scope SHARED_LOCKS_REQUIRED(mu1));
void slr_fun_params2(int lvar SHARED_LOCKS_REQUIRED(mu1)); // \
- // expected-warning {{'shared_locks_required' attribute applies to function parameters only if their type is a reference to a 'scoped_lockable'-annotated type}}
+ // expected-warning {{'shared_locks_required' attribute applies to function parameters only if their type is a function pointer or a reference to a 'scoped_lockable'-annotated type}}
class SlrFoo {
private:
@@ -1784,6 +1784,17 @@ void fp_param_assert(void (*pf)(void) ASSERT_EXCLUSIVE_LOCK(mu1));
void fp_param_try(bool (*pf)(void) EXCLUSIVE_TRYLOCK_FUNCTION(true, mu1));
void fp_ref(void (*&rf)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1));
+// Function references name a function to call, just like function pointers.
+void fn_impl(void);
+void (&fn_ref_lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1) = fn_impl;
+void (&fn_ref_requires)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1) = fn_impl;
+struct FnRefFields {
+ void (&lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1);
+ void (&requires_mu)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1);
+};
+void fn_ref_param(void (&rf)(void) EXCLUSIVE_LOCK_FUNCTION(mu1));
+void fn_ref_param_requires(void (&rf)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1));
+
int bad_fp_var EXCLUSIVE_LOCK_FUNCTION(mu1); // \
// expected-warning {{'exclusive_lock_function' attribute on a variable requires the variable to be of function pointer type}}
struct BadFPFields {
More information about the cfe-commits
mailing list