Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -3008,6 +3008,14 @@ def warn_thread_attribute_ignored : Warning< "ignoring %0 attribute because its argument is invalid">, InGroup, DefaultIgnore; +def warn_thread_attribute_not_on_non_static_member : Warning< + "%0 attribute without capability arguments can only be applied to non-static " + "methods of a class">, + InGroup, DefaultIgnore; +def warn_thread_attribute_not_on_capability_member : Warning< + "%0 attribute without capability arguments refers to 'this', but %1 isn't " + "annotated with 'capability' or 'scoped_lockable' attribute">, + InGroup, DefaultIgnore; def warn_thread_attribute_argument_not_lockable : Warning< "%0 attribute requires arguments whose type is annotated " "with 'capability' attribute; type here is %1">, Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -475,6 +475,29 @@ return nullptr; } +template +static bool checkRecordDeclForAttr(const RecordDecl *RD) { + // Check if the record itself has the attribute. + if (RD->hasAttr()) + return true; + + // Else check if any base classes have the attribute. + if (const auto *CRD = dyn_cast(RD)) { + CXXBasePaths BPaths(false, false); + if (CRD->lookupInBases( + [](const CXXBaseSpecifier *BS, CXXBasePath &) { + const auto &Ty = *BS->getType(); + // If it's type-dependent, we assume it could have the attribute. + if (Ty.isDependentType()) + return true; + return Ty.getAs()->getDecl()->hasAttr(); + }, + BPaths, true)) + return true; + } + return false; +} + static bool checkRecordTypeForCapability(Sema &S, QualType Ty) { const RecordType *RT = getRecordType(Ty); @@ -490,21 +513,7 @@ if (threadSafetyCheckIsSmartPointer(S, RT)) return true; - // Check if the record itself has a capability. - RecordDecl *RD = RT->getDecl(); - if (RD->hasAttr()) - return true; - - // Else check if any base classes have a capability. - if (const auto *CRD = dyn_cast(RD)) { - CXXBasePaths BPaths(false, false); - if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) { - const auto *Type = BS->getType()->getAs(); - return Type->getDecl()->hasAttr(); - }, BPaths)) - return true; - } - return false; + return checkRecordDeclForAttr(RT->getDecl()); } static bool checkTypedefTypeForCapability(QualType Ty) { @@ -562,8 +571,26 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, const ParsedAttr &AL, SmallVectorImpl &Args, - int Sidx = 0, + unsigned Sidx = 0, bool ParamIdxOk = false) { + if (Sidx == AL.getNumArgs()) { + // If we don't have any capability arguments, the attribute implicitly + // refers to 'this'. So we need to make sure that 'this' exists, i.e. we're + // a non-static method, and that the class is a (scoped) capability. + const auto *MD = dyn_cast(D); + if (MD && !MD->isStatic()) { + const CXXRecordDecl *RD = MD->getParent(); + // FIXME -- need to check this again on template instantiation + if (!checkRecordDeclForAttr(RD) && + !checkRecordDeclForAttr(RD)) + S.Diag(AL.getLoc(), + diag::warn_thread_attribute_not_on_capability_member) + << AL << MD->getParent(); + } else + S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member) + << AL; + } + for (unsigned Idx = Sidx; Idx < AL.getNumArgs(); ++Idx) { Expr *ArgExp = AL.getArgAsExpr(Idx); Index: test/Sema/attr-capabilities.c =================================================================== --- test/Sema/attr-capabilities.c +++ test/Sema/attr-capabilities.c @@ -37,15 +37,25 @@ void Func7(void) __attribute__((assert_capability(GUI))) {} void Func8(void) __attribute__((assert_shared_capability(GUI))) {} +void Func9(void) __attribute__((assert_capability())) {} // expected-warning {{'assert_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +void Func10(void) __attribute__((assert_shared_capability())) {} // expected-warning {{'assert_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} + void Func11(void) __attribute__((acquire_capability(GUI))) {} void Func12(void) __attribute__((acquire_shared_capability(GUI))) {} +void Func13(void) __attribute__((acquire_capability())) {} // expected-warning {{'acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +void Func14(void) __attribute__((acquire_shared_capability())) {} // expected-warning {{'acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} + void Func15(void) __attribute__((release_capability(GUI))) {} void Func16(void) __attribute__((release_shared_capability(GUI))) {} void Func17(void) __attribute__((release_generic_capability(GUI))) {} -void Func21(void) __attribute__((try_acquire_capability(1))) {} -void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} +void Func18(void) __attribute__((release_capability())) {} // expected-warning {{'release_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without capability arguments can only be applied to non-static methods of a class}} + +void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {} void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {} Index: test/SemaCXX/warn-thread-safety-parsing.cpp =================================================================== --- test/SemaCXX/warn-thread-safety-parsing.cpp +++ test/SemaCXX/warn-thread-safety-parsing.cpp @@ -572,11 +572,11 @@ // takes zero or more arguments, all locks (vars/fields) -void elf_function() EXCLUSIVE_LOCK_FUNCTION(); +void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}} void elf_function_args() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2); -int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); +int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int elf_testfn(int y) { int x EXCLUSIVE_LOCK_FUNCTION() = y; // \ @@ -591,7 +591,8 @@ private: int test_field EXCLUSIVE_LOCK_FUNCTION(); // \ // expected-warning {{'exclusive_lock_function' attribute only applies to functions}} - void test_method() EXCLUSIVE_LOCK_FUNCTION(); + 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 { // \ @@ -644,11 +645,11 @@ // takes zero or more arguments, all locks (vars/fields) -void slf_function() SHARED_LOCK_FUNCTION(); +void slf_function() SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}} void slf_function_args() SHARED_LOCK_FUNCTION(mu1, mu2); -int slf_testfn(int y) SHARED_LOCK_FUNCTION(); +int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int slf_testfn(int y) { int x SHARED_LOCK_FUNCTION() = y; // \ @@ -666,7 +667,8 @@ private: int test_field SHARED_LOCK_FUNCTION(); // \ // expected-warning {{'shared_lock_function' attribute only applies to functions}} - void test_method() SHARED_LOCK_FUNCTION(); + 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 { // \ @@ -717,14 +719,16 @@ // takes a mandatory boolean or integer argument specifying the retval // plus an optional list of locks (vars/fields) -void etf_function() __attribute__((exclusive_trylock_function)); // \ +void etf_function() __attribute__((exclusive_trylock_function)); // \ // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); -void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); +void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ + // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} -int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); +int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ + // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int etf_testfn(int y) { int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \ @@ -739,7 +743,8 @@ private: int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} - void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); + 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 { // \ @@ -760,7 +765,8 @@ int etf_function_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, muRef); int etf_function_7() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu()); int etf_functetfn_8() EXCLUSIVE_TRYLOCK_FUNCTION(1, muPointer); -int etf_function_9() EXCLUSIVE_TRYLOCK_FUNCTION(true); +int etf_function_9() EXCLUSIVE_TRYLOCK_FUNCTION(true); // \ + // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} // illegal attribute arguments @@ -795,9 +801,11 @@ void stf_function_args() SHARED_TRYLOCK_FUNCTION(1, mu2); -void stf_function_arg() SHARED_TRYLOCK_FUNCTION(1); +void stf_function_arg() SHARED_TRYLOCK_FUNCTION(1); // \ + // expected-warning {{'shared_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} -int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1); +int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1); // \ + // expected-warning {{'shared_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int stf_testfn(int y) { int x SHARED_TRYLOCK_FUNCTION(1) = y; // \ @@ -816,7 +824,8 @@ private: int test_field SHARED_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'shared_trylock_function' attribute only applies to functions}} - void test_method() SHARED_TRYLOCK_FUNCTION(1); + 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 { // \ @@ -834,7 +843,8 @@ int stf_function_6() SHARED_TRYLOCK_FUNCTION(1, muRef); int stf_function_7() SHARED_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu()); int stf_function_8() SHARED_TRYLOCK_FUNCTION(1, muPointer); -int stf_function_9() SHARED_TRYLOCK_FUNCTION(true); +int stf_function_9() SHARED_TRYLOCK_FUNCTION(true); // \ + // expected-warning {{'shared_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} // illegal attribute arguments @@ -863,11 +873,14 @@ // takes zero or more arguments, all locks (vars/fields) -void uf_function() UNLOCK_FUNCTION(); +void uf_function() UNLOCK_FUNCTION(); // \ + // expected-warning {{'unlock_function' attribute without capability arguments can only be applied to non-static methods of a class}} + void uf_function_args() UNLOCK_FUNCTION(mu1, mu2); -int uf_testfn(int y) UNLOCK_FUNCTION(); +int uf_testfn(int y) UNLOCK_FUNCTION(); //\ + // expected-warning {{'unlock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int uf_testfn(int y) { int x UNLOCK_FUNCTION() = y; // \ @@ -882,7 +895,8 @@ private: int test_field UNLOCK_FUNCTION(); // \ // expected-warning {{'unlock_function' attribute only applies to functions}} - void test_method() UNLOCK_FUNCTION(); + 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}} }; class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \ @@ -1250,6 +1264,32 @@ } }; +template +struct SCOPED_LOCKABLE SLTemplateClass { + ~SLTemplateClass() UNLOCK_FUNCTION(); +}; + +template +struct NonSLTemplateClass { + ~NonSLTemplateClass() UNLOCK_FUNCTION(); // \ + // expected-warning{{'unlock_function' attribute without capability arguments refers to 'this', but 'NonSLTemplateClass' isn't annotated with 'capability' or 'scoped_lockable' attribute}} +}; + +template <> +struct SLTemplateClass {}; + +template +struct SLTemplateDerived : public SLTemplateClass { + ~SLTemplateDerived() UNLOCK_FUNCTION(); +}; + +// FIXME: warn on template instantiation. +template struct SLTemplateDerived; + +struct SLDerived : public SLTemplateClass { + ~SLDerived() UNLOCK_FUNCTION(); // \ + // expected-warning{{'unlock_function' attribute without capability arguments refers to 'this', but 'SLDerived' isn't annotated with 'capability' or 'scoped_lockable' attribute}} +}; //----------------------------------------------------- // Parsing of member variables and function parameters @@ -1525,4 +1565,4 @@ Mutex mu_; }; } -#endif \ No newline at end of file +#endif