Author: jmgao Date: Tue Aug 8 12:44:35 2017 New Revision: 310403 URL: http://llvm.org/viewvc/llvm-project?rev=310403&view=rev Log: Thread Safety Analysis: warn on nonsensical attributes.
Add warnings in cases where an implicit `this` argument is expected to attributes because either `this` doesn't exist because the attribute is on a free function, or because `this` is on a type that doesn't have a corresponding capability/lockable/scoped_lockable attribute. Reviewers: delesley, aaron.ballman Differential Revision: https://reviews.llvm.org/D36237 Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/test/Sema/attr-capabilities.c cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=310403&r1=310402&r2=310403&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug 8 12:44:35 2017 @@ -2932,6 +2932,16 @@ def warn_thread_attribute_decl_not_locka "%0 attribute can only be applied in a context annotated " "with 'capability(\"mutex\")' attribute">, InGroup<ThreadSafetyAttributes>, DefaultIgnore; +def warn_thread_attribute_noargs_not_lockable : Warning< + "%0 attribute requires type annotated with 'capability' attribute; " + "type here is %1">, + InGroup<ThreadSafetyAttributes>, DefaultIgnore; +def warn_thread_attribute_noargs_not_method : Warning< + "%0 attribute without arguments can only be applied to a method of a class">, + InGroup<ThreadSafetyAttributes>, DefaultIgnore; +def warn_thread_attribute_noargs_static_method : Warning< + "%0 attribute without arguments cannot be applied to a static method">, + InGroup<ThreadSafetyAttributes>, DefaultIgnore; def warn_thread_attribute_decl_not_pointer : Warning< "%0 only applies to pointer types; type here is %1">, InGroup<ThreadSafetyAttributes>, DefaultIgnore; Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=310403&r1=310402&r2=310403&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug 8 12:44:35 2017 @@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q return nullptr; } -static bool checkRecordTypeForCapability(Sema &S, QualType Ty) { +template <typename T> static bool checkRecordTypeForAttr(Sema &S, QualType Ty) { const RecordType *RT = getRecordType(Ty); if (!RT) @@ -497,7 +497,7 @@ static bool checkRecordTypeForCapability // Check if the record itself has a capability. RecordDecl *RD = RT->getDecl(); - if (RD->hasAttr<CapabilityAttr>()) + if (RD->hasAttr<T>()) return true; // Else check if any base classes have a capability. @@ -505,14 +505,14 @@ static bool checkRecordTypeForCapability CXXBasePaths BPaths(false, false); if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) { const auto *Type = BS->getType()->getAs<RecordType>(); - return Type->getDecl()->hasAttr<CapabilityAttr>(); + return Type->getDecl()->hasAttr<T>(); }, BPaths)) return true; } return false; } -static bool checkTypedefTypeForCapability(QualType Ty) { +template <typename T> static bool checkTypedefTypeForAttr(QualType Ty) { const auto *TD = Ty->getAs<TypedefType>(); if (!TD) return false; @@ -521,19 +521,27 @@ static bool checkTypedefTypeForCapabilit if (!TN) return false; - return TN->hasAttr<CapabilityAttr>(); + return TN->hasAttr<T>(); } -static bool typeHasCapability(Sema &S, QualType Ty) { - if (checkTypedefTypeForCapability(Ty)) +template <typename T> static bool typeHasAttr(Sema &S, QualType Ty) { + if (checkTypedefTypeForAttr<T>(Ty)) return true; - if (checkRecordTypeForCapability(S, Ty)) + if (checkRecordTypeForAttr<T>(S, Ty)) return true; return false; } +static bool typeHasCapability(Sema &S, QualType Ty) { + return typeHasAttr<CapabilityAttr>(S, Ty); +} + +static bool typeHasScopedLockable(Sema &S, QualType Ty) { + return typeHasAttr<ScopedLockableAttr>(S, Ty); +} + static bool isCapabilityExpr(Sema &S, const Expr *Ex) { // Capability expressions are simple expressions involving the boolean logic // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once @@ -570,6 +578,8 @@ static void checkAttrArgsAreCapabilityOb SmallVectorImpl<Expr *> &Args, int Sidx = 0, bool ParamIdxOk = false) { + bool TriedParam = false; + for (unsigned Idx = Sidx; Idx < Attr.getNumArgs(); ++Idx) { Expr *ArgExp = Attr.getArgAsExpr(Idx); @@ -610,15 +620,18 @@ static void checkAttrArgsAreCapabilityOb const RecordType *RT = getRecordType(ArgTy); // Now check if we index into a record type function param. - if(!RT && ParamIdxOk) { + if (!RT && ParamIdxOk) { FunctionDecl *FD = dyn_cast<FunctionDecl>(D); IntegerLiteral *IL = dyn_cast<IntegerLiteral>(ArgExp); - if(FD && IL) { + if (FD && IL) { + // Don't emit free function warnings if an index was given. + TriedParam = true; + unsigned int NumParams = FD->getNumParams(); llvm::APInt ArgValue = IL->getValue(); uint64_t ParamIdxFromOne = ArgValue.getZExtValue(); uint64_t ParamIdxFromZero = ParamIdxFromOne - 1; - if(!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) { + if (!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) { S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_range) << Attr.getName() << Idx + 1 << NumParams; continue; @@ -637,6 +650,28 @@ static void checkAttrArgsAreCapabilityOb Args.push_back(ArgExp); } + + // If we don't have any lockable arguments, verify that we're an instance + // method on a lockable type. + if (Args.empty() && !TriedParam) { + if (auto *MD = dyn_cast<CXXMethodDecl>(D)) { + if (MD->isStatic()) { + S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_static_method) + << Attr.getName(); + return; + } + + QualType ThisType = MD->getThisType(MD->getASTContext()); + if (!typeHasCapability(S, ThisType) && + !typeHasScopedLockable(S, ThisType)) { + S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_not_lockable) + << Attr.getName() << ThisType; + } + } else { + S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_not_method) + << Attr.getName(); + } + } } //===----------------------------------------------------------------------===// Modified: cfe/trunk/test/Sema/attr-capabilities.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-capabilities.c?rev=310403&r1=310402&r2=310403&view=diff ============================================================================== --- cfe/trunk/test/Sema/attr-capabilities.c (original) +++ cfe/trunk/test/Sema/attr-capabilities.c Tue Aug 8 12:44:35 2017 @@ -37,15 +37,25 @@ void Func6(void) __attribute__((requires 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 arguments can only be applied to a method of a class}} +void Func10(void) __attribute__((assert_shared_capability())) {} // expected-warning {{'assert_shared_capability' attribute without arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} +void Func14(void) __attribute__((acquire_shared_capability())) {} // expected-warning {{'acquire_shared_capability' attribute without arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} +void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without arguments can only be applied to a method of a class}} +void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without arguments can only be applied to a method of a class}} + +void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without arguments can only be applied to a method of a class}} +void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without arguments can only be applied to a method of a class}} void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {} void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {} Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp?rev=310403&r1=310402&r2=310403&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp Tue Aug 8 12:44:35 2017 @@ -571,11 +571,11 @@ UnlockableMu ab_var_arg_bad_5 ACQUIRED_B // 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 arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} int elf_testfn(int y) { int x EXCLUSIVE_LOCK_FUNCTION() = y; // \ @@ -590,7 +590,7 @@ class ElfFoo { 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 requires type annotated with 'capability' attribute; type here is 'ElfFoo *'}} }; class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \ @@ -643,11 +643,11 @@ int elf_function_bad_7() EXCLUSIVE_LOCK_ // 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 arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} int slf_testfn(int y) { int x SHARED_LOCK_FUNCTION() = y; // \ @@ -665,7 +665,8 @@ class SlfFoo { 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 requires type annotated with 'capability' attribute; type here is 'SlfFoo *'}} }; class SHARED_LOCK_FUNCTION() SlfTestClass { // \ @@ -716,14 +717,16 @@ int slf_function_bad_7() SHARED_LOCK_FUN // 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)); // \ - // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} +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 arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} int etf_testfn(int y) { int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \ @@ -732,13 +735,14 @@ int etf_testfn(int y) { }; int etf_test_var EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ - // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} + // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} \ class EtfFoo { 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 requires type annotated with 'capability' attribute; type here is 'EtfFoo *'}} }; class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \ @@ -759,7 +763,8 @@ int etf_function_5() EXCLUSIVE_TRYLOCK_F 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 arguments can only be applied to a method of a class}} // illegal attribute arguments @@ -794,9 +799,11 @@ void stf_function() __attribute__((share 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 arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} int stf_testfn(int y) { int x SHARED_TRYLOCK_FUNCTION(1) = y; // \ @@ -815,7 +822,8 @@ class StfFoo { 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 requires type annotated with 'capability' attribute; type here is 'StfFoo *'}} }; class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \ @@ -833,7 +841,8 @@ int stf_function_5() SHARED_TRYLOCK_FUNC 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 arguments can only be applied to a method of a class}} // illegal attribute arguments @@ -862,11 +871,14 @@ int stf_function_bad_6() SHARED_TRYLOCK_ // 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 arguments can only be applied to a method 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 arguments can only be applied to a method of a class}} int uf_testfn(int y) { int x UNLOCK_FUNCTION() = y; // \ @@ -881,7 +893,8 @@ class UfFoo { 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 requires type annotated with 'capability' attribute; type here is 'UfFoo *'}} }; class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \ @@ -1524,4 +1537,4 @@ namespace CRASH_POST_R301735 { Mutex mu_; }; } -#endif \ No newline at end of file +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits