[cfe-commits] r146174 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp lib/Sema/AnalysisBasedWarnings.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Thu Dec 8 12:23:07 PST 2011
Author: delesley
Date: Thu Dec 8 14:23:06 2011
New Revision: 146174
URL: http://llvm.org/viewvc/llvm-project?rev=146174&view=rev
Log:
This patch extends thread safety analysis with support for the scoped_lockable attribute.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.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=146174&r1=146173&r2=146174&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Dec 8 14:23:06 2011
@@ -172,6 +172,10 @@
}
public:
+ explicit MutexID(clang::Decl::EmptyShell e) {
+ DeclSeq.clear();
+ }
+
/// \param MutexExp The original mutex expression within an attribute
/// \param DeclExp An expression involving the Decl on which the attribute
/// occurs.
@@ -249,9 +253,14 @@
///
/// FIXME: add support for re-entrant locking and lock up/downgrading
LockKind LKind;
+ MutexID UnderlyingMutex; // for ScopedLockable objects
LockData(SourceLocation AcquireLoc, LockKind LKind)
- : AcquireLoc(AcquireLoc), LKind(LKind) {}
+ : AcquireLoc(AcquireLoc), LKind(LKind), UnderlyingMutex(Decl::EmptyShell())
+ {}
+
+ LockData(SourceLocation AcquireLoc, LockKind LKind, const MutexID &Mu)
+ : AcquireLoc(AcquireLoc), LKind(LKind), UnderlyingMutex(Mu) {}
bool operator==(const LockData &other) const {
return AcquireLoc == other.AcquireLoc && LKind == other.LKind;
@@ -285,11 +294,12 @@
Lockset::Factory &LocksetFactory;
// Helper functions
- void removeLock(SourceLocation UnlockLoc, MutexID &Mutex);
- void addLock(SourceLocation LockLoc, MutexID &Mutex, LockKind LK);
+ void addLock(const MutexID &Mutex, const LockData &LDat);
+ void removeLock(const MutexID &Mutex, SourceLocation UnlockLoc);
template <class AttrType>
- void addLocksToSet(LockKind LK, AttrType *Attr, Expr *Exp, NamedDecl *D);
+ void addLocksToSet(LockKind LK, AttrType *Attr,
+ Expr *Exp, NamedDecl *D, VarDecl *VD = 0);
void removeLocksFromSet(UnlockFunctionAttr *Attr,
Expr *Exp, NamedDecl* FunDecl);
@@ -298,7 +308,7 @@
Expr *MutexExp, ProtectedOperationKind POK);
void checkAccess(Expr *Exp, AccessKind AK);
void checkDereference(Expr *Exp, AccessKind AK);
- void handleCall(Expr *Exp, NamedDecl *D);
+ void handleCall(Expr *Exp, NamedDecl *D, VarDecl *VD = 0);
/// \brief Returns true if the lockset contains a lock, regardless of whether
/// the lock is held exclusively or shared.
@@ -342,51 +352,62 @@
void VisitCastExpr(CastExpr *CE);
void VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp);
void VisitCXXConstructExpr(CXXConstructExpr *Exp);
+ void VisitDeclStmt(DeclStmt *S);
};
/// \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, MutexID &Mutex,
- LockKind LK) {
- // FIXME: deal with acquired before/after annotations. We can write a first
- // pass that does the transitive lookup lazily, and refine afterwards.
- LockData NewLock(LockLoc, LK);
-
+/// \param Mutex -- the Mutex expression for the lock
+/// \param LDat -- the LockData for the lock
+void BuildLockset::addLock(const MutexID &Mutex, const LockData& LDat) {
+ // FIXME: deal with acquired before/after annotations.
// FIXME: Don't always warn when we have support for reentrant locks.
if (locksetContains(Mutex))
- Handler.handleDoubleLock(Mutex.getName(), LockLoc);
+ Handler.handleDoubleLock(Mutex.getName(), LDat.AcquireLoc);
else
- LSet = LocksetFactory.add(LSet, Mutex, NewLock);
+ LSet = LocksetFactory.add(LSet, Mutex, LDat);
}
/// \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, MutexID &Mutex) {
- Lockset NewLSet = LocksetFactory.remove(LSet, Mutex);
- if(NewLSet == LSet)
+void BuildLockset::removeLock(const MutexID &Mutex, SourceLocation UnlockLoc) {
+ const LockData *LDat = LSet.lookup(Mutex);
+ if (!LDat)
Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc);
- else
- LSet = NewLSet;
+ else {
+ // For scoped-lockable vars, remove the mutex associated with this var.
+ if (LDat->UnderlyingMutex.isValid())
+ removeLock(LDat->UnderlyingMutex, UnlockLoc);
+ LSet = LocksetFactory.remove(LSet, Mutex);
+ }
}
/// \brief This function, parameterized by an attribute type, is used to add a
/// set of locks specified as attribute arguments to the lockset.
template <typename AttrType>
void BuildLockset::addLocksToSet(LockKind LK, AttrType *Attr,
- Expr *Exp, NamedDecl* FunDecl) {
+ Expr *Exp, NamedDecl* FunDecl, VarDecl *VD) {
typedef typename AttrType::args_iterator iterator_type;
SourceLocation ExpLocation = Exp->getExprLoc();
+ // Figure out if we're calling the constructor of scoped lockable class
+ bool isScopedVar = false;
+ if (VD) {
+ if (CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FunDecl)) {
+ CXXRecordDecl* PD = CD->getParent();
+ if (PD && PD->getAttr<ScopedLockableAttr>())
+ isScopedVar = true;
+ }
+ }
+
if (Attr->args_size() == 0) {
// The mutex held is the "this" object.
MutexID Mutex(0, Exp, FunDecl);
if (!Mutex.isValid())
MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl);
else
- addLock(ExpLocation, Mutex, LK);
+ addLock(Mutex, LockData(ExpLocation, LK));
return;
}
@@ -394,8 +415,15 @@
MutexID Mutex(*I, Exp, FunDecl);
if (!Mutex.isValid())
MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
- else
- addLock(ExpLocation, Mutex, LK);
+ else {
+ addLock(Mutex, LockData(ExpLocation, LK));
+ if (isScopedVar) {
+ // For scoped lockable vars, map this var to its underlying mutex.
+ DeclRefExpr DRE(VD, VD->getType(), VK_LValue, VD->getLocation());
+ MutexID SMutex(&DRE, 0, 0);
+ addLock(SMutex, LockData(VD->getLocation(), LK, Mutex));
+ }
+ }
}
}
@@ -412,7 +440,7 @@
if (!Mu.isValid())
MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl);
else
- removeLock(ExpLocation, Mu);
+ removeLock(Mu, ExpLocation);
return;
}
@@ -422,7 +450,7 @@
if (!Mutex.isValid())
MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
else
- removeLock(ExpLocation, Mutex);
+ removeLock(Mutex, ExpLocation);
}
}
@@ -508,7 +536,7 @@
///
/// FIXME: Do not flag an error for member variables accessed in constructors/
/// destructors
-void BuildLockset::handleCall(Expr *Exp, NamedDecl *D) {
+void BuildLockset::handleCall(Expr *Exp, NamedDecl *D, VarDecl *VD) {
AttrVec &ArgAttrs = D->getAttrs();
for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
Attr *Attr = ArgAttrs[i];
@@ -517,7 +545,7 @@
// to our lockset with kind exclusive.
case attr::ExclusiveLockFunction: {
ExclusiveLockFunctionAttr *A = cast<ExclusiveLockFunctionAttr>(Attr);
- addLocksToSet(LK_Exclusive, A, Exp, D);
+ addLocksToSet(LK_Exclusive, A, Exp, D, VD);
break;
}
@@ -525,7 +553,7 @@
// to our lockset with kind shared.
case attr::SharedLockFunction: {
SharedLockFunctionAttr *A = cast<SharedLockFunctionAttr>(Attr);
- addLocksToSet(LK_Shared, A, Exp, D);
+ addLocksToSet(LK_Shared, A, Exp, D, VD);
break;
}
@@ -627,10 +655,23 @@
}
void BuildLockset::VisitCXXConstructExpr(CXXConstructExpr *Exp) {
- NamedDecl *D = cast<NamedDecl>(Exp->getConstructor());
- if(!D || !D->hasAttrs())
- return;
- handleCall(Exp, D);
+ // FIXME -- only handles constructors in DeclStmt below.
+}
+
+void BuildLockset::VisitDeclStmt(DeclStmt *S) {
+ DeclGroupRef DGrp = S->getDeclGroup();
+ for (DeclGroupRef::iterator I = DGrp.begin(), E = DGrp.end(); I != E; ++I) {
+ Decl *D = *I;
+ if (VarDecl *VD = dyn_cast_or_null<VarDecl>(D)) {
+ Expr *E = VD->getInit();
+ if (CXXConstructExpr *CE = dyn_cast_or_null<CXXConstructExpr>(E)) {
+ NamedDecl *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());
+ if (!CtorD || !CtorD->hasAttrs())
+ return;
+ handleCall(CE, CtorD, VD);
+ }
+ }
+ }
}
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=146174&r1=146173&r2=146174&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Dec 8 14:23:06 2011
@@ -834,8 +834,8 @@
// prototyping, but we need a way for analyses to say what expressions they
// expect to always be CFGElements and then fill in the BuildOptions
// appropriately. This is essentially a layering violation.
- if (P.enableCheckUnreachable) {
- // Unreachable code analysis requires a linearized CFG.
+ if (P.enableCheckUnreachable || P.enableThreadSafetyAnalysis) {
+ // Unreachable code analysis and thread safety require a linearized CFG.
AC.getCFGBuildOptions().setAllAlwaysAdd();
}
else {
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=146174&r1=146173&r2=146174&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Dec 8 14:23:06 2011
@@ -36,6 +36,18 @@
void LockWhen(const int &cond) __attribute__((exclusive_lock_function));
};
+class __attribute__((scoped_lockable)) MutexLock {
+ public:
+ MutexLock(Mutex *mu) __attribute__((exclusive_lock_function(mu)));
+ ~MutexLock() __attribute__((unlock_function));
+};
+
+class __attribute__((scoped_lockable)) ReaderMutexLock {
+ public:
+ ReaderMutexLock(Mutex *mu) __attribute__((exclusive_lock_function(mu)));
+ ~ReaderMutexLock() __attribute__((unlock_function));
+};
+
Mutex sls_mu;
@@ -1549,3 +1561,47 @@
template struct W<int>; // expected-note {{here}}
}
+
+namespace test_scoped_lockable {
+
+struct TestScopedLockable {
+ Mutex mu1;
+ Mutex mu2;
+ int a __attribute__((guarded_by(mu1)));
+ int b __attribute__((guarded_by(mu2)));
+
+ bool getBool();
+
+ void foo1() {
+ MutexLock mulock(&mu1);
+ a = 5;
+ }
+
+ void foo2() {
+ ReaderMutexLock mulock1(&mu1);
+ if (getBool()) {
+ MutexLock mulock2a(&mu2);
+ b = a + 1;
+ }
+ else {
+ MutexLock mulock2b(&mu2);
+ b = a + 2;
+ }
+ }
+
+ void foo3() {
+ MutexLock mulock_a(&mu1);
+ MutexLock mulock_b(&mu1); // \
+ // expected-warning {{locking 'mu1' that is already locked}}
+ } // expected-warning {{unlocking 'mu1' that was not locked}}
+
+ void foo4() {
+ MutexLock mulock1(&mu1), mulock2(&mu2);
+ a = b+1;
+ b = a+1;
+ }
+};
+
+} // end namespace test_scoped_lockable
+
+
More information about the cfe-commits
mailing list