[cfe-commits] r139370 - in /cfe/trunk: include/clang/Analysis/Analyses/ThreadSafety.h include/clang/Basic/DiagnosticSemaKinds.td lib/Analysis/ThreadSafety.cpp lib/Sema/AnalysisBasedWarnings.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
Caitlin Sadowski
supertri at google.com
Fri Sep 9 09:21:55 PDT 2011
Author: supertri
Date: Fri Sep 9 11:21:55 2011
New Revision: 139370
URL: http://llvm.org/viewvc/llvm-project?rev=139370&view=rev
Log:
Thread safety: This patch deals with previously unhandled cases when building lock expressions. We now resolve this expressions, avoid crashing when encountering cast expressions, and have a diagnostic for unresolved lock expressions
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
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=139370&r1=139369&r2=139370&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Fri Sep 9 11:21:55 2011
@@ -47,6 +47,7 @@
typedef llvm::StringRef Name;
ThreadSafetyHandler() {}
virtual ~ThreadSafetyHandler() {}
+ virtual void handleInvalidLockExp(SourceLocation Loc) {}
virtual void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {}
virtual void handleDoubleLock(Name LockName, SourceLocation Loc) {}
virtual void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc){}
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=139370&r1=139369&r2=139370&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 9 11:21:55 2011
@@ -1424,6 +1424,9 @@
def warn_fun_excludes_mutex : Warning<
"cannot call function '%0' while holding mutex '%1'">,
InGroup<ThreadSafety>, DefaultIgnore;
+def warn_cannot_resolve_lock : Warning<
+ "cannot resolve lock expression to a specific lockable object">,
+ InGroup<ThreadSafety>, DefaultIgnore;
def warn_impcast_vector_scalar : Warning<
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=139370&r1=139369&r2=139370&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 9 11:21:55 2011
@@ -37,6 +37,15 @@
using namespace clang;
using namespace thread_safety;
+// Helper functions
+static Expr *getParent(Expr *Exp) {
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp))
+ return ME->getBase();
+ if (CXXMemberCallExpr *CE = dyn_cast<CXXMemberCallExpr>(Exp))
+ return CE->getImplicitObjectArgument();
+ return 0;
+}
+
namespace {
/// \brief Implements a set of CFGBlocks using a BitVector.
///
@@ -146,28 +155,32 @@
/// foo(MyL); // requires lock MyL->Mu to be held
class MutexID {
SmallVector<NamedDecl*, 2> DeclSeq;
+ ThreadSafetyHandler &Handler;
/// Build a Decl sequence representing the lock from the given expression.
/// Recursive function that bottoms out when the final DeclRefExpr is reached.
- void buildMutexID(Expr *Exp) {
+ void buildMutexID(Expr *Exp, Expr *Parent) {
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) {
NamedDecl *ND = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl());
DeclSeq.push_back(ND);
} else if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) {
NamedDecl *ND = ME->getMemberDecl();
DeclSeq.push_back(ND);
- buildMutexID(ME->getBase());
+ buildMutexID(ME->getBase(), Parent);
} else if (isa<CXXThisExpr>(Exp)) {
- return;
- } else {
- // FIXME: add diagnostic
- llvm::report_fatal_error("Expected lock expression!");
- }
+ if (!Parent)
+ return;
+ buildMutexID(Parent, 0);
+ } else if (CastExpr *CE = dyn_cast<CastExpr>(Exp))
+ buildMutexID(CE->getSubExpr(), Parent);
+ else
+ Handler.handleInvalidLockExp(Exp->getExprLoc());
}
public:
- MutexID(Expr *LExpr) {
- buildMutexID(LExpr);
+ MutexID(ThreadSafetyHandler &Handler, Expr *LExpr, Expr *ParentExpr)
+ : Handler(Handler) {
+ buildMutexID(LExpr, ParentExpr);
assert(!DeclSeq.empty());
}
@@ -252,8 +265,9 @@
Lockset::Factory &LocksetFactory;
// Helper functions
- void removeLock(SourceLocation UnlockLoc, Expr *LockExp);
- void addLock(SourceLocation LockLoc, Expr *LockExp, LockKind LK);
+ void removeLock(SourceLocation UnlockLoc, Expr *LockExp, Expr *Parent);
+ void addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent,
+ LockKind LK);
const ValueDecl *getValueDecl(Expr *Exp);
void warnIfMutexNotHeld (const NamedDecl *D, Expr *Exp, AccessKind AK,
Expr *MutexExp, ProtectedOperationKind POK);
@@ -307,10 +321,10 @@
/// \brief Add a new lock to the lockset, warning if the lock is already there.
/// \param LockLoc The source location of the acquire
/// \param LockExp The lock expression corresponding to the lock to be added
-void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp,
+void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent,
LockKind LK) {
// FIXME: deal with acquired before/after annotations
- MutexID Mutex(LockExp);
+ MutexID Mutex(Handler, LockExp, Parent);
LockData NewLock(LockLoc, LK);
// FIXME: Don't always warn when we have support for reentrant locks.
@@ -322,8 +336,9 @@
/// \brief Remove a lock from the lockset, warning if the lock is not there.
/// \param LockExp The lock expression corresponding to the lock to be removed
/// \param UnlockLoc The source location of the unlock (only used in error msg)
-void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp) {
- MutexID Mutex(LockExp);
+void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp,
+ Expr *Parent) {
+ MutexID Mutex(Handler, LockExp, Parent);
Lockset NewLSet = LocksetFactory.remove(LSet, Mutex);
if(NewLSet == LSet)
@@ -349,7 +364,8 @@
AccessKind AK, Expr *MutexExp,
ProtectedOperationKind POK) {
LockKind LK = getLockKindFromAccessKind(AK);
- MutexID Mutex(MutexExp);
+ Expr *Parent = getParent(Exp);
+ MutexID Mutex(Handler, MutexExp, Parent);
if (!locksetContainsAtLeast(Mutex, LK))
Handler.handleMutexNotHeld(D, POK, Mutex.getName(), LK, Exp->getExprLoc());
}
@@ -451,13 +467,13 @@
if (SpecificAttr->args_size() == 0) {
// The mutex held is the "this" object.
- addLock(ExpLocation, Parent, LK);
+ addLock(ExpLocation, Parent, Parent, LK);
return;
}
for (iterator_type I = SpecificAttr->args_begin(),
E = SpecificAttr->args_end(); I != E; ++I)
- addLock(ExpLocation, *I, LK);
+ addLock(ExpLocation, *I, Parent, LK);
}
/// \brief When visiting CXXMemberCallExprs we need to examine the attributes on
@@ -501,13 +517,13 @@
UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr);
if (UFAttr->args_size() == 0) { // The lock held is the "this" object.
- removeLock(ExpLocation, Parent);
+ removeLock(ExpLocation, Parent, Parent);
break;
}
for (UnlockFunctionAttr::args_iterator I = UFAttr->args_begin(),
E = UFAttr->args_end(); I != E; ++I)
- removeLock(ExpLocation, *I);
+ removeLock(ExpLocation, *I, Parent);
break;
}
@@ -540,7 +556,7 @@
LocksExcludedAttr *LEAttr = cast<LocksExcludedAttr>(Attr);
for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(),
E = LEAttr->args_end(); I != E; ++I) {
- MutexID Mutex(*I);
+ MutexID Mutex(Handler, *I, Parent);
if (locksetContains(Mutex))
Handler.handleFunExcludesLock(D->getName(), Mutex.getName(),
ExpLocation);
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=139370&r1=139369&r2=139370&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Sep 9 11:21:55 2011
@@ -648,6 +648,10 @@
S.Diag(I->first, I->second);
}
+ void handleInvalidLockExp(SourceLocation Loc) {
+ PartialDiagnostic Warning = S.PDiag(diag::warn_cannot_resolve_lock) << Loc;
+ Warnings.push_back(DelayedDiag(Loc, Warning));
+ }
void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {
warnLockMismatch(diag::warn_unlock_but_no_lock, LockName, 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=139370&r1=139369&r2=139370&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Sep 9 11:21:55 2011
@@ -425,6 +425,80 @@
Mutex mu;
};
+class LateBar {
+ public:
+ int a_ __attribute__((guarded_by(mu1_)));
+ int b_;
+ int *q __attribute__((pt_guarded_by(mu)));
+ Mutex mu1_;
+ Mutex mu;
+ LateFoo Foo;
+ LateFoo Foo2;
+ LateFoo *FooPointer;
+};
+
+LateBar b1, *b3;
+
+void late_0() {
+ LateFoo FooA;
+ LateFoo FooB;
+ FooA.mu.Lock();
+ FooA.a = 5;
+ FooA.mu.Unlock();
+}
+
+void late_1() {
+ LateBar BarA;
+ BarA.FooPointer->mu.Lock();
+ BarA.FooPointer->a = 2;
+ BarA.FooPointer->mu.Unlock();
+}
+
+void late_bad_0() {
+ LateFoo fooA;
+ LateFoo fooB;
+ fooA.mu.Lock();
+ fooB.a = 5; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ fooA.mu.Unlock();
+}
+
+void late_bad_1() {
+ Mutex mu;
+ mu.Lock();
+ b1.mu1_.Lock();
+ int res = b1.a_ + b3->b_;
+ b3->b_ = *b1.q; // \
+ // expected-warning{{reading the value pointed to by 'q' requires lock on 'mu' to be held}}
+ b1.mu1_.Unlock();
+ b1.b_ = res;
+ mu.Unlock();
+}
+
+void late_bad_2() {
+ LateBar BarA;
+ BarA.FooPointer->mu.Lock();
+ BarA.Foo.a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.FooPointer->mu.Unlock();
+}
+
+void late_bad_3() {
+ LateBar BarA;
+ BarA.Foo.mu.Lock();
+ BarA.FooPointer->a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.Foo.mu.Unlock();
+}
+
+void late_bad_4() {
+ LateBar BarA;
+ BarA.Foo.mu.Lock();
+ BarA.Foo2.a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.Foo.mu.Unlock();
+}
+
//-----------------------------------------------//
// Extra warnings for shared vs. exclusive locks
// ----------------------------------------------//
More information about the cfe-commits
mailing list