Reverted in r310698. On Fri, Aug 11, 2017 at 12:51 AM, Josh Gao <jm...@google.com> wrote:
> 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/SemaD >>> eclAttr.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_o >>> ut_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_no >>> args_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_no >>> args_not_lockable) >>> + << Attr.getName() << ThisType; >>> + } >>> + } else { >>> + S.Diag(Attr.getLoc(), diag::warn_thread_attribute_no >>> args_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
