aaronpuchert updated this revision to Diff 168505.
aaronpuchert added a comment.
Rebase on top of https://reviews.llvm.org/D52443. We also check the move
constructor argument for write access, as suggested in a review.
This isn't intended to be merged (yet?), it should be seen as an RFC.
Repository:
rC Clang
https://reviews.llvm.org/D52395
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Analysis/ThreadSafety.cpp
test/SemaCXX/warn-thread-safety-analysis.cpp
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -5058,27 +5058,27 @@
void test1() {
copy(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
- write1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- write2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ write1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ write2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
read1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
read2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- destroy(mymove(foo)); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ destroy(mymove(foo)); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
copyVariadic(foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
readVariadic(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- writeVariadic(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ writeVariadic(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
copyVariadicC(1, foo); // expected-warning {{reading variable 'foo' requires holding mutex 'mu'}}
FooRead reader(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- FooWrite writer(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ FooWrite writer(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
- mwrite1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- mwrite2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ mwrite1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ mwrite2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
mread1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
mread2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- smwrite1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
- smwrite2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
+ smwrite1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ smwrite2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
smread1(foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
smread2(10, foo); // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
@@ -5094,12 +5094,13 @@
(*this) << foo; // expected-warning {{passing variable 'foo' by reference requires holding mutex 'mu'}}
copy(*foop); // expected-warning {{reading the value pointed to by 'foop' requires holding mutex 'mu'}}
- write1(*foop); // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
- write2(10, *foop); // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+ write1(*foop); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+ write2(10, *foop); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
read1(*foop); // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
read2(10, *foop); // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
- destroy(mymove(*foop)); // expected-warning {{passing the value that 'foop' points to by reference requires holding mutex 'mu'}}
+ destroy(mymove(*foop)); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+ // TODO -- sometimes this should warn about writing.
copy(*foosp); // expected-warning {{reading the value pointed to by 'foosp' requires holding mutex 'mu'}}
write1(*foosp); // expected-warning {{reading the value pointed to by 'foosp' requires holding mutex 'mu'}}
write2(10, *foosp); // expected-warning {{reading the value pointed to by 'foosp' requires holding mutex 'mu'}}
@@ -5115,6 +5116,66 @@
read2(10, *foosp.get());
destroy(mymove(*foosp.get()));
}
+
+ void test2() {
+ mu.ReaderLock();
+
+ copy(foo);
+ write1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ write2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ read1(foo);
+ read2(10, foo);
+ destroy(mymove(foo)); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+
+ copyVariadic(foo);
+ readVariadic(foo);
+ writeVariadic(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ copyVariadicC(1, foo);
+
+ FooRead reader(foo);
+ FooWrite writer(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+
+ mwrite1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ mwrite2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ mread1(foo);
+ mread2(10, foo);
+
+ smwrite1(foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ smwrite2(10, foo); // expected-warning {{passing variable 'foo' by non-const reference requires holding mutex 'mu' exclusively}}
+ smread1(foo);
+ smread2(10, foo);
+
+ foo + foo2;
+ foo / foo2;
+ foo * foo2;
+ foo[foo2];
+ foo();
+ (*this) << foo;
+
+ copy(*foop);
+ write1(*foop); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+ write2(10, *foop); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+ read1(*foop);
+ read2(10, *foop);
+ destroy(mymove(*foop)); // expected-warning {{passing the value that 'foop' points to by non-const reference requires holding mutex 'mu' exclusively}}
+
+ // TODO -- these require better smart pointer handling.
+ copy(*foosp);
+ write1(*foosp);
+ write2(10, *foosp);
+ read1(*foosp);
+ read2(10, *foosp);
+ destroy(mymove(*foosp));
+
+ copy(*foosp.get());
+ write1(*foosp.get());
+ write2(10, *foosp.get());
+ read1(*foosp.get());
+ read2(10, *foosp.get());
+ destroy(mymove(*foosp.get()));
+
+ mu.ReaderUnlock();
+ }
};
Index: lib/Analysis/ThreadSafety.cpp
===================================================================
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -1966,9 +1966,13 @@
// There can be default arguments, so we stop when one iterator is at end().
for (auto Arg = ArgBegin; Param != Params.end() && Arg != ArgEnd;
++Param, ++Arg) {
- QualType Qt = (*Param)->getType();
- if (Qt->isReferenceType())
- checkAccess(*Arg, AK_Read, POK_PassByRef);
+ QualType Ct = (*Param)->getType().getCanonicalType();
+ if (const auto *RT = dyn_cast<const ReferenceType>(&*Ct)) {
+ if (RT->getPointeeType().isConstQualified())
+ checkAccess(*Arg, AK_Read, POK_PassByRef);
+ else
+ checkAccess(*Arg, AK_Written, POK_PassByRef);
+ }
}
}
@@ -2037,8 +2041,9 @@
void BuildLockset::VisitCXXConstructExpr(const CXXConstructExpr *Exp) {
const CXXConstructorDecl *D = Exp->getConstructor();
if (D && D->isCopyConstructor()) {
- const Expr* Source = Exp->getArg(0);
- checkAccess(Source, AK_Read);
+ checkAccess(Exp->getArg(0), AK_Read);
+ } else if (D && D->isMoveConstructor()) {
+ checkAccess(Exp->getArg(0), AK_Written);
} else {
examineArguments(D, Exp->arg_begin(), Exp->arg_end());
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3100,12 +3100,12 @@
// Thread safety warnings on pass by reference
def warn_guarded_pass_by_reference : Warning<
- "passing variable %1 by reference requires holding %0 "
- "%select{'%2'|'%2' exclusively}3">,
+ "passing variable %1 by %select{reference|non-const reference}3 requires "
+ "holding %0 %select{'%2'|'%2' exclusively}3">,
InGroup<ThreadSafetyReference>, DefaultIgnore;
def warn_pt_guarded_pass_by_reference : Warning<
- "passing the value that %1 points to by reference requires holding %0 "
- "%select{'%2'|'%2' exclusively}3">,
+ "passing the value that %1 points to by %select{reference|non-const "
+ "reference}3 requires holding %0 %select{'%2'|'%2' exclusively}3">,
InGroup<ThreadSafetyReference>, DefaultIgnore;
// Imprecise thread safety warnings
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
