[cfe-commits] r163537 - in /cfe/trunk: include/clang/Analysis/Analyses/ThreadSafety.h include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Analysis/ThreadSafety.cpp lib/Sema/AnalysisBasedWarnings.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Mon Sep 10 12:58:23 PDT 2012
Author: delesley
Date: Mon Sep 10 14:58:23 2012
New Revision: 163537
URL: http://llvm.org/viewvc/llvm-project?rev=163537&view=rev
Log:
Thread-safety analysis: differentiate between two forms of analysis; a precise
analysis that may give false positives because it is confused by aliasing, and
a less precise analysis that has fewer false positives, but may have false
negatives. The more precise warnings are enabled by -Wthread-safety-precise.
An additional note clarify the warnings in the precise case.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Mon Sep 10 14:58:23 2012
@@ -132,7 +132,8 @@
/// \param Loc -- The location of the protected operation.
virtual void handleMutexNotHeld(const NamedDecl *D,
ProtectedOperationKind POK, Name LockName,
- LockKind LK, SourceLocation Loc) {}
+ LockKind LK, SourceLocation Loc,
+ Name *PossibleMatch=0) {}
/// Warn when a function is called while an excluded mutex is locked. For
/// example, the mutex may be locked inside the function.
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Sep 10 14:58:23 2012
@@ -386,9 +386,12 @@
// Thread Safety warnings
def ThreadSafetyAttributes : DiagGroup<"thread-safety-attributes">;
-def ThreadSafetyAnalysis : DiagGroup<"thread-safety-analysis">;
-def ThreadSafety : DiagGroup<"thread-safety",
- [ThreadSafetyAttributes, ThreadSafetyAnalysis]>;
+def ThreadSafetyAnalysis : DiagGroup<"thread-safety-analysis">;
+def ThreadSafetyPrecise : DiagGroup<"thread-safety-precise">;
+def ThreadSafety : DiagGroup<"thread-safety",
+ [ThreadSafetyAttributes,
+ ThreadSafetyAnalysis,
+ ThreadSafetyPrecise]>;
// Note that putting warnings in -Wall will not disable them by default. If a
// warning should be active _only_ when -Wall is passed in, mark it as
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep 10 14:58:23 2012
@@ -1866,14 +1866,6 @@
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def note_lock_exclusive_and_shared : Note<
"the other lock of mutex '%0' is here">;
-def warn_variable_requires_lock : Warning<
- "%select{reading|writing}2 variable '%0' requires locking "
- "%select{'%1'|'%1' exclusively}2">,
- InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
-def warn_var_deref_requires_lock : Warning<
- "%select{reading|writing}2 the value pointed to by '%0' requires locking "
- "%select{'%1'|'%1' exclusively}2">,
- InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_variable_requires_any_lock : Warning<
"%select{reading|writing}1 variable '%0' requires locking "
"%select{any mutex|any mutex exclusively}1">,
@@ -1882,9 +1874,6 @@
"%select{reading|writing}1 the value pointed to by '%0' requires locking "
"%select{any mutex|any mutex exclusively}1">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
-def warn_fun_requires_lock : Warning<
- "calling function '%0' requires %select{shared|exclusive}2 lock on '%1'">,
- InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_fun_excludes_mutex : Warning<
"cannot call function '%0' while mutex '%1' is locked">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
@@ -1892,6 +1881,32 @@
"cannot resolve lock expression">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+// Imprecise thread safety warnings
+def warn_variable_requires_lock : Warning<
+ "%select{reading|writing}2 variable '%0' requires locking "
+ "%select{'%1'|'%1' exclusively}2">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_var_deref_requires_lock : Warning<
+ "%select{reading|writing}2 the value pointed to by '%0' requires locking "
+ "%select{'%1'|'%1' exclusively}2">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+def warn_fun_requires_lock : Warning<
+ "calling function '%0' requires %select{shared|exclusive}2 lock on '%1'">,
+ InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
+
+// Precise thread safety warnings
+def warn_variable_requires_lock_precise : Warning<
+ "%select{reading|writing}2 variable '%0' requires locking "
+ "%select{'%1'|'%1' exclusively}2">,
+ InGroup<ThreadSafetyPrecise>, DefaultIgnore;
+def warn_var_deref_requires_lock_precise : Warning<
+ "%select{reading|writing}2 the value pointed to by '%0' requires locking "
+ "%select{'%1'|'%1' exclusively}2">,
+ InGroup<ThreadSafetyPrecise>, DefaultIgnore;
+def warn_fun_requires_lock_precise : Warning<
+ "calling function '%0' requires %select{shared|exclusive}2 lock on '%1'">,
+ InGroup<ThreadSafetyPrecise>, DefaultIgnore;
+def note_found_mutex_near_match : Note<"found near match '%0'">;
def warn_impcast_vector_scalar : Warning<
"implicit conversion turns vector to scalar: %0 to %1">,
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Sep 10 14:58:23 2012
@@ -575,6 +575,15 @@
return false;
}
+ // A partial match between a.mu and b.mu returns true a and b have the same
+ // type (and thus mu refers to the same mutex declaration), regardless of
+ // whether a and b are different objects or not.
+ bool partiallyMatches(const SExpr &Other) const {
+ if (NodeVec[0].kind() == EOP_Dot)
+ return NodeVec[0].matches(Other.NodeVec[0]);
+ return false;
+ }
+
/// \brief Pretty print a lock expression for use in error messages.
std::string toString(unsigned i = 0) const {
assert(isValid());
@@ -820,7 +829,7 @@
return false;
}
- LockData* findLock(FactManager& FM, const SExpr& M) const {
+ LockData* findLock(FactManager &FM, const SExpr &M) const {
for (const_iterator I = begin(), E = end(); I != E; ++I) {
const SExpr &Exp = FM[*I].MutID;
if (Exp.matches(M))
@@ -829,7 +838,7 @@
return 0;
}
- LockData* findLockUniv(FactManager& FM, const SExpr& M) const {
+ LockData* findLockUniv(FactManager &FM, const SExpr &M) const {
for (const_iterator I = begin(), E = end(); I != E; ++I) {
const SExpr &Exp = FM[*I].MutID;
if (Exp.matches(M) || Exp.isUniversal())
@@ -837,6 +846,14 @@
}
return 0;
}
+
+ FactEntry* findPartialMatch(FactManager &FM, const SExpr &M) const {
+ for (const_iterator I=begin(), E=end(); I != E; ++I) {
+ const SExpr& Exp = FM[*I].MutID;
+ if (Exp.partiallyMatches(M)) return &FM[*I];
+ }
+ return 0;
+ }
};
@@ -1742,7 +1759,26 @@
}
LockData* LDat = FSet.findLockUniv(Analyzer->FactMan, Mutex);
- if (!LDat || !LDat->isAtLeast(LK))
+ bool NoError = true;
+ if (!LDat) {
+ // No exact match found. Look for a partial match.
+ FactEntry* FEntry = FSet.findPartialMatch(Analyzer->FactMan, Mutex);
+ if (FEntry) {
+ // Warn that there's no precise match.
+ LDat = &FEntry->LDat;
+ std::string PartMatchStr = FEntry->MutID.toString();
+ StringRef PartMatchName(PartMatchStr);
+ Analyzer->Handler.handleMutexNotHeld(D, POK, Mutex.toString(), LK,
+ Exp->getExprLoc(), &PartMatchName);
+ } else {
+ // Warn that there's no match at all.
+ Analyzer->Handler.handleMutexNotHeld(D, POK, Mutex.toString(), LK,
+ Exp->getExprLoc());
+ }
+ NoError = false;
+ }
+ // Make sure the mutex we found is the right kind.
+ if (NoError && LDat && !LDat->isAtLeast(LK))
Analyzer->Handler.handleMutexNotHeld(D, POK, Mutex.toString(), LK,
Exp->getExprLoc());
}
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Sep 10 14:58:23 2012
@@ -1089,22 +1089,42 @@
}
void handleMutexNotHeld(const NamedDecl *D, ProtectedOperationKind POK,
- Name LockName, LockKind LK, SourceLocation Loc) {
+ Name LockName, LockKind LK, SourceLocation Loc,
+ Name *PossibleMatch) {
unsigned DiagID = 0;
- switch (POK) {
- case POK_VarAccess:
- DiagID = diag::warn_variable_requires_lock;
- break;
- case POK_VarDereference:
- DiagID = diag::warn_var_deref_requires_lock;
- break;
- case POK_FunctionCall:
- DiagID = diag::warn_fun_requires_lock;
- break;
+ if (PossibleMatch) {
+ switch (POK) {
+ case POK_VarAccess:
+ DiagID = diag::warn_variable_requires_lock_precise;
+ break;
+ case POK_VarDereference:
+ DiagID = diag::warn_var_deref_requires_lock_precise;
+ break;
+ case POK_FunctionCall:
+ DiagID = diag::warn_fun_requires_lock_precise;
+ break;
+ }
+ PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID)
+ << D->getName() << LockName << LK);
+ PartialDiagnosticAt Note(Loc, S.PDiag(diag::note_found_mutex_near_match)
+ << *PossibleMatch);
+ Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note)));
+ } else {
+ switch (POK) {
+ case POK_VarAccess:
+ DiagID = diag::warn_variable_requires_lock;
+ break;
+ case POK_VarDereference:
+ DiagID = diag::warn_var_deref_requires_lock;
+ break;
+ case POK_FunctionCall:
+ DiagID = diag::warn_fun_requires_lock;
+ break;
+ }
+ PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID)
+ << D->getName() << LockName << LK);
+ Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
}
- PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID)
- << D->getName() << LockName << LK);
- Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
}
void handleFunExcludesLock(Name FunName, Name LockName, SourceLocation Loc) {
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=163537&r1=163536&r2=163537&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Sep 10 14:58:23 2012
@@ -533,7 +533,8 @@
LateFoo fooB;
fooA.mu.Lock();
fooB.a = 5; // \
- // expected-warning{{writing variable 'a' requires locking 'fooB.mu' exclusively}}
+ // expected-warning{{writing variable 'a' requires locking 'fooB.mu' exclusively}} \
+ // expected-note{{found near match 'fooA.mu'}}
fooA.mu.Unlock();
}
@@ -553,7 +554,8 @@
LateBar BarA;
BarA.FooPointer->mu.Lock();
BarA.Foo.a = 2; // \
- // expected-warning{{writing variable 'a' requires locking 'BarA.Foo.mu' exclusively}}
+ // expected-warning{{writing variable 'a' requires locking 'BarA.Foo.mu' exclusively}} \
+ // expected-note{{found near match 'BarA.FooPointer->mu'}}
BarA.FooPointer->mu.Unlock();
}
@@ -561,7 +563,8 @@
LateBar BarA;
BarA.Foo.mu.Lock();
BarA.FooPointer->a = 2; // \
- // expected-warning{{writing variable 'a' requires locking 'BarA.FooPointer->mu' exclusively}}
+ // expected-warning{{writing variable 'a' requires locking 'BarA.FooPointer->mu' exclusively}} \
+ // expected-note{{found near match 'BarA.Foo.mu'}}
BarA.Foo.mu.Unlock();
}
@@ -569,7 +572,8 @@
LateBar BarA;
BarA.Foo.mu.Lock();
BarA.Foo2.a = 2; // \
- // expected-warning{{writing variable 'a' requires locking 'BarA.Foo2.mu' exclusively}}
+ // expected-warning{{writing variable 'a' requires locking 'BarA.Foo2.mu' exclusively}} \
+ // expected-note{{found near match 'BarA.Foo.mu'}}
BarA.Foo.mu.Unlock();
}
@@ -1237,7 +1241,9 @@
{
b1->MyLock();
b1->a_ = 5;
- b2->a_ = 3; // expected-warning {{writing variable 'a_' requires locking 'b2->mu1_' exclusively}}
+ b2->a_ = 3; // \
+ // expected-warning {{writing variable 'a_' requires locking 'b2->mu1_' exclusively}} \
+ // expected-note {{found near match 'b1->mu1_'}}
b2->MyLock();
b2->MyUnlock();
b1->MyUnlock();
@@ -1267,11 +1273,13 @@
int x;
b3->mu1_.Lock();
res = b1.a_ + b3->b_; // expected-warning {{reading variable 'a_' requires locking 'b1.mu1_'}} \
- // expected-warning {{writing variable 'res' requires locking 'mu' exclusively}}
+ // expected-warning {{writing variable 'res' requires locking 'mu' exclusively}} \
+ // expected-note {{found near match 'b3->mu1_'}}
*p = i; // expected-warning {{reading variable 'p' requires locking 'mu'}} \
// expected-warning {{writing the value pointed to by 'p' requires locking 'mu' exclusively}}
b1.a_ = res + b3->b_; // expected-warning {{reading variable 'res' requires locking 'mu'}} \
- // expected-warning {{writing variable 'a_' requires locking 'b1.mu1_' exclusively}}
+ // expected-warning {{writing variable 'a_' requires locking 'b1.mu1_' exclusively}} \
+ // expected-note {{found near match 'b3->mu1_'}}
b3->b_ = *b1.q; // expected-warning {{reading the value pointed to by 'q' requires locking 'mu'}}
b3->mu1_.Unlock();
b1.b_ = res; // expected-warning {{reading variable 'res' requires locking 'mu'}}
@@ -1296,8 +1304,12 @@
child->Func(new_foo); // There shouldn't be any warning here as the
// acquired lock is not in child.
- child->bar(7); // expected-warning {{calling function 'bar' requires exclusive lock on 'child->lock_'}}
- child->a_ = 5; // expected-warning {{writing variable 'a_' requires locking 'child->lock_' exclusively}}
+ child->bar(7); // \
+ // expected-warning {{calling function 'bar' requires exclusive lock on 'child->lock_'}} \
+ // expected-note {{found near match 'lock_'}}
+ child->a_ = 5; // \
+ // expected-warning {{writing variable 'a_' requires locking 'child->lock_' exclusively}} \
+ // expected-note {{found near match 'lock_'}}
lock_.Unlock();
}
@@ -1495,7 +1507,8 @@
DataLocker dlr;
dlr.lockData(d1);
foo(d2); // \
- // expected-warning {{calling function 'foo' requires exclusive lock on 'd2->mu'}}
+ // expected-warning {{calling function 'foo' requires exclusive lock on 'd2->mu'}} \
+ // expected-note {{found near match 'd1->mu'}}
dlr.unlockData(d1);
}
};
@@ -1833,7 +1846,8 @@
f1.mu_.Unlock();
bt.barTD(&f1); // \
- // expected-warning {{calling function 'barTD' requires exclusive lock on 'f1.mu_'}}
+ // expected-warning {{calling function 'barTD' requires exclusive lock on 'f1.mu_'}} \
+ // expected-note {{found near match 'bt.fooBase.mu_'}}
bt.fooBase.mu_.Unlock();
bt.fooBaseT.mu_.Unlock();
@@ -2239,27 +2253,32 @@
bar.getFoo().mu_.Lock();
bar.getFooey().a = 0; // \
- // expected-warning {{writing variable 'a' requires locking 'bar.getFooey().mu_' exclusively}}
+ // expected-warning {{writing variable 'a' requires locking 'bar.getFooey().mu_' exclusively}} \
+ // expected-note {{found near match 'bar.getFoo().mu_'}}
bar.getFoo().mu_.Unlock();
bar.getFoo2(a).mu_.Lock();
bar.getFoo2(b).a = 0; // \
- // expected-warning {{writing variable 'a' requires locking 'bar.getFoo2(b).mu_' exclusively}}
+ // expected-warning {{writing variable 'a' requires locking 'bar.getFoo2(b).mu_' exclusively}} \
+ // expected-note {{found near match 'bar.getFoo2(a).mu_'}}
bar.getFoo2(a).mu_.Unlock();
bar.getFoo3(a, b).mu_.Lock();
bar.getFoo3(a, c).a = 0; // \
- // expected-warning {{writing variable 'a' requires locking 'bar.getFoo3(a,c).mu_' exclusively}}
+ // expected-warning {{writing variable 'a' requires locking 'bar.getFoo3(a,c).mu_' exclusively}} \
+ // expected-note {{'bar.getFoo3(a,b).mu_'}}
bar.getFoo3(a, b).mu_.Unlock();
getBarFoo(bar, a).mu_.Lock();
getBarFoo(bar, b).a = 0; // \
- // expected-warning {{writing variable 'a' requires locking 'getBarFoo(bar,b).mu_' exclusively}}
+ // expected-warning {{writing variable 'a' requires locking 'getBarFoo(bar,b).mu_' exclusively}} \
+ // expected-note {{'getBarFoo(bar,a).mu_'}}
getBarFoo(bar, a).mu_.Unlock();
(a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock();
(a > 0 ? fooArray[b] : fooArray[c]).a = 0; // \
- // expected-warning {{writing variable 'a' requires locking '((a#_)#_#fooArray[b]).mu_' exclusively}}
+ // expected-warning {{writing variable 'a' requires locking '((a#_)#_#fooArray[b]).mu_' exclusively}} \
+ // expected-note {{'((a#_)#_#fooArray[_]).mu_'}}
(a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock();
}
@@ -2318,7 +2337,9 @@
f1->a = 0;
f1->foo();
- f1->foo2(f2); // expected-warning {{calling function 'foo2' requires exclusive lock on 'f2->mu_'}}
+ f1->foo2(f2); // \
+ // expected-warning {{calling function 'foo2' requires exclusive lock on 'f2->mu_'}} \
+ // expected-note {{found near match 'f1->mu_'}}
Foo::getMu(f2)->Lock();
f1->foo2(f2);
@@ -2358,7 +2379,9 @@
b1->b = 0;
b1->bar();
- b1->bar2(b2); // expected-warning {{calling function 'bar2' requires exclusive lock on 'b2->mu_'}}
+ b1->bar2(b2); // \
+ // expected-warning {{calling function 'bar2' requires exclusive lock on 'b2->mu_'}} \
+ // // expected-note {{found near match 'b1->mu_'}}
b2->getMu()->Lock();
b1->bar2(b2);
More information about the cfe-commits
mailing list