jmgao updated this revision to Diff 110054.
jmgao added a comment.
Reword warnings.
https://reviews.llvm.org/D36237
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Analysis/ThreadSafety.cpp
lib/Sema/SemaDeclAttr.cpp
test/Sema/attr-capabilities.c
test/SemaCXX/warn-thread-safety-analysis.cpp
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
@@ -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 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 @@
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 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 @@
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,29 +717,32 @@
// 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; // \
// expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
return x;
};
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 can only be applied to a method of a class}}
// 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 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 @@
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 can only be applied to a method of a class}}
// 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 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 @@
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 @@
Mutex mu_;
};
}
-#endif
\ No newline at end of file
+#endif
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/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 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))) {}
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 <typename T> static bool checkRecordTypeForAttr(Sema &S, QualType Ty) {
const RecordType *RT = getRecordType(Ty);
if (!RT)
@@ -497,41 +497,49 @@
// 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.
if (CXXRecordDecl *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>();
+ 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;
TypedefNameDecl *TN = TD->getDecl();
if (!TN)
return false;
- return TN->hasAttr<CapabilityAttr>();
+ return TN->hasAttr<T>();
+}
+
+template <typename T> static bool typeHasAttr(Sema &S, QualType Ty) {
+ if (checkTypedefTypeForAttr<T>(Ty))
+ return true;
+
+ if (checkRecordTypeForAttr<T>(S, Ty))
+ return true;
+
+ return false;
}
static bool typeHasCapability(Sema &S, QualType Ty) {
- if (checkTypedefTypeForCapability(Ty))
- return true;
+ return typeHasAttr<CapabilityAttr>(S, Ty);
+}
- if (checkRecordTypeForCapability(S, Ty))
- return true;
-
- return false;
+static bool typeHasScopedLockable(Sema &S, QualType Ty) {
+ return typeHasAttr<ScopedLockableAttr>(S, Ty);
}
static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
@@ -570,6 +578,8 @@
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 @@
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_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<CXXMethodDecl>(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<Expr*, 1> 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: 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<LockableFactEntry>(
- AssertLock, LK_Shared, Loc, false, true),
+ Analyzer->addLock(FSet,
+ llvm::make_unique<LockableFactEntry>(
+ AssertLock, LK_Shared, Loc, false, true),
+ ClassifyDiagnostic(A));
+ break;
+ }
+
+ case attr::AssertCapability: {
+ AssertCapabilityAttr *A = cast<AssertCapabilityAttr>(At);
+ CapExprSet AssertLocks;
+ Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+ for (const auto &AssertLock : AssertLocks)
+ Analyzer->addLock(FSet,
+ llvm::make_unique<LockableFactEntry>(
+ AssertLock,
+ A->isShared() ? LK_Shared : LK_Exclusive, Loc,
+ false, true),
ClassifyDiagnostic(A));
break;
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2932,6 +2932,16 @@
"%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;
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">]>];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
