r215671 - Thread Safety Analysis: fix to improve handling of references to guarded
DeLesley Hutchins
delesley at google.com
Thu Aug 14 12:17:06 PDT 2014
Author: delesley
Date: Thu Aug 14 14:17:06 2014
New Revision: 215671
URL: http://llvm.org/viewvc/llvm-project?rev=215671&view=rev
Log:
Thread Safety Analysis: fix to improve handling of references to guarded
data members and range based for loops.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=215671&r1=215670&r2=215671&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 14 14:17:06 2014
@@ -1215,7 +1215,7 @@ class BuildLockset : public StmtVisitor<
// helper functions
void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK,
Expr *MutexExp, ProtectedOperationKind POK,
- StringRef DiagKind);
+ StringRef DiagKind, SourceLocation Loc);
void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp,
StringRef DiagKind);
@@ -1247,7 +1247,7 @@ public:
void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
AccessKind AK, Expr *MutexExp,
ProtectedOperationKind POK,
- StringRef DiagKind) {
+ StringRef DiagKind, SourceLocation Loc) {
LockKind LK = getLockKindFromAccessKind(AK);
CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
@@ -1263,7 +1263,7 @@ void BuildLockset::warnIfMutexNotHeld(co
FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(
- DiagKind, D->getNameAsString(), (!Cp).toString(), Exp->getExprLoc());
+ DiagKind, D->getNameAsString(), (!Cp).toString(), Loc);
return;
}
@@ -1276,7 +1276,7 @@ void BuildLockset::warnIfMutexNotHeld(co
LDat = FSet.findLock(Analyzer->FactMan, Cp);
if (!LDat) {
Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
- LK_Shared, Exp->getExprLoc());
+ LK_Shared, Loc);
}
return;
}
@@ -1290,30 +1290,25 @@ void BuildLockset::warnIfMutexNotHeld(co
// Warn that there's no precise match.
std::string PartMatchStr = LDat->toString();
StringRef PartMatchName(PartMatchStr);
- Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
- Cp.toString(),
- LK, Exp->getExprLoc(),
- &PartMatchName);
+ Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+ LK, Loc, &PartMatchName);
} else {
// Warn that there's no match at all.
- Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
- Cp.toString(),
- LK, Exp->getExprLoc());
+ Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+ LK, Loc);
}
NoError = false;
}
// Make sure the mutex we found is the right kind.
if (NoError && LDat && !LDat->isAtLeast(LK)) {
- Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
- Cp.toString(),
- LK, Exp->getExprLoc());
+ Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(),
+ LK, Loc);
}
}
/// \brief Warn if the LSet contains the given lock.
void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
- Expr *MutexExp,
- StringRef DiagKind) {
+ Expr *MutexExp, StringRef DiagKind) {
CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp);
if (Cp.isInvalid()) {
warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind);
@@ -1337,6 +1332,23 @@ void BuildLockset::warnIfMutexHeld(const
void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK) {
Exp = Exp->IgnoreParenCasts();
+ SourceLocation Loc = Exp->getExprLoc();
+
+ if (Analyzer->Handler.issueBetaWarnings()) {
+ // Local variables of reference type cannot be re-assigned;
+ // map them to their initializer.
+ while (auto *DRE = dyn_cast<DeclRefExpr>(Exp)) {
+ auto *VD = dyn_cast<VarDecl>(DRE->getDecl()->getCanonicalDecl());
+ if (VD && VD->isLocalVarDecl() && VD->getType()->isReferenceType()) {
+ if (auto *E = VD->getInit()) {
+ Exp = E;
+ continue;
+ }
+ }
+ break;
+ }
+ }
+
if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Exp)) {
// For dereferences
if (UO->getOpcode() == clang::UO_Deref)
@@ -1362,14 +1374,15 @@ void BuildLockset::checkAccess(const Exp
if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan)) {
Analyzer->Handler.handleNoMutexHeld("mutex", D, POK_VarAccess, AK,
- Exp->getExprLoc());
+ Loc);
}
for (const auto *I : D->specific_attrs<GuardedByAttr>())
warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarAccess,
- ClassifyDiagnostic(I));
+ ClassifyDiagnostic(I), Loc);
}
+
/// \brief Checks pt_guarded_by and pt_guarded_var attributes.
void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) {
while (true) {
@@ -1400,7 +1413,7 @@ void BuildLockset::checkPtAccess(const E
for (auto const *I : D->specific_attrs<PtGuardedByAttr>())
warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarDereference,
- ClassifyDiagnostic(I));
+ ClassifyDiagnostic(I), Exp->getExprLoc());
}
/// \brief Process a function call, method call, constructor call,
@@ -1480,7 +1493,8 @@ void BuildLockset::handleCall(Expr *Exp,
RequiresCapabilityAttr *A = cast<RequiresCapabilityAttr>(At);
for (auto *Arg : A->args())
warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
- POK_FunctionCall, ClassifyDiagnostic(A));
+ POK_FunctionCall, ClassifyDiagnostic(A),
+ Exp->getExprLoc());
break;
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=215671&r1=215670&r2=215671&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Aug 14 14:17:06 2014
@@ -98,6 +98,7 @@ public:
};
+// For testing operator overloading
template <class K, class T>
class MyMap {
public:
@@ -105,6 +106,29 @@ public:
};
+// For testing handling of containers.
+template <class T>
+class MyContainer {
+public:
+ MyContainer();
+
+ typedef T* iterator;
+ typedef const T* const_iterator;
+
+ T* begin();
+ T* end();
+
+ const T* cbegin();
+ const T* cend();
+
+ T& operator[](int i);
+ const T& operator[](int i) const;
+
+private:
+ T* ptr_;
+};
+
+
Mutex sls_mu;
@@ -4645,3 +4669,62 @@ class Foo {
} // end namespace AssertSharedExclusive
+namespace RangeBasedForAndReferences {
+
+class Foo {
+ struct MyStruct {
+ int a;
+ };
+
+ Mutex mu;
+ int a GUARDED_BY(mu);
+ MyContainer<int> cntr GUARDED_BY(mu);
+ MyStruct s GUARDED_BY(mu);
+ int arr[10] GUARDED_BY(mu);
+
+ void nonref_test() {
+ int b = a; // expected-warning {{reading variable 'a' requires holding mutex 'mu'}}
+ b = 0; // no warning
+ }
+
+ void auto_test() {
+ auto b = a; // expected-warning {{reading variable 'a' requires holding mutex 'mu'}}
+ b = 0; // no warning
+ auto &c = a; // no warning
+ c = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}}
+ }
+
+ void ref_test() {
+ int &b = a;
+ int &c = b;
+ int &d = c;
+ b = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}}
+ c = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}}
+ d = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}}
+
+ MyStruct &rs = s;
+ rs.a = 0; // expected-warning {{writing variable 's' requires holding mutex 'mu' exclusively}}
+
+ int (&rarr)[10] = arr;
+ rarr[2] = 0; // expected-warning {{writing variable 'arr' requires holding mutex 'mu' exclusively}}
+ }
+
+ void ptr_test() {
+ int *b = &a;
+ *b = 0; // no expected warning yet
+ }
+
+ void for_test() {
+ int total = 0;
+ for (int i : cntr) { // expected-warning2 {{reading variable 'cntr' requires holding mutex 'mu'}}
+ total += i;
+ }
+ }
+};
+
+
+} // end namespace RangeBasedForAndReferences
+
+
+
+
More information about the cfe-commits
mailing list