Index: include/clang/Basic/Attr.td =================================================================== --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -2138,7 +2138,7 @@ let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let DuplicatesAllowedWhileMerging = 1; - let Args = [ExprArgument<"Expr">]; + let Args = [VariadicExprArgument<"Args">]; let Accessors = [Accessor<"isShared", [GNU<"assert_shared_capability">, CXX11<"clang", "assert_shared_capability">]>]; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -2932,6 +2932,17 @@ "%0 attribute can only be applied in a context annotated " "with 'capability(\"mutex\")' attribute">, InGroup, DefaultIgnore; +def warn_thread_attribute_noargs_not_lockable : Warning< + "%0 attribute requires type annotated with 'capability' attribute; " + "type here is %1">, + InGroup, DefaultIgnore; +def warn_thread_attribute_noargs_not_method : Warning< + "%0 attribute without arguments cannot be applied to a free function">, + InGroup, DefaultIgnore; +def warn_thread_attribute_noargs_static_method : Warning< + "%0 attribute without arguments can only be applied on an instance method " + " on a type with 'capability' attribute">, + InGroup, DefaultIgnore; def warn_thread_attribute_decl_not_pointer : Warning< "%0 only applies to pointer types; type here is %1">, InGroup, DefaultIgnore; Index: lib/Analysis/ThreadSafety.cpp =================================================================== --- lib/Analysis/ThreadSafety.cpp +++ lib/Analysis/ThreadSafety.cpp @@ -1735,8 +1735,23 @@ CapExprSet AssertLocks; Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); for (const auto &AssertLock : AssertLocks) - Analyzer->addLock(FSet, llvm::make_unique( - AssertLock, LK_Shared, Loc, false, true), + Analyzer->addLock(FSet, + llvm::make_unique( + AssertLock, LK_Shared, Loc, false, true), + ClassifyDiagnostic(A)); + break; + } + + case attr::AssertCapability: { + AssertCapabilityAttr *A = cast(At); + CapExprSet AssertLocks; + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (const auto &AssertLock : AssertLocks) + Analyzer->addLock(FSet, + llvm::make_unique( + AssertLock, + A->isShared() ? LK_Shared : LK_Exclusive, Loc, + false, true), ClassifyDiagnostic(A)); break; } Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -480,7 +480,7 @@ return nullptr; } -static bool checkRecordTypeForCapability(Sema &S, QualType Ty) { +template static bool checkRecordTypeForAttr(Sema &S, QualType Ty) { const RecordType *RT = getRecordType(Ty); if (!RT) @@ -497,7 +497,7 @@ // Check if the record itself has a capability. RecordDecl *RD = RT->getDecl(); - if (RD->hasAttr()) + if (RD->hasAttr()) return true; // Else check if any base classes have a capability. @@ -505,14 +505,14 @@ CXXBasePaths BPaths(false, false); if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) { const auto *Type = BS->getType()->getAs(); - return Type->getDecl()->hasAttr(); + return Type->getDecl()->hasAttr(); }, BPaths)) return true; } return false; } -static bool checkTypedefTypeForCapability(QualType Ty) { +template static bool checkTypedefTypeForAttr(QualType Ty) { const auto *TD = Ty->getAs(); if (!TD) return false; @@ -521,17 +521,25 @@ if (!TN) return false; - return TN->hasAttr(); + return TN->hasAttr(); +} + +template static bool typeHasAttr(Sema &S, QualType Ty) { + if (checkTypedefTypeForAttr(Ty)) + return true; + + if (checkRecordTypeForAttr(S, Ty)) + return true; + + return false; } static bool typeHasCapability(Sema &S, QualType Ty) { - if (checkTypedefTypeForCapability(Ty)) - return true; + return typeHasAttr(S, Ty); +} - if (checkRecordTypeForCapability(S, Ty)) - return true; - - return false; +static bool typeHasScopedLockable(Sema &S, QualType Ty) { + return typeHasAttr(S, Ty); } static bool isCapabilityExpr(Sema &S, const Expr *Ex) { @@ -570,6 +578,8 @@ SmallVectorImpl &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 @@ 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(D); IntegerLiteral *IL = dyn_cast(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 @@ 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(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(); + } + } } //===----------------------------------------------------------------------===// @@ -5686,8 +5721,12 @@ static void handleAssertCapabilityAttr(Sema &S, Decl *D, const AttributeList &Attr) { + SmallVector Args; + if (!checkLockFunAttrCommon(S, D, Attr, Args)) + return; + D->addAttr(::new (S.Context) AssertCapabilityAttr(Attr.getRange(), S.Context, - Attr.getArgAsExpr(0), + Args.data(), Args.size(), Attr.getAttributeSpellingListIndex())); } Index: test/Sema/attr-capabilities.c =================================================================== --- test/Sema/attr-capabilities.c +++ test/Sema/attr-capabilities.c @@ -37,18 +37,25 @@ void Func7(void) __attribute__((assert_capability(GUI))) {} void Func8(void) __attribute__((assert_shared_capability(GUI))) {} -void Func9(void) __attribute__((assert_capability())) {} // expected-error {{'assert_capability' attribute takes one argument}} -void Func10(void) __attribute__((assert_shared_capability())) {} // expected-error {{'assert_shared_capability' attribute takes one argument}} +void Func9(void) __attribute__((assert_capability())) {} // expected-warning {{'assert_capability' attribute without arguments cannot be applied to a free function}} +void Func10(void) __attribute__((assert_shared_capability())) {} // expected-warning {{'assert_shared_capability' attribute without arguments cannot be applied to a free function}} 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 cannot be applied to a free function}} +void Func14(void) __attribute__((acquire_shared_capability())) {} // expected-warning {{'acquire_shared_capability' attribute without arguments cannot be applied to a free function}} + 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 cannot be applied to a free function}} +void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without arguments cannot be applied to a free function}} +void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without arguments cannot be applied to a free function}} + +void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without arguments cannot be applied to a free function}} +void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without arguments cannot be applied to a free function}} 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-analysis.cpp =================================================================== --- test/SemaCXX/warn-thread-safety-analysis.cpp +++ test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s @@ -13,8 +14,15 @@ #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__))) #define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__((exclusive_lock_function(__VA_ARGS__))) #define SHARED_LOCK_FUNCTION(...) __attribute__((shared_lock_function(__VA_ARGS__))) + +#if USE_ASSERT_CAPABILITY +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_capability(__VA_ARGS__))) +#define ASSERT_SHARED_LOCK(...) __attribute__((assert_shared_capability(__VA_ARGS__))) +#else #define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_exclusive_lock(__VA_ARGS__))) #define ASSERT_SHARED_LOCK(...) __attribute__((assert_shared_lock(__VA_ARGS__))) +#endif + #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__((exclusive_trylock_function(__VA_ARGS__))) #define SHARED_TRYLOCK_FUNCTION(...) __attribute__((shared_trylock_function(__VA_ARGS__))) #define UNLOCK_FUNCTION(...) __attribute__((unlock_function(__VA_ARGS__))) @@ -3675,9 +3683,14 @@ SHARED_TRYLOCK_FUNCTION(true, mu2_); void assertBoth() ASSERT_EXCLUSIVE_LOCK(mu1_) ASSERT_EXCLUSIVE_LOCK(mu2_); + + void alsoAssertBoth() ASSERT_EXCLUSIVE_LOCK(mu1_, mu2_); + void assertShared() ASSERT_SHARED_LOCK(mu1_) ASSERT_SHARED_LOCK(mu2_); + void alsoAssertShared() ASSERT_SHARED_LOCK(mu1_, mu2_); + void test(); void testAssert(); void testAssertShared(); @@ -3741,17 +3754,33 @@ // Force duplication of attributes void Foo::assertBoth() { } +void Foo::alsoAssertBoth() { } void Foo::assertShared() { } +void Foo::alsoAssertShared() { } void Foo::testAssert() { - assertBoth(); - a = 0; - b = 0; + { + assertBoth(); + a = 0; + b = 0; + } + { + alsoAssertBoth(); + a = 0; + b = 0; + } } void Foo::testAssertShared() { - assertShared(); - int zz = a + b; + { + assertShared(); + int zz = a + b; + } + + { + alsoAssertShared(); + int zz = a + b; + } } Index: test/SemaCXX/warn-thread-safety-parsing.cpp =================================================================== --- test/SemaCXX/warn-thread-safety-parsing.cpp +++ test/SemaCXX/warn-thread-safety-parsing.cpp @@ -571,11 +571,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 arguments cannot be applied to a free function}} 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 cannot be applied to a free function}} int elf_testfn(int y) { int x EXCLUSIVE_LOCK_FUNCTION() = y; // \ @@ -590,7 +590,7 @@ 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 @@ // 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 cannot be applied to a free function}} 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 cannot be applied to a free function}} int slf_testfn(int y) { int x SHARED_LOCK_FUNCTION() = y; // \ @@ -665,7 +665,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 requires type annotated with 'capability' attribute; type here is 'SlfFoo *'}} }; class SHARED_LOCK_FUNCTION() SlfTestClass { // \ @@ -716,14 +717,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)); // \ - // 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 cannot be applied to a free function}} -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 cannot be applied to a free function}} int etf_testfn(int y) { int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \ @@ -732,13 +735,14 @@ }; 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_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 cannot be applied to a free function}} // illegal attribute arguments @@ -794,9 +799,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 arguments cannot be applied to a free function}} -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 cannot be applied to a free function}} int stf_testfn(int y) { int x SHARED_TRYLOCK_FUNCTION(1) = y; // \ @@ -815,7 +822,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 requires type annotated with 'capability' attribute; type here is 'StfFoo *'}} }; class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \ @@ -833,7 +841,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 arguments cannot be applied to a free function}} // illegal attribute arguments @@ -862,11 +871,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 arguments cannot be applied to a free function}} + 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 cannot be applied to a free function}} int uf_testfn(int y) { int x UNLOCK_FUNCTION() = y; // \ @@ -881,7 +893,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 requires type annotated with 'capability' attribute; type here is 'UfFoo *'}} }; class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \