Sorry for the breakage, I'm reverting the patch that caused this now. On Fri, Aug 11, 2017 at 12:36 AM, NAKAMURA Takumi <geek4ci...@gmail.com> wrote:
> It causes failure in check-libcxx with just-built clang. > > http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/758 > > > On Wed, Aug 9, 2017 at 4:45 AM Josh Gao via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> 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 >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
