https://github.com/melver updated https://github.com/llvm/llvm-project/pull/191187
>From 52b30be8ec8512cc6beab0c45386859a57bb179c Mon Sep 17 00:00:00 2001 From: Marco Elver <[email protected]> Date: Thu, 9 Apr 2026 14:55:29 +0200 Subject: [PATCH] Thread Safety Analysis: Support attributes on function pointers Allow acquire_capability, release_capability, requires_capability, try_acquire_capability, assert_capability, and locks_excluded attributes (incl. their shared variants) on function pointer variables and struct fields. Calls through annotated function pointers are checked the same way as direct function calls. The attributes are placed on variable/field declarations, not on the function pointer type itself. This is a deliberate trade-off: making these "attributes" part of the type system would require diagnosing mismatched assignments, which would be a significant type-system extension with limited practical benefit, which would likely require promoting the TSA vocabulary to full type-qualifiers. Instead, the analysis trusts the annotations on the variable at the call site, and sticks with the attribute-based semantics. This matches the existing philosophy where the analysis tries to avoid false positives where possible and attribute mismatches on direct functions are likewise not hard errors or warnings (yet). The primary motivation is to avoid false positives in large C codebases, such as the Linux kernel [1], which tend to use structs containing function pointers to emulate subtype polymorphism and dynamic dispatch. [1] https://lore.kernel.org/all/[email protected]/ --- clang/docs/ReleaseNotes.rst | 6 + clang/docs/ThreadSafetyAnalysis.rst | 32 +++++ clang/include/clang/Basic/Attr.td | 12 +- .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/include/clang/Sema/Sema.h | 2 + clang/lib/Analysis/ThreadSafetyCommon.cpp | 11 +- clang/lib/Sema/SemaDeclAttr.cpp | 60 ++++++-- clang/lib/Sema/SemaTemplateInstantiate.cpp | 2 +- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 6 +- clang/test/Sema/attr-capabilities.c | 6 +- clang/test/Sema/warn-thread-safety-analysis.c | 21 +++ .../SemaCXX/warn-thread-safety-analysis.cpp | 129 ++++++++++++++++++ .../SemaCXX/warn-thread-safety-parsing.cpp | 109 ++++++++++----- 13 files changed, 343 insertions(+), 56 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2da7175b51ea3..4fd97e99cd132 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -252,6 +252,12 @@ Attribute Changes in Clang sound because any writer must hold all capabilities, so holding any one prevents concurrent writes. +- :doc:`ThreadSafetyAnalysis` attributes like ``acquire_capability``, + ``release_capability``, ``requires_capability``, ``locks_excluded``, + ``try_acquire_capability``, and ``assert_capability`` can now be applied to + function pointer variables and fields. The analysis checks calls through + annotated function pointers the same way it checks direct function calls. + - Added support for ``[[msvc::forceinline]]`` for functions and ``[[msvc::forceinline_calls]]`` for statements. Both are aliases to ``[[clang::always_inline]]`` with additional checks to ensure that they diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index 571c6c0e04ea1..a5908050b2281 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -544,6 +544,38 @@ GUARDED_VAR and PT_GUARDED_VAR 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. + +.. code-block:: c++ + + Mutex mu; + int x GUARDED_BY(mu); + + void (*lock_fn)(void) ACQUIRE(mu); + void (*unlock_fn)(void) RELEASE(mu); + + struct Ops { + void (*read)(void) REQUIRES(mu); + }; + + void test(Ops *ops) { + lock_fn(); + x = 1; + ops->read(); + unlock_fn(); + } + +Note that the attributes are on the *variable* (or field), not on the function +pointer type. Assigning a function with different (or no) attributes to an +annotated function pointer variable is not diagnosed. The analysis trusts the +annotations on the variable at the call site. + + Warning flags ------------- diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 328e70b3ed900..6ab51243bb297 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4079,7 +4079,7 @@ def AssertCapability : InheritableAttr { Clang<"assert_shared_capability", 0>, GNU<"assert_exclusive_lock">, GNU<"assert_shared_lock">]; - let Subjects = SubjectList<[Function]>; + let Subjects = SubjectList<[Function, NonParmVar, Field]>; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -4097,7 +4097,7 @@ def AcquireCapability : InheritableAttr { Clang<"acquire_shared_capability", 0>, GNU<"exclusive_lock_function">, GNU<"shared_lock_function">]; - let Subjects = SubjectList<[Function, ParmVar]>; + let Subjects = SubjectList<[Function, ParmVar, NonParmVar, Field]>; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -4115,7 +4115,7 @@ def TryAcquireCapability : InheritableAttr { Clang<"try_acquire_shared_capability", 0>, GNU<"exclusive_trylock_function">, GNU<"shared_trylock_function">]; - let Subjects = SubjectList<[Function]>; + let Subjects = SubjectList<[Function, NonParmVar, Field]>; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -4133,7 +4133,7 @@ def ReleaseCapability : InheritableAttr { Clang<"release_shared_capability", 0>, Clang<"release_generic_capability", 0>, Clang<"unlock_function", 0>]; - let Subjects = SubjectList<[Function, ParmVar]>; + let Subjects = SubjectList<[Function, ParmVar, NonParmVar, Field]>; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -4159,7 +4159,7 @@ def RequiresCapability : InheritableAttr { let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; - let Subjects = SubjectList<[Function, ParmVar]>; + let Subjects = SubjectList<[Function, ParmVar, NonParmVar, Field]>; let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]>]; let Documentation = [Undocumented]; @@ -4236,7 +4236,7 @@ def LocksExcluded : InheritableAttr { let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; - let Subjects = SubjectList<[Function, ParmVar]>; + let Subjects = SubjectList<[Function, ParmVar, NonParmVar, Field]>; let Documentation = [Undocumented]; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6d2fae551566f..a798cd0d2fbe8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4300,6 +4300,9 @@ def warn_thread_attribute_not_on_scoped_lockable_param : Warning< def warn_thread_attribute_requires_preceded : Warning< "%0 attribute on %1 must be preceded by %2 attribute">, InGroup<ThreadSafetyAttributes>, DefaultIgnore; +def warn_thread_attribute_not_on_fun_ptr : Warning< + "%0 attribute on a %select{variable|field}1 requires the %select{variable|field}1 to be of function pointer type">, + InGroup<ThreadSafetyAttributes>, DefaultIgnore; def err_attribute_argument_out_of_bounds_extra_info : Error< "%0 attribute parameter %1 is out of bounds: " "%plural{0:no parameters to index into|" diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 1b8a9803be472..fbd93fd194054 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -14237,6 +14237,8 @@ class Sema final : public SemaBase { }; typedef SmallVector<LateInstantiatedAttribute, 1> LateInstantiatedAttrVec; + bool checkDependentThreadSafetyAttrs(Decl *D, const Attr *A); + void InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, const Decl *Pattern, Decl *Inst, LateInstantiatedAttrVec *LateAttrs = nullptr, diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp index b43a986521f99..fd125ee56f136 100644 --- a/clang/lib/Analysis/ThreadSafetyCommon.cpp +++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp @@ -187,10 +187,15 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, } // If the attribute has no arguments, then assume the argument is "this". - if (!AttrExp) + // SelfArg may be null for non-method callees (e.g. function pointers). + if (!AttrExp) { + if (!Ctx.SelfArg) + return CapabilityExpr(); return translateAttrExpr(cast<const Expr *>(Ctx.SelfArg), nullptr); - else // For most attributes. - return translateAttrExpr(AttrExp, &Ctx); + } + + // For most attributes. + return translateAttrExpr(AttrExp, &Ctx); } /// Translate a clang expression in an attribute to a til::SExpr. diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 8a856215a9627..2b1ddcb2e548a 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -436,6 +436,18 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, } } +/// Checks that thread-safety attributes on variables or fields apply only to +/// function pointer types. +static bool checkThreadSafetyValueDeclIsFunPtr(Sema &S, const ValueDecl *VD, + const AttributeCommonInfo &A) { + if (VD->getType()->isDependentType() || + VD->getType()->isFunctionPointerType()) + return true; + S.Diag(A.getLoc(), diag::warn_thread_attribute_not_on_fun_ptr) + << A << (isa<FieldDecl>(VD) ? 1 : 0); + return false; +} + static bool checkFunParamsAreScopedLockable(Sema &S, const ParmVarDecl *ParamDecl, const ParsedAttr &AL) { @@ -449,6 +461,35 @@ static bool checkFunParamsAreScopedLockable(Sema &S, return false; } +static bool checkThreadSafetyAttrSubject(Sema &S, Decl *D, const ParsedAttr &AL, + bool CheckParmVar = false) { + const auto *VD = dyn_cast<ValueDecl>(D); + if (!VD || isa<FunctionDecl>(VD)) + return true; + + if (CheckParmVar) { + if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(VD)) + return checkFunParamsAreScopedLockable(S, ParmDecl, AL); + } + + return checkThreadSafetyValueDeclIsFunPtr(S, VD, AL); +} + +/// Recheck instantiated thread-safety attributes that could not be validated +/// on the dependent pattern declaration. +bool Sema::checkDependentThreadSafetyAttrs(Decl *D, const Attr *A) { + if (!isa<AssertCapabilityAttr, AcquireCapabilityAttr, + TryAcquireCapabilityAttr, ReleaseCapabilityAttr, + RequiresCapabilityAttr, LocksExcludedAttr>(A)) + return true; + + const auto *VD = dyn_cast<ValueDecl>(D); + if (!VD || isa<FunctionDecl, ParmVarDecl>(VD)) + return true; + + return checkThreadSafetyValueDeclIsFunPtr(*this, VD, *A); +} + //===----------------------------------------------------------------------===// // Attribute Implementations //===----------------------------------------------------------------------===// @@ -630,8 +671,7 @@ static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D); - ParmDecl && !checkFunParamsAreScopedLockable(S, ParmDecl, AL)) + if (!checkThreadSafetyAttrSubject(S, D, AL, true)) return; if (!AL.checkAtLeastNumArgs(S, 1)) @@ -6634,6 +6674,9 @@ static void handleReentrantCapabilityAttr(Sema &S, Decl *D, } static void handleAssertCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (!checkThreadSafetyAttrSubject(S, D, AL)) + return; + SmallVector<Expr*, 1> Args; if (!checkLockFunAttrCommon(S, D, AL, Args)) return; @@ -6644,8 +6687,7 @@ static void handleAssertCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { static void handleAcquireCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D); - ParmDecl && !checkFunParamsAreScopedLockable(S, ParmDecl, AL)) + if (!checkThreadSafetyAttrSubject(S, D, AL, true)) return; SmallVector<Expr*, 1> Args; @@ -6658,6 +6700,9 @@ static void handleAcquireCapabilityAttr(Sema &S, Decl *D, static void handleTryAcquireCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (!checkThreadSafetyAttrSubject(S, D, AL)) + return; + SmallVector<Expr*, 2> Args; if (!checkTryLockFunAttrCommon(S, D, AL, Args)) return; @@ -6668,9 +6713,9 @@ static void handleTryAcquireCapabilityAttr(Sema &S, Decl *D, static void handleReleaseCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D); - ParmDecl && !checkFunParamsAreScopedLockable(S, ParmDecl, AL)) + if (!checkThreadSafetyAttrSubject(S, D, AL, true)) return; + // Check that all arguments are lockable objects. SmallVector<Expr *, 1> Args; checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, true); @@ -6681,8 +6726,7 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D, static void handleRequiresCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (const auto *ParmDecl = dyn_cast<ParmVarDecl>(D); - ParmDecl && !checkFunParamsAreScopedLockable(S, ParmDecl, AL)) + if (!checkThreadSafetyAttrSubject(S, D, AL, true)) return; if (!AL.checkAtLeastNumArgs(S, 1)) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 34ed5dffa11b4..ee6e7dccca16e 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -3662,7 +3662,7 @@ bool Sema::InstantiateClassImpl( Attr *NewAttr = instantiateTemplateAttribute(I->TmplAttr, Context, *this, TemplateArgs); - if (NewAttr) + if (NewAttr && checkDependentThreadSafetyAttrs(I->NewDecl, NewAttr)) I->NewDecl->addAttr(NewAttr); LocalInstantiationScope::deleteScopes(I->Scope, Instantiator.getStartingScope()); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 61878cba32235..dd2a20614d5f3 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -838,7 +838,8 @@ void Sema::InstantiateAttrsForDecl( Attr *NewAttr = sema::instantiateTemplateAttributeForDecl( TmplAttr, Context, *this, TemplateArgs); - if (NewAttr && isRelevantAttr(*this, New, NewAttr)) + if (NewAttr && isRelevantAttr(*this, New, NewAttr) && + checkDependentThreadSafetyAttrs(New, NewAttr)) New->addAttr(NewAttr); } } @@ -1060,7 +1061,8 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, Attr *NewAttr = sema::instantiateTemplateAttribute(TmplAttr, Context, *this, TemplateArgs); - if (NewAttr && isRelevantAttr(*this, New, TmplAttr)) + if (NewAttr && isRelevantAttr(*this, New, TmplAttr) && + checkDependentThreadSafetyAttrs(New, NewAttr)) New->addAttr(NewAttr); } } diff --git a/clang/test/Sema/attr-capabilities.c b/clang/test/Sema/attr-capabilities.c index 12b18b687803e..6d594bca07535 100644 --- a/clang/test/Sema/attr-capabilities.c +++ b/clang/test/Sema/attr-capabilities.c @@ -13,9 +13,9 @@ struct __attribute__((capability("custom"))) CustomName {}; int Test1 __attribute__((capability("test1"))); // expected-error {{'capability' attribute only applies to structs, unions, classes, and typedefs}} int Test2 __attribute__((shared_capability("test2"))); // expected-error {{'shared_capability' attribute only applies to structs, unions, classes, and typedefs}} -int Test3 __attribute__((acquire_capability("test3"))); // expected-warning {{'acquire_capability' attribute only applies to functions}} -int Test4 __attribute__((try_acquire_capability("test4"))); // expected-warning {{'try_acquire_capability' attribute only applies to functions}} -int Test5 __attribute__((release_capability("test5"))); // expected-warning {{'release_capability' attribute only applies to functions}} +int Test3 __attribute__((acquire_capability("test3"))); // expected-warning {{'acquire_capability' attribute on a variable requires the variable to be of function pointer type}} +int Test4 __attribute__((try_acquire_capability("test4"))); // expected-warning {{'try_acquire_capability' attribute on a variable requires the variable to be of function pointer type}} +int Test5 __attribute__((release_capability("test5"))); // expected-warning {{'release_capability' attribute on a variable requires the variable to be of function pointer type}} struct __attribute__((capability(12))) Test3 {}; // expected-error {{expected string literal as argument of 'capability' attribute}} struct __attribute__((shared_capability(Test2))) Test4 {}; // expected-error {{expected string literal as argument of 'shared_capability' attribute}} diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 0fd382eb02b78..533009c47ab5d 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -282,6 +282,27 @@ struct TestInit test_init(void) { return foo; } +// Function pointer struct members. +struct FPOps { + struct Mutex mu; + int a GUARDED_BY(&mu); + void (*lock)(void) EXCLUSIVE_LOCK_FUNCTION(&mu); + void (*unlock)(void) UNLOCK_FUNCTION(&mu); + void (*requires_mu)(void) EXCLUSIVE_LOCKS_REQUIRED(&mu); +}; + +void test_fp_ops(struct FPOps *ops) { + ops->lock(); + ops->a = 42; + ops->requires_mu(); + ops->unlock(); +} + +void test_fp_ops_fail(struct FPOps *ops) { + ops->a = 42; // expected-warning {{writing variable 'a' requires holding mutex '&FPOps::mu' exclusively}} + ops->requires_mu(); // expected-warning {{calling function 'requires_mu' requires holding mutex '&FPOps::mu' exclusively}} +} + // We had a problem where we'd skip all attributes that follow a late-parsed // attribute in a single __attribute__. void run(void) __attribute__((guarded_by(mu1), guarded_by(mu1))); // expected-warning 2{{only applies to non-static data members and global variables}} diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 079e068ca7811..110d304502045 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -7885,3 +7885,132 @@ void test3() { } } // namespace WideStringLiteral + +namespace FunctionPointers { + +Mutex mu; +int x GUARDED_BY(mu); + +void (*lock_fn)(void) EXCLUSIVE_LOCK_FUNCTION(mu); +void (*shared_lock_fn)(void) SHARED_LOCK_FUNCTION(mu); +void (*unlock_fn)(void) UNLOCK_FUNCTION(mu); +void (*shared_unlock_fn)(void) SHARED_UNLOCK_FUNCTION(mu); +void (*requires_fn)(void) EXCLUSIVE_LOCKS_REQUIRED(mu); +void (*shared_requires_fn)(void) SHARED_LOCKS_REQUIRED(mu); +void (*excludes_fn)(void) LOCKS_EXCLUDED(mu); +bool (*try_lock_fn)(void) EXCLUSIVE_TRYLOCK_FUNCTION(true, mu); +bool (*shared_try_lock_fn)(void) SHARED_TRYLOCK_FUNCTION(true, mu); +void (*assert_fn)(void) ASSERT_EXCLUSIVE_LOCK(mu); +void (*shared_assert_fn)(void) ASSERT_SHARED_LOCK(mu); + +void testAcquireRelease() { + lock_fn(); + x = 1; + unlock_fn(); +} + +void testSharedAcquireRelease() { + shared_lock_fn(); + (void)x; + shared_unlock_fn(); +} + +void testSharedAcquireWriteFail() { + shared_lock_fn(); + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + shared_unlock_fn(); +} + +void testAcquireNoRelease() { + lock_fn(); // expected-note {{mutex acquired here}} + x = 1; +} // expected-warning {{mutex 'mu' is still held at the end of function}} + +void testNoAcquire() { + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} +} + +void testRequires() { + mu.Lock(); + requires_fn(); + mu.Unlock(); +} + +void testRequiresFail() { + requires_fn(); // expected-warning {{calling function 'requires_fn' requires holding mutex 'mu' exclusively}} +} + +void testSharedRequires() { + mu.ReaderLock(); + shared_requires_fn(); + mu.ReaderUnlock(); +} + +void testSharedRequiresFail() { + shared_requires_fn(); // expected-warning {{calling function 'shared_requires_fn' requires holding mutex 'mu'}} +} + +void testExcludes() { + excludes_fn(); +} + +void testExcludesFail() { + mu.Lock(); + excludes_fn(); // expected-warning {{cannot call function 'excludes_fn' while mutex 'mu' is held}} + mu.Unlock(); +} + +void testTryLock() { + if (try_lock_fn()) { + x = 1; + mu.Unlock(); + } +} + +void testSharedTryLock() { + if (shared_try_lock_fn()) { + (void)x; + mu.Unlock(); + } +} + +void testAssert() { + assert_fn(); + x = 1; +} + +void testSharedAssert() { + shared_assert_fn(); + (void)x; +} + +struct Ops { + void (*lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu); + void (*unlock)(void) UNLOCK_FUNCTION(mu); + void (*do_thing)(void) EXCLUSIVE_LOCKS_REQUIRED(mu); + void (*read_thing)(void) SHARED_LOCKS_REQUIRED(mu); +}; + +void testStructOps(Ops *ops) { + ops->lock(); + x = 1; + ops->do_thing(); + ops->read_thing(); + ops->unlock(); +} + +void testStructFail(Ops *ops) { + ops->do_thing(); // expected-warning {{calling function 'do_thing' requires holding mutex 'mu' exclusively}} + ops->read_thing(); // expected-warning {{calling function 'read_thing' requires holding mutex 'mu'}} +} + +// Incompatible reassignment not an error. +void otherLock(); +void testReassignIncompatible() { + lock_fn = otherLock; + lock_fn(); + x = 1; + mu.Unlock(); +} + +} // namespace FunctionPointers diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 862b2d58238c9..ef447fb2d0117 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -596,23 +596,23 @@ int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusiv int elf_testfn(int y) { int x EXCLUSIVE_LOCK_FUNCTION() = y; // \ - // expected-warning {{'exclusive_lock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_lock_function' attribute on a variable requires the variable to be of function pointer type}} return x; }; int elf_test_var EXCLUSIVE_LOCK_FUNCTION(); // \ - // expected-warning {{'exclusive_lock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_lock_function' attribute on a variable requires the variable to be of function pointer type}} class ElfFoo { private: int test_field EXCLUSIVE_LOCK_FUNCTION(); // \ - // expected-warning {{'exclusive_lock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_lock_function' attribute on a field requires the field to be of function pointer type}} void test_method() EXCLUSIVE_LOCK_FUNCTION(); // \ // expected-warning {{'exclusive_lock_function' attribute without capability arguments refers to 'this', but 'ElfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \ - // expected-warning {{'exclusive_lock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_lock_function' attribute only applies to functions, parameters, variables, and non-static data members}} }; void elf_fun_params1(MutexLock& scope EXCLUSIVE_LOCK_FUNCTION(mu1)); @@ -690,12 +690,12 @@ int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock int slf_testfn(int y) { int x SHARED_LOCK_FUNCTION() = y; // \ - // expected-warning {{'shared_lock_function' attribute only applies to functions}} + // expected-warning {{'shared_lock_function' attribute on a variable requires the variable to be of function pointer type}} return x; }; int slf_test_var SHARED_LOCK_FUNCTION(); // \ - // expected-warning {{'shared_lock_function' attribute only applies to functions}} + // expected-warning {{'shared_lock_function' attribute on a variable requires the variable to be of function pointer type}} void slf_fun_params1(MutexLock& scope SHARED_LOCK_FUNCTION(mu1)); void slf_fun_params2(int lvar SHARED_LOCK_FUNCTION(mu1)); // \ @@ -706,13 +706,13 @@ void slf_fun_params3(MutexLock& scope SHARED_LOCK_FUNCTION()); // \ class SlfFoo { private: int test_field SHARED_LOCK_FUNCTION(); // \ - // expected-warning {{'shared_lock_function' attribute only applies to functions}} + // expected-warning {{'shared_lock_function' attribute on a field requires the field to be of function pointer type}} void test_method() SHARED_LOCK_FUNCTION(); // \ // expected-warning {{'shared_lock_function' attribute without capability arguments refers to 'this', but 'SlfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; class SHARED_LOCK_FUNCTION() SlfTestClass { // \ - // expected-warning {{'shared_lock_function' attribute only applies to functions}} + // expected-warning {{'shared_lock_function' attribute only applies to functions, parameters, variables, and non-static data members}} }; // Check argument parsing. @@ -790,27 +790,27 @@ int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ int etf_testfn(int y) { int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute on a variable requires the variable to be of function pointer type}} return x; }; int etf_test_var EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute on a variable requires the variable to be of function pointer type}} class EtfFoo { private: int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute on a field requires the field to be of function pointer type}} void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'exclusive_trylock_function' attribute without capability arguments refers to 'this', but 'EtfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute only applies to functions, variables, and non-static data members}} }; void etf_fun_params(int lvar EXCLUSIVE_TRYLOCK_FUNCTION(1)); // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute only applies to functions, variables, and non-static data members}} // Check argument parsing. @@ -885,27 +885,27 @@ int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1); // \ int stf_testfn(int y) { int x SHARED_TRYLOCK_FUNCTION(1) = y; // \ - // expected-warning {{'shared_trylock_function' attribute only applies to functions}} + // expected-warning {{'shared_trylock_function' attribute on a variable requires the variable to be of function pointer type}} return x; }; int stf_test_var SHARED_TRYLOCK_FUNCTION(1); // \ - // expected-warning {{'shared_trylock_function' attribute only applies to functions}} + // expected-warning {{'shared_trylock_function' attribute on a variable requires the variable to be of function pointer type}} void stf_fun_params(int lvar SHARED_TRYLOCK_FUNCTION(1)); // \ - // expected-warning {{'shared_trylock_function' attribute only applies to functions}} + // expected-warning {{'shared_trylock_function' attribute only applies to functions, variables, and non-static data members}} class StfFoo { private: int test_field SHARED_TRYLOCK_FUNCTION(1); // \ - // expected-warning {{'shared_trylock_function' attribute only applies to functions}} + // expected-warning {{'shared_trylock_function' attribute on a field requires the field to be of function pointer type}} void test_method() SHARED_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'shared_trylock_function' attribute without capability arguments refers to 'this', but 'StfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \ - // expected-warning {{'shared_trylock_function' attribute only applies to functions}} + // expected-warning {{'shared_trylock_function' attribute only applies to functions, variables, and non-static data members}} }; // Check argument parsing. @@ -978,17 +978,17 @@ int uf_testfn(int y) UNLOCK_FUNCTION(); //\ int uf_testfn(int y) { int x UNLOCK_FUNCTION() = y; // \ - // expected-warning {{'unlock_function' attribute only applies to functions}} + // expected-warning {{'unlock_function' attribute on a variable requires the variable to be of function pointer type}} return x; }; int uf_test_var UNLOCK_FUNCTION(); // \ - // expected-warning {{'unlock_function' attribute only applies to functions}} + // expected-warning {{'unlock_function' attribute on a variable requires the variable to be of function pointer type}} class UfFoo { private: int test_field UNLOCK_FUNCTION(); // \ - // expected-warning {{'unlock_function' attribute only applies to functions}} + // expected-warning {{'unlock_function' attribute on a field requires the field to be of function pointer type}} void test_method() UNLOCK_FUNCTION(); // \ // expected-warning {{'unlock_function' attribute without capability arguments refers to 'this', but 'UfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; @@ -1143,12 +1143,12 @@ int le_testfn(int y) LOCKS_EXCLUDED(mu1); int le_testfn(int y) { int x LOCKS_EXCLUDED(mu1) = y; // \ - // expected-warning {{'locks_excluded' attribute only applies to functions}} + // expected-warning {{'locks_excluded' attribute on a variable requires the variable to be of function pointer type}} return x; }; int le_test_var LOCKS_EXCLUDED(mu1); // \ - // expected-warning {{'locks_excluded' attribute only applies to functions}} + // expected-warning {{'locks_excluded' attribute on a variable requires the variable to be of function pointer type}} void le_fun_params1(MutexLock& scope LOCKS_EXCLUDED(mu1)); void le_fun_params2(int lvar LOCKS_EXCLUDED(mu1)); // \ @@ -1157,12 +1157,12 @@ void le_fun_params2(int lvar LOCKS_EXCLUDED(mu1)); // \ class LeFoo { private: int test_field LOCKS_EXCLUDED(mu1); // \ - // expected-warning {{'locks_excluded' attribute only applies to functions}} + // expected-warning {{'locks_excluded' attribute on a field requires the field to be of function pointer type}} void test_method() LOCKS_EXCLUDED(mu1); }; class LOCKS_EXCLUDED(mu1) LeTestClass { // \ - // expected-warning {{'locks_excluded' attribute only applies to functions}} + // expected-warning {{'locks_excluded' attribute only applies to functions, parameters, variables, and non-static data members}} }; // Check argument parsing. @@ -1228,12 +1228,12 @@ int elr_testfn(int y) EXCLUSIVE_LOCKS_REQUIRED(mu1); int elr_testfn(int y) { int x EXCLUSIVE_LOCKS_REQUIRED(mu1) = y; // \ - // expected-warning {{'exclusive_locks_required' attribute only applies to functions}} + // expected-warning {{'exclusive_locks_required' attribute on a variable requires the variable to be of function pointer type}} return x; }; int elr_test_var EXCLUSIVE_LOCKS_REQUIRED(mu1); // \ - // expected-warning {{'exclusive_locks_required' attribute only applies to functions}} + // expected-warning {{'exclusive_locks_required' attribute on a variable requires the variable to be of function pointer type}} void elr_fun_params1(MutexLock& scope EXCLUSIVE_LOCKS_REQUIRED(mu1)); void elr_fun_params2(int lvar EXCLUSIVE_LOCKS_REQUIRED(mu1)); // \ @@ -1242,12 +1242,12 @@ void elr_fun_params2(int lvar EXCLUSIVE_LOCKS_REQUIRED(mu1)); // \ class ElrFoo { private: int test_field EXCLUSIVE_LOCKS_REQUIRED(mu1); // \ - // expected-warning {{'exclusive_locks_required' attribute only applies to functions}} + // expected-warning {{'exclusive_locks_required' attribute on a field requires the field to be of function pointer type}} void test_method() EXCLUSIVE_LOCKS_REQUIRED(mu1); }; class EXCLUSIVE_LOCKS_REQUIRED(mu1) ElrTestClass { // \ - // expected-warning {{'exclusive_locks_required' attribute only applies to functions}} + // expected-warning {{'exclusive_locks_required' attribute only applies to functions, parameters, variables, and non-static data members}} }; // Check argument parsing. @@ -1315,12 +1315,12 @@ int slr_testfn(int y) SHARED_LOCKS_REQUIRED(mu1); int slr_testfn(int y) { int x SHARED_LOCKS_REQUIRED(mu1) = y; // \ - // expected-warning {{'shared_locks_required' attribute only applies to functions}} + // expected-warning {{'shared_locks_required' attribute on a variable requires the variable to be of function pointer type}} return x; }; int slr_test_var SHARED_LOCKS_REQUIRED(mu1); // \ - // expected-warning {{'shared_locks_required' attribute only applies to functions}} + // expected-warning {{'shared_locks_required' attribute on a variable requires the variable to be of function pointer type}} void slr_fun_params1(MutexLock& scope SHARED_LOCKS_REQUIRED(mu1)); void slr_fun_params2(int lvar SHARED_LOCKS_REQUIRED(mu1)); // \ @@ -1329,12 +1329,12 @@ void slr_fun_params2(int lvar SHARED_LOCKS_REQUIRED(mu1)); // \ class SlrFoo { private: int test_field SHARED_LOCKS_REQUIRED(mu1); // \ - // expected-warning {{'shared_locks_required' attribute only applies to functions}} + // expected-warning {{'shared_locks_required' attribute on a field requires the field to be of function pointer type}} void test_method() SHARED_LOCKS_REQUIRED(mu1); }; class SHARED_LOCKS_REQUIRED(mu1) SlrTestClass { // \ - // expected-warning {{'shared_locks_required' attribute only applies to functions}} + // expected-warning {{'shared_locks_required' attribute only applies to functions, parameters, variables, and non-static data members}} }; // Check argument parsing. @@ -1746,3 +1746,46 @@ namespace CRASH_POST_R301735 { }; } #endif + +//-----------------------------------------// +// Function pointer attributes +//-----------------------------------------// + +namespace FunctionPointers { + +void (*fp_lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1); +void (*fp_unlock)(void) UNLOCK_FUNCTION(mu1); +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); +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); + +struct FPFields { + void (*lock)(void) EXCLUSIVE_LOCK_FUNCTION(mu1); + void (*unlock)(void) UNLOCK_FUNCTION(mu1); + void (*requires_mu)(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 { + int bad_field EXCLUSIVE_LOCKS_REQUIRED(mu1); // \ + // expected-warning {{'exclusive_locks_required' attribute on a field requires the field to be of function pointer type}} +}; + +template <typename FuncPtr> +struct DependentFPFields { + FuncPtr lock EXCLUSIVE_LOCK_FUNCTION(mu1); // \ + // 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}} +}; + +typedef void (*GoodLockFn)(void); +DependentFPFields<GoodLockFn> dependent_fp_fields_ok; +DependentFPFields<int> dependent_fp_fields_bad; // expected-note {{in instantiation of template class 'FunctionPointers::DependentFPFields<int>' requested here}} + +} // namespace FunctionPointers _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
