r342605 - Thread Safety Analysis: warnings for attributes without arguments
Aaron Puchert via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 19 17:39:27 PDT 2018
Author: aaronpuchert
Date: Wed Sep 19 17:39:27 2018
New Revision: 342605
URL: http://llvm.org/viewvc/llvm-project?rev=342605&view=rev
Log:
Thread Safety Analysis: warnings for attributes without arguments
Summary:
When thread safety annotations are used without capability arguments,
they are assumed to apply to `this` instead. So we warn when either
`this` doesn't exist, or the class is not a capability type.
This is based on earlier work by Josh Gao that was committed in r310403,
but reverted in r310698 because it didn't properly work in template
classes. See also D36237.
The solution is not to go via the QualType of `this`, which is then a
template type, hence the attributes are not known because it could be
specialized. Instead we look directly at the class in which we are
contained.
Additionally I grouped two of the warnings together. There are two
issues here: the existence of `this`, which requires us to be a
non-static member function, and the appropriate annotation on the class
we are contained in. So we don't distinguish between not being in a
class and being static, because in both cases we don't have `this`.
Fixes PR38399.
Reviewers: aaron.ballman, delesley, jmgao, rtrieu
Reviewed By: delesley
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D51901
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=342605&r1=342604&r2=342605&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 19 17:39:27 2018
@@ -3008,6 +3008,14 @@ def warn_invalid_capability_name : Warni
def warn_thread_attribute_ignored : Warning<
"ignoring %0 attribute because its argument is invalid">,
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
+def warn_thread_attribute_not_on_non_static_member : Warning<
+ "%0 attribute without capability arguments can only be applied to non-static "
+ "methods of a class">,
+ InGroup<ThreadSafetyAttributes>, DefaultIgnore;
+def warn_thread_attribute_not_on_capability_member : Warning<
+ "%0 attribute without capability arguments refers to 'this', but %1 isn't "
+ "annotated with 'capability' or 'scoped_lockable' attribute">,
+ InGroup<ThreadSafetyAttributes>, DefaultIgnore;
def warn_thread_attribute_argument_not_lockable : Warning<
"%0 attribute requires arguments whose type is annotated "
"with 'capability' attribute; type here is %1">,
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=342605&r1=342604&r2=342605&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Sep 19 17:39:27 2018
@@ -476,6 +476,29 @@ static const RecordType *getRecordType(Q
return nullptr;
}
+template <typename AttrType>
+static bool checkRecordDeclForAttr(const RecordDecl *RD) {
+ // Check if the record itself has the attribute.
+ if (RD->hasAttr<AttrType>())
+ return true;
+
+ // Else check if any base classes have the attribute.
+ if (const auto *CRD = dyn_cast<CXXRecordDecl>(RD)) {
+ CXXBasePaths BPaths(false, false);
+ if (CRD->lookupInBases(
+ [](const CXXBaseSpecifier *BS, CXXBasePath &) {
+ const auto &Ty = *BS->getType();
+ // If it's type-dependent, we assume it could have the attribute.
+ if (Ty.isDependentType())
+ return true;
+ return Ty.getAs<RecordType>()->getDecl()->hasAttr<AttrType>();
+ },
+ BPaths, true))
+ return true;
+ }
+ return false;
+}
+
static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
const RecordType *RT = getRecordType(Ty);
@@ -491,21 +514,7 @@ static bool checkRecordTypeForCapability
if (threadSafetyCheckIsSmartPointer(S, RT))
return true;
- // Check if the record itself has a capability.
- RecordDecl *RD = RT->getDecl();
- if (RD->hasAttr<CapabilityAttr>())
- return true;
-
- // Else check if any base classes have a capability.
- if (const auto *CRD = dyn_cast<CXXRecordDecl>(RD)) {
- CXXBasePaths BPaths(false, false);
- if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
- const auto *Type = BS->getType()->getAs<RecordType>();
- return Type->getDecl()->hasAttr<CapabilityAttr>();
- }, BPaths))
- return true;
- }
- return false;
+ return checkRecordDeclForAttr<CapabilityAttr>(RT->getDecl());
}
static bool checkTypedefTypeForCapability(QualType Ty) {
@@ -563,8 +572,27 @@ static bool isCapabilityExpr(Sema &S, co
static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
const ParsedAttr &AL,
SmallVectorImpl<Expr *> &Args,
- int Sidx = 0,
+ unsigned Sidx = 0,
bool ParamIdxOk = false) {
+ if (Sidx == AL.getNumArgs()) {
+ // If we don't have any capability arguments, the attribute implicitly
+ // refers to 'this'. So we need to make sure that 'this' exists, i.e. we're
+ // a non-static method, and that the class is a (scoped) capability.
+ const auto *MD = dyn_cast<const CXXMethodDecl>(D);
+ if (MD && !MD->isStatic()) {
+ const CXXRecordDecl *RD = MD->getParent();
+ // FIXME -- need to check this again on template instantiation
+ if (!checkRecordDeclForAttr<CapabilityAttr>(RD) &&
+ !checkRecordDeclForAttr<ScopedLockableAttr>(RD))
+ S.Diag(AL.getLoc(),
+ diag::warn_thread_attribute_not_on_capability_member)
+ << AL << MD->getParent();
+ } else {
+ S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
+ << AL;
+ }
+ }
+
for (unsigned Idx = Sidx; Idx < AL.getNumArgs(); ++Idx) {
Expr *ArgExp = AL.getArgAsExpr(Idx);
Modified: cfe/trunk/test/Sema/attr-capabilities.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-capabilities.c?rev=342605&r1=342604&r2=342605&view=diff
==============================================================================
--- cfe/trunk/test/Sema/attr-capabilities.c (original)
+++ cfe/trunk/test/Sema/attr-capabilities.c Wed Sep 19 17:39:27 2018
@@ -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 capability arguments can only be applied to non-static methods of a class}}
+void Func10(void) __attribute__((assert_shared_capability())) {} // expected-warning {{'assert_shared_capability' attribute without capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
+void Func14(void) __attribute__((acquire_shared_capability())) {} // expected-warning {{'acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
+void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+
+void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods 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=342605&r1=342604&r2=342605&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp Wed Sep 19 17:39:27 2018
@@ -572,11 +572,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 capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
int elf_testfn(int y) {
int x EXCLUSIVE_LOCK_FUNCTION() = y; // \
@@ -591,7 +591,8 @@ 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 without capability arguments refers to 'this', but 'ElfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
};
class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -644,11 +645,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 capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
int slf_testfn(int y) {
int x SHARED_LOCK_FUNCTION() = y; // \
@@ -666,7 +667,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 without capability arguments refers to 'this', but 'SlfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
};
class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -717,14 +719,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)); // \
+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 capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
int etf_testfn(int y) {
int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \
@@ -739,7 +743,8 @@ 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 without capability arguments refers to 'this', but 'EtfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
};
class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
@@ -760,7 +765,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 capability arguments can only be applied to non-static methods of a class}}
// illegal attribute arguments
@@ -795,9 +801,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 capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
int stf_testfn(int y) {
int x SHARED_TRYLOCK_FUNCTION(1) = y; // \
@@ -816,7 +824,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 without capability arguments refers to 'this', but 'StfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
};
class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \
@@ -834,7 +843,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 capability arguments can only be applied to non-static methods of a class}}
// illegal attribute arguments
@@ -863,11 +873,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 capability arguments can only be applied to non-static methods 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 capability arguments can only be applied to non-static methods of a class}}
int uf_testfn(int y) {
int x UNLOCK_FUNCTION() = y; // \
@@ -882,7 +895,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 without capability arguments refers to 'this', but 'UfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
};
class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \
@@ -1250,6 +1264,36 @@ struct Foomgoper {
}
};
+template <typename Mutex>
+struct SCOPED_LOCKABLE SLTemplateClass {
+ ~SLTemplateClass() UNLOCK_FUNCTION();
+};
+
+template <typename Mutex>
+struct NonSLTemplateClass {
+ ~NonSLTemplateClass() UNLOCK_FUNCTION(); // \
+ // expected-warning{{'unlock_function' attribute without capability arguments refers to 'this', but 'NonSLTemplateClass' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
+};
+
+template <>
+struct SLTemplateClass<int> {};
+
+template <typename Mutex>
+struct SLTemplateDerived : public SLTemplateClass<Mutex> {
+ ~SLTemplateDerived() UNLOCK_FUNCTION();
+};
+
+// FIXME: warn on template instantiation.
+template struct SLTemplateDerived<int>;
+
+struct SLDerived1 : public SLTemplateClass<double> {
+ ~SLDerived1() UNLOCK_FUNCTION();
+};
+
+struct SLDerived2 : public SLTemplateClass<int> {
+ ~SLDerived2() UNLOCK_FUNCTION(); // \
+ // expected-warning{{'unlock_function' attribute without capability arguments refers to 'this', but 'SLDerived2' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
+};
//-----------------------------------------------------
// Parsing of member variables and function parameters
@@ -1525,4 +1569,4 @@ namespace CRASH_POST_R301735 {
Mutex mu_;
};
}
-#endif
\ No newline at end of file
+#endif
More information about the cfe-commits
mailing list