[cfe-commits] r139720 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
Caitlin Sadowski
supertri at google.com
Wed Sep 14 13:00:24 PDT 2011
Author: supertri
Date: Wed Sep 14 15:00:24 2011
New Revision: 139720
URL: http://llvm.org/viewvc/llvm-project?rev=139720&view=rev
Log:
Thread safety: adding test cases for unparseable lock expressions and expanding the handling of these expressions
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=139720&r1=139719&r2=139720&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Wed Sep 14 15:00:24 2011
@@ -154,7 +154,6 @@
/// 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.
@@ -167,20 +166,24 @@
DeclSeq.push_back(ND);
buildMutexID(ME->getBase(), Parent);
} else if (isa<CXXThisExpr>(Exp)) {
- if (!Parent)
- return;
- buildMutexID(Parent, 0);
+ if (Parent)
+ buildMutexID(Parent, 0);
+ else
+ return; // mutexID is still valid in this case
} else if (CastExpr *CE = dyn_cast<CastExpr>(Exp))
buildMutexID(CE->getSubExpr(), Parent);
else
- Handler.handleInvalidLockExp(Exp->getExprLoc());
+ DeclSeq.clear(); // invalid lock expression
}
public:
- MutexID(ThreadSafetyHandler &Handler, Expr *LExpr, Expr *ParentExpr)
- : Handler(Handler) {
+ MutexID(Expr *LExpr, Expr *ParentExpr) {
buildMutexID(LExpr, ParentExpr);
- assert(!DeclSeq.empty());
+ }
+
+ /// If we encounter part of a lock expression we cannot parse
+ bool isValid() const {
+ return !DeclSeq.empty();
}
bool operator==(const MutexID &other) const {
@@ -206,6 +209,7 @@
/// 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(isValid());
return DeclSeq.front()->getName();
}
@@ -324,7 +328,12 @@
void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent,
LockKind LK) {
// FIXME: deal with acquired before/after annotations
- MutexID Mutex(Handler, LockExp, Parent);
+ MutexID Mutex(LockExp, Parent);
+ if (!Mutex.isValid()) {
+ Handler.handleInvalidLockExp(LockExp->getExprLoc());
+ return;
+ }
+
LockData NewLock(LockLoc, LK);
// FIXME: Don't always warn when we have support for reentrant locks.
@@ -338,7 +347,11 @@
/// \param UnlockLoc The source location of the unlock (only used in error msg)
void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp,
Expr *Parent) {
- MutexID Mutex(Handler, LockExp, Parent);
+ MutexID Mutex(LockExp, Parent);
+ if (!Mutex.isValid()) {
+ Handler.handleInvalidLockExp(LockExp->getExprLoc());
+ return;
+ }
Lockset NewLSet = LocksetFactory.remove(LSet, Mutex);
if(NewLSet == LSet)
@@ -365,8 +378,10 @@
ProtectedOperationKind POK) {
LockKind LK = getLockKindFromAccessKind(AK);
Expr *Parent = getParent(Exp);
- MutexID Mutex(Handler, MutexExp, Parent);
- if (!locksetContainsAtLeast(Mutex, LK))
+ MutexID Mutex(MutexExp, Parent);
+ if (!Mutex.isValid())
+ Handler.handleInvalidLockExp(MutexExp->getExprLoc());
+ else if (!locksetContainsAtLeast(Mutex, LK))
Handler.handleMutexNotHeld(D, POK, Mutex.getName(), LK, Exp->getExprLoc());
}
@@ -467,7 +482,7 @@
if (SpecificAttr->args_size() == 0) {
// The mutex held is the "this" object.
- addLock(ExpLocation, Parent, Parent, LK);
+ addLock(ExpLocation, Parent, 0, LK);
return;
}
@@ -517,7 +532,7 @@
UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr);
if (UFAttr->args_size() == 0) { // The lock held is the "this" object.
- removeLock(ExpLocation, Parent, Parent);
+ removeLock(ExpLocation, Parent, 0);
break;
}
@@ -556,8 +571,10 @@
LocksExcludedAttr *LEAttr = cast<LocksExcludedAttr>(Attr);
for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(),
E = LEAttr->args_end(); I != E; ++I) {
- MutexID Mutex(Handler, *I, Parent);
- if (locksetContains(Mutex))
+ MutexID Mutex(*I, Parent);
+ if (!Mutex.isValid())
+ Handler.handleInvalidLockExp((*I)->getExprLoc());
+ else if (locksetContains(Mutex))
Handler.handleFunExcludesLock(D->getName(), Mutex.getName(),
ExpLocation);
}
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=139720&r1=139719&r2=139720&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Wed Sep 14 15:00:24 2011
@@ -28,14 +28,15 @@
class MutexWrapper {
public:
Mutex mu;
- // int x __attribute__((guarded_by(mu))); // FIXME: scoping error
+ int x __attribute__((guarded_by(mu)));
+ void MyLock() __attribute__((exclusive_lock_function(mu)));
};
MutexWrapper sls_mw;
void sls_fun_0() {
sls_mw.mu.Lock();
- // sls_mw.x = 5; // FIXME: turn mu into sls_mw.mu
+ sls_mw.x = 5;
sls_mw.mu.Unlock();
}
@@ -121,6 +122,11 @@
sls_mu.Unlock();
}
+void sls_fun_good_8() {
+ sls_mw.MyLock();
+ sls_mw.mu.Unlock();
+}
+
void sls_fun_bad_1() {
sls_mu.Unlock(); // \
// expected-warning{{unlocking 'sls_mu' that was not locked}}
@@ -701,3 +707,31 @@
// 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}}
+}
+
More information about the cfe-commits
mailing list