Here is a patch which adds test cases, and identifies locks which cannot be resolved more cleanly.
http://codereview.appspot.com/4960064/ Cheers, Caitlin On Fri, Sep 9, 2011 at 9:43 AM, Chandler Carruth <[email protected]> wrote: > Err, I don't see any tests that actually exercise this case: > On Fri, Sep 9, 2011 at 9:21 AM, Caitlin Sadowski <[email protected]> > wrote: >> >> +def warn_cannot_resolve_lock : Warning< >> + "cannot resolve lock expression to a specific lockable object">, >> + InGroup<ThreadSafety>, DefaultIgnore; > >
From 69a80db6f612c6c64822098bc0d060be54731a87 Mon Sep 17 00:00:00 2001 From: Caitlin Sadowski <[email protected]> Date: Fri, 9 Sep 2011 13:22:17 -0700 Subject: [PATCH] Thread safety: adding test cases for unparseable lock expressions and expanding the handling of these expressions --- lib/Analysis/ThreadSafety.cpp | 25 +++++++++++++++------- test/SemaCXX/warn-thread-safety-analysis.cpp | 28 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index cefedfd..bed1192 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -156,7 +156,6 @@ public: 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, Expr *Parent) { @@ -167,21 +166,27 @@ class MutexID { NamedDecl *ND = ME->getMemberDecl(); DeclSeq.push_back(ND); buildMutexID(ME->getBase(), Parent); - } else if (isa<CXXThisExpr>(Exp)) { - if (!Parent) - return; + } else if (isa<CXXThisExpr>(Exp) && Parent) { buildMutexID(Parent, 0); } else if (CastExpr *CE = dyn_cast<CastExpr>(Exp)) buildMutexID(CE->getSubExpr(), Parent); else - Handler.handleInvalidLockExp(Exp->getExprLoc()); + Valid = false; + Valid = true; } public: + /// If we could not properly build the DeclSeq, we mark this lock as not Valid + /// and do not use it later. + bool Valid; + MutexID(ThreadSafetyHandler &Handler, Expr *LExpr, Expr *ParentExpr) : Handler(Handler) { buildMutexID(LExpr, ParentExpr); - assert(!DeclSeq.empty()); + if (DeclSeq.empty()) + Valid = false; + if (!Valid) + Handler.handleInvalidLockExp(LExpr->getExprLoc()); } bool operator==(const MutexID &other) const { @@ -207,6 +212,7 @@ public: /// name in diagnostics is a way to get simple, and consistent, mutex names. /// We do not want to output the entire expression text for security reasons. StringRef getName() const { + assert(Valid); return DeclSeq.front()->getName(); } @@ -325,6 +331,8 @@ void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent, LockKind LK) { // FIXME: deal with acquired before/after annotations MutexID Mutex(Handler, LockExp, Parent); + if (!Mutex.Valid) return; + LockData NewLock(LockLoc, LK); // FIXME: Don't always warn when we have support for reentrant locks. @@ -339,6 +347,7 @@ void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent, void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp, Expr *Parent) { MutexID Mutex(Handler, LockExp, Parent); + if (!Mutex.Valid) return; Lockset NewLSet = LocksetFactory.remove(LSet, Mutex); if(NewLSet == LSet) @@ -366,7 +375,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp, LockKind LK = getLockKindFromAccessKind(AK); Expr *Parent = getParent(Exp); MutexID Mutex(Handler, MutexExp, Parent); - if (!locksetContainsAtLeast(Mutex, LK)) + if (Mutex.Valid && !locksetContainsAtLeast(Mutex, LK)) Handler.handleMutexNotHeld(D, POK, Mutex.getName(), LK, Exp->getExprLoc()); } @@ -557,7 +566,7 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) { for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(), E = LEAttr->args_end(); I != E; ++I) { MutexID Mutex(Handler, *I, Parent); - if (locksetContains(Mutex)) + if (Mutex.Valid && locksetContains(Mutex)) Handler.handleFunExcludesLock(D->getName(), Mutex.getName(), ExpLocation); } diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 85de918..a3f37ef 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -701,3 +701,31 @@ void es_bad_7() { // expected-warning {{cannot call function 'le_fun' while holding mutex 'sls_mu'}} sls_mu.Unlock(); } + +//-----------------------------------------------// +// Unparseable lock expressions +// ----------------------------------------------// + +Mutex UPmu; +// FIXME: add support for lock expressions involving arrays. +Mutex mua[5]; + +int x __attribute__((guarded_by(UPmu = sls_mu))); // \ + // expected-warning{{cannot resolve lock expression to a specific lockable object}} +int y __attribute__((guarded_by(mua[0]))); // \ + // expected-warning{{cannot resolve lock expression to a specific lockable object}} + + +void testUnparse() { + // no errors, since the lock expressions are not resolved + x = 5; + y = 5; +} + +void testUnparse2() { + mua[0].Lock(); // \ + // expected-warning{{cannot resolve lock expression to a specific lockable object}} + (&(mua[0]) + 4)->Lock(); // \ + // expected-warning{{cannot resolve lock expression to a specific lockable object}} +} + -- 1.7.3.1
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
