aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley, jmgao.
Herald added a subscriber: cfe-commits.
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 https://reviews.llvm.org/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.
Repository:
rC Clang
https://reviews.llvm.org/D51901
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDeclAttr.cpp
test/Sema/attr-capabilities.c
test/SemaCXX/warn-thread-safety-parsing.cpp
Index: test/SemaCXX/warn-thread-safety-parsing.cpp
===================================================================
--- test/SemaCXX/warn-thread-safety-parsing.cpp
+++ test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -572,11 +572,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 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 @@
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 can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'ElfFoo' isn't}}
};
class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -644,11 +645,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 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 @@
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 can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'SlfFoo' isn't}}
};
class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -717,14 +719,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)); // \
+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 @@
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 can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'EtfFoo' isn't}}
};
class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
@@ -760,7 +765,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 capability arguments can only be applied to non-static methods of a class}}
// illegal attribute arguments
@@ -795,9 +801,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 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 @@
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 can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'StfFoo' isn't}}
};
class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \
@@ -834,7 +843,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 capability arguments can only be applied to non-static methods of a class}}
// illegal attribute arguments
@@ -863,11 +873,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 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 @@
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 can only be applied in a class annotated with 'capability' or 'scoped_lockable' attribute, but 'UfFoo' isn't}}
};
class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \
@@ -1250,6 +1264,11 @@
}
};
+template <typename Mutex>
+class SCOPED_LOCKABLE SLTemplateClass {
+ ~SLTemplateClass() UNLOCK_FUNCTION();
+};
+
//-----------------------------------------------------
// Parsing of member variables and function parameters
@@ -1525,4 +1544,4 @@
Mutex mu_;
};
}
-#endif
\ No newline at end of file
+#endif
Index: test/Sema/attr-capabilities.c
===================================================================
--- test/Sema/attr-capabilities.c
+++ test/Sema/attr-capabilities.c
@@ -37,15 +37,25 @@
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))) {}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -475,6 +475,26 @@
return nullptr;
}
+template <typename AttrType>
+static bool checkRecordDeclForAttr(const RecordDecl *RD) {
+ // Check if the record itself has a capability.
+ if (RD->hasAttr<AttrType>())
+ 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<AttrType>();
+ },
+ BPaths))
+ return true;
+ }
+ return false;
+}
+
static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
const RecordType *RT = getRecordType(Ty);
@@ -490,21 +510,7 @@
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) {
@@ -562,8 +568,25 @@
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();
+ 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);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3008,6 +3008,14 @@
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 can only be applied in a class "
+ "annotated with 'capability' or 'scoped_lockable' attribute, but %1 isn't">,
+ 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">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
