https://github.com/vtjnash updated https://github.com/llvm/llvm-project/pull/226561
>From f8ee42c2d3d3fc814def2a28c73839b5341c6490 Mon Sep 17 00:00:00 2001 From: Jameson Nash <[email protected]> Date: Sun, 11 Oct 2026 19:47:13 -0400 Subject: [PATCH 1/2] [clang][ThreadSafety] Support lock_returned on function pointers d1e84bb9ad3b allowed the thread safety function attributes on function pointers, but not lock_returned, so a call through a function pointer could not return a known capability: struct Ops { struct Mutex *(*lock_of)(struct Dev *d) LOCK_RETURNED(&d->lock); }; Handle lock_returned like the other attributes: - Allow it on variables and fields of function pointer or function reference type, rechecked after template instantiation. - Parse its argument in the prototype's scope, so it can name the pointee's parameters. - Look it up on the callee declaration rather than only a direct callee, substituting the call's arguments for the pointee's parameters. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- clang/docs/ReleaseNotes.md | 10 +++++++ clang/docs/ThreadSafetyAnalysis.md | 5 +++- clang/include/clang/Basic/Attr.td | 3 +- clang/lib/Analysis/ThreadSafetyCommon.cpp | 12 ++++---- clang/lib/Sema/SemaDeclAttr.cpp | 9 ++++-- clang/test/Sema/warn-thread-safety-analysis.c | 27 ++++++++++++++++++ .../SemaCXX/warn-thread-safety-analysis.cpp | 28 +++++++++++++++++++ .../SemaCXX/warn-thread-safety-parsing.cpp | 21 +++++++++++--- 8 files changed, 102 insertions(+), 13 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 77a1e73bc8af6..8f7d0f711111c 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -345,6 +345,16 @@ features cannot lower the translation-unit ABI level; and view constructors and factory functions to opt in to `-Wunsafe-buffer-usage-in-container` diagnostics. +- The thread safety attribute `lock_returned` (`RETURN_CAPABILITY`) can now be + applied to function pointers, like the other thread safety function + attributes: + + ```c + struct Ops { + struct Mutex *(*lock_of)(struct Dev *d) RETURN_CAPABILITY(&d->lock); + }; + ``` + ### Improvements to Clang's diagnostics - Fixed spurious `-Wimplicit-void-ptr-cast` warnings in C for parenthesized diff --git a/clang/docs/ThreadSafetyAnalysis.md b/clang/docs/ThreadSafetyAnalysis.md index f29b5bbc55e1e..3dca46a1d8c8d 100644 --- a/clang/docs/ThreadSafetyAnalysis.md +++ b/clang/docs/ThreadSafetyAnalysis.md @@ -382,7 +382,8 @@ interface of a function, and should thus be placed on the function definition `RETURN_CAPABILITY` is an attribute on functions or methods, which declares that the function returns a reference to the given capability. It is used to -annotate getter methods that return mutexes. +annotate getter methods that return mutexes. It can also be applied to a +function pointer (see {ref}`function-pointers`). ```c++ class MyClass { @@ -508,6 +509,8 @@ annotated function. See {ref}`mutexheader`, below, for example uses. Use of these attributes has been deprecated. +(function-pointers)= + ### Function Pointers Thread safety attributes may also be applied to variables, fields, and diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b41c1c78ddd99..c9e9e2ae82c4f 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4364,7 +4364,8 @@ def LockReturned : InheritableAttr { let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; - let Subjects = SubjectList<[Function]>; + let ParseArgsInFunctionScope = 1; + let Subjects = SubjectList<[Function, Var, Field]>; let Documentation = [Undocumented]; } diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp index d9414d42a9524..bea41f2c57113 100644 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp @@ -542,12 +542,14 @@ til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE, CallingContext *Ctx, const Expr *SelfE) { if (CapabilityExprMode) { - // Handle LOCK_RETURNED - if (const FunctionDecl *FD = CE->getDirectCallee()) { - FD = FD->getMostRecentDecl(); - if (LockReturnedAttr *At = FD->getAttr<LockReturnedAttr>()) { + // Handle LOCK_RETURNED, on a function or a function pointer. + if (const auto *D = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) { + const auto *AttrD = D; + if (const auto *FD = dyn_cast<FunctionDecl>(D)) + AttrD = FD->getMostRecentDecl(); + if (const auto *At = AttrD->getAttr<LockReturnedAttr>()) { CallingContext LRCallCtx(Ctx); - LRCallCtx.AttrDecl = CE->getDirectCallee(); + LRCallCtx.AttrDecl = D; LRCallCtx.SelfArg = SelfE; LRCallCtx.NumArgs = CE->getNumArgs(); LRCallCtx.FunArgs = CE->getArgs(); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 0361a362402bc..50b2a23f3c818 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -493,7 +493,7 @@ static bool checkThreadSafetyAttrSubject(Sema &S, Decl *D, const ParsedAttr &AL, bool Sema::checkInstantiatedThreadSafetyAttrs(const Decl *D, const Attr *A) { if (!isa<AssertCapabilityAttr, AcquireCapabilityAttr, TryAcquireCapabilityAttr, ReleaseCapabilityAttr, - RequiresCapabilityAttr, LocksExcludedAttr>(A)) + RequiresCapabilityAttr, LocksExcludedAttr, LockReturnedAttr>(A)) return true; const auto *VD = dyn_cast<ValueDecl>(D); @@ -502,7 +502,9 @@ 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)) { + // lock_returned does not apply to scoped_lockable references. + if (const auto *PVD = dyn_cast<ParmVarDecl>(VD); + PVD && !isa<LockReturnedAttr>(A)) { if (isCallbackOrDependent(PVD->getType())) return true; return checkFunParamsAreScopedLockable(*this, PVD, *A); @@ -684,6 +686,9 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, } static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (!checkThreadSafetyAttrSubject(S, D, AL)) + return; + // check that the argument is lockable object SmallVector<Expr*, 1> Args; checkAttrArgsAreCapabilityObjs(S, D, AL, Args); diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index c61152d59a7dd..b195898be42e0 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -376,6 +376,33 @@ void test_bdev_ops_fail(struct BDevOps *ops, struct BDev *bdev) { ops->unlock(bdev); // expected-warning {{releasing mutex 'bdev->lock' that was not held}} } +// lock_returned on a function pointer, naming the pointee's parameter. +struct Mutex *(*bdev_lock_of)(struct BDev *bdev) LOCK_RETURNED(&bdev->lock); +struct BDevLockOps { + struct Mutex *(*lock_of)(struct BDev *bdev) LOCK_RETURNED(&bdev->lock); +}; + +void test_bdev_lock_of(struct BDevLockOps *ops, struct BDev *bdev) { + mutex_exclusive_lock(bdev_lock_of(bdev)); + bdev->a = 42; + mutex_exclusive_unlock(ops->lock_of(bdev)); + mutex_exclusive_lock(ops->lock_of(bdev)); + mutex_exclusive_unlock(&bdev->lock); +} + +void test_bdev_lock_of_fail(struct BDevLockOps *ops, struct BDev *bdev) { + mutex_exclusive_lock(&mu1); + mutex_exclusive_unlock(ops->lock_of(bdev)); // expected-warning {{releasing mutex 'bdev->lock' that was not held}} + mutex_exclusive_unlock(&mu1); +} + +void test_lock_of_param(struct BDev *bdev, + struct Mutex *(*lock_of)(struct BDev *) LOCK_RETURNED(&bdev->lock)) { + mutex_exclusive_lock(lock_of(bdev)); + bdev->a = 42; + mutex_exclusive_unlock(&bdev->lock); +} + // Test unusual trylock patterns void do_some_work(void); int work_data GUARDED_BY(mu1); diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index a58b0d24e4239..868c008390f20 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -8370,4 +8370,32 @@ void testDependent(int n) { callDependent<void (&)(int)>(callback, n); // expected-note {{in instantiation of function template specialization 'FunctionPointers::callDependent<void (&)(int)>' requested here}} } +// lock_returned on a function pointer or reference. +struct Guarded { + Mutex mu; + int data GUARDED_BY(mu); +}; +Mutex &mu_ref_impl(Guarded &g) LOCK_RETURNED(g.mu); +Mutex *(*mu_of)(Guarded *g) LOCK_RETURNED(&g->mu); +Mutex &(&mu_ref_of)(Guarded &g) LOCK_RETURNED(g.mu) = mu_ref_impl; + +void testLockReturned(Guarded *g) { + mu_of(g)->Lock(); + g->data = 1; + mu_ref_of(*g).Unlock(); // expected-note {{mutex released here}} + mu_of(g)->Unlock(); // expected-warning {{releasing mutex 'g->mu' that was not held}} +} + +// A dependent parameter type is classified after instantiation. +template <typename F> +void lockReturnedDependent(Guarded *g, F mu_of_g LOCK_RETURNED(&g->mu)) { + mu_of_g(g)->Lock(); + g->data = 1; + g->mu.Unlock(); +} + +void testLockReturnedDependent(Guarded *g) { + lockReturnedDependent<Mutex *(*)(Guarded *)>(g, mu_of); +} + } // namespace FunctionPointers diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 8ceacb5a83418..70b3edf97e12f 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1076,20 +1076,20 @@ int lr_testfn(int y) LOCK_RETURNED(mu1); int lr_testfn(int y) { int x LOCK_RETURNED(mu1) = y; // \ - // expected-warning {{'lock_returned' attribute only applies to functions}} + // expected-warning {{'lock_returned' attribute on a variable requires the variable to be of function pointer type}} return x; }; int lr_test_var LOCK_RETURNED(mu1); // \ - // expected-warning {{'lock_returned' attribute only applies to functions}} + // expected-warning {{'lock_returned' attribute on a variable requires the variable to be of function pointer type}} void lr_fun_params(int lvar LOCK_RETURNED(mu1)); // \ - // expected-warning {{'lock_returned' attribute only applies to functions}} + // expected-warning {{'lock_returned' attribute on a variable requires the variable to be of function pointer type}} class LrFoo { private: int test_field LOCK_RETURNED(mu1); // \ - // expected-warning {{'lock_returned' attribute only applies to functions}} + // expected-warning {{'lock_returned' attribute on a field requires the field to be of function pointer type}} void test_method() LOCK_RETURNED(mu1); }; @@ -1768,6 +1768,7 @@ void (*fp_requires)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1); void (*fp_excludes)(void) LOCKS_EXCLUDED(mu1); bool (*fp_trylock)(void) EXCLUSIVE_TRYLOCK_FUNCTION(true, mu1); void (*fp_assert)(void) ASSERT_EXCLUSIVE_LOCK(mu1); +Mutex *(*fp_returned)(void) LOCK_RETURNED(&mu1); void (*fp_shared_lock)(void) SHARED_LOCK_FUNCTION(mu1); void (*fp_shared_require)(void) SHARED_LOCKS_REQUIRED(mu1); void (*fp_shared_trylock)(void) SHARED_TRYLOCK_FUNCTION(true, mu1); @@ -1776,10 +1777,12 @@ struct FPFields { void (*lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1); void (*unlock)(void) UNLOCK_FUNCTION(mu1); void (*requires_mu)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1); + Mutex *(*returned)(void) LOCK_RETURNED(&mu1); }; // Function pointer parameters and references-to-function-pointer. void fp_param(void (*pf)(void) EXCLUSIVE_LOCK_FUNCTION(mu1)); +void fp_param_returned(Mutex *(*pf)(void) LOCK_RETURNED(&mu1)); 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)); @@ -1794,6 +1797,14 @@ struct FnRefFields { }; void fn_ref_param(void (&rf)(void) EXCLUSIVE_LOCK_FUNCTION(mu1)); void fn_ref_param_requires(void (&rf)(void) EXCLUSIVE_LOCKS_REQUIRED(mu1)); +Mutex &mu1_impl(void); +Mutex &(&fn_ref_returned)(void) LOCK_RETURNED(mu1) = mu1_impl; + +// lock_returned may name the pointee's parameters. +struct MutexHolder { + Mutex mu; +}; +Mutex *(*fp_returned_param)(MutexHolder *h) LOCK_RETURNED(&h->mu); 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}} @@ -1821,6 +1832,8 @@ struct DependentFPFields { // expected-warning {{'exclusive_lock_function' attribute on a field requires the field to be of function pointer type}} FuncPtr requires_mu EXCLUSIVE_LOCKS_REQUIRED(mu1); // \ // expected-warning {{'exclusive_locks_required' attribute on a field requires the field to be of function pointer type}} + FuncPtr returned LOCK_RETURNED(&mu1); // \ + // expected-warning {{'lock_returned' attribute on a field requires the field to be of function pointer type}} }; typedef void (*GoodLockFn)(void); >From d61cc8d2985024db401c2a83943746f3c70303ae Mon Sep 17 00:00:00 2001 From: Jameson Nash <[email protected]> Date: Sun, 11 Oct 2026 20:55:20 -0400 Subject: [PATCH 2/2] fixup! [clang][ThreadSafety] Support lock_returned on function pointers --- clang/docs/ThreadSafetyAnalysis.md | 5 +---- clang/lib/Analysis/ThreadSafetyCommon.cpp | 2 +- clang/test/SemaCXX/warn-thread-safety-analysis.cpp | 2 ++ 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/docs/ThreadSafetyAnalysis.md b/clang/docs/ThreadSafetyAnalysis.md index 3dca46a1d8c8d..f29b5bbc55e1e 100644 --- a/clang/docs/ThreadSafetyAnalysis.md +++ b/clang/docs/ThreadSafetyAnalysis.md @@ -382,8 +382,7 @@ interface of a function, and should thus be placed on the function definition `RETURN_CAPABILITY` is an attribute on functions or methods, which declares that the function returns a reference to the given capability. It is used to -annotate getter methods that return mutexes. It can also be applied to a -function pointer (see {ref}`function-pointers`). +annotate getter methods that return mutexes. ```c++ class MyClass { @@ -509,8 +508,6 @@ annotated function. See {ref}`mutexheader`, below, for example uses. Use of these attributes has been deprecated. -(function-pointers)= - ### Function Pointers Thread safety attributes may also be applied to variables, fields, and diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp index bea41f2c57113..e903070bc9b02 100644 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp @@ -544,7 +544,7 @@ til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE, if (CapabilityExprMode) { // Handle LOCK_RETURNED, on a function or a function pointer. if (const auto *D = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) { - const auto *AttrD = D; + const NamedDecl *AttrD = D; if (const auto *FD = dyn_cast<FunctionDecl>(D)) AttrD = FD->getMostRecentDecl(); if (const auto *At = AttrD->getAttr<LockReturnedAttr>()) { diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 868c008390f20..0166e2d77d498 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -8377,6 +8377,8 @@ struct Guarded { }; Mutex &mu_ref_impl(Guarded &g) LOCK_RETURNED(g.mu); Mutex *(*mu_of)(Guarded *g) LOCK_RETURNED(&g->mu); +// As with the other attributes (see testReassignIncompatible), the initializer +// is not checked against the attribute: the analysis trusts the declaration. Mutex &(&mu_ref_of)(Guarded &g) LOCK_RETURNED(g.mu) = mu_ref_impl; void testLockReturned(Guarded *g) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
