From: Josh Gao Date: Tue, 8 Aug 2017 19:44:34 +0000 (+0000) Subject: Reland "Thread Safety Analysis: fix assert_capability." X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=56427e245e62f2cb664f183fd2159bfb4b0e6373;p=clang Reland "Thread Safety Analysis: fix assert_capability." Delete the test that was broken by rL309725, and add it back in a follow up commit. Also, improve the tests a bit. Reviewers: delesley, aaron.ballman Differential Revision: https://reviews.llvm.org/D36237 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@310402 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 431905ece4..70f6b62aca 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -2138,7 +2138,7 @@ def AssertCapability : InheritableAttr { 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">]>]; diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 879a15c9c2..902570b319 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -1735,8 +1735,23 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { 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; } diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index cef84116db..f1e47291da 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -5700,8 +5700,12 @@ static void handleCapabilityAttr(Sema &S, Decl *D, const AttributeList &Attr) { 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())); } diff --git a/test/Sema/attr-capabilities.c b/test/Sema/attr-capabilities.c index 8996770486..88fdf306ac 100644 --- a/test/Sema/attr-capabilities.c +++ b/test/Sema/attr-capabilities.c @@ -37,9 +37,6 @@ void Func6(void) __attribute__((requires_shared_capability(BadCapability))) {} 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 Func11(void) __attribute__((acquire_capability(GUI))) {} void Func12(void) __attribute__((acquire_shared_capability(GUI))) {} diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index bbb4f9b48d..86a2915ae1 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/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 @@ class Foo { 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 @@ void Foo::test() { // 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; + } }