[cfe-commits] r159780 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Thu Jul 5 14:16:29 PDT 2012
Author: delesley
Date: Thu Jul 5 16:16:29 2012
New Revision: 159780
URL: http://llvm.org/viewvc/llvm-project?rev=159780&view=rev
Log:
Thread-safety analysis: eliminate false positives in case where the definition
duplicates attributes on the declaration. Also eliminates a false negative in
ReleasableMutexLock. Fixing this bug required some refactoring.
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=159780&r1=159779&r2=159780&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Jul 5 16:16:29 2012
@@ -356,6 +356,25 @@
};
+/// \brief A short list of MutexIDs
+class MutexIDList : public SmallVector<MutexID, 3> {
+public:
+ /// \brief Return true if the list contains the specified MutexID
+ /// Performs a linear search, because these lists are almost always very small.
+ bool contains(const MutexID& M) {
+ for (iterator I=begin(),E=end(); I != E; ++I)
+ if ((*I) == M) return true;
+ return false;
+ }
+
+ /// \brief Push M onto list, bud discard duplicates
+ void push_back_nodup(const MutexID& M) {
+ if (!contains(M)) push_back(M);
+ }
+};
+
+
+
/// \brief This is a helper class that stores info about the most recent
/// accquire of a Lock.
///
@@ -945,25 +964,20 @@
ThreadSafetyAnalyzer(ThreadSafetyHandler &H) : Handler(H) {}
Lockset addLock(const Lockset &LSet, const MutexID &Mutex,
- const LockData &LDat, bool Warn=true);
- Lockset addLock(const Lockset &LSet, Expr *MutexExp, const NamedDecl *D,
- const LockData &LDat, bool Warn=true);
+ const LockData &LDat);
Lockset removeLock(const Lockset &LSet, const MutexID &Mutex,
- SourceLocation UnlockLoc,
- bool Warn=true, bool FullyRemove=false);
+ SourceLocation UnlockLoc, bool FullyRemove=false);
- template <class AttrType>
- Lockset addLocksToSet(const Lockset &LSet, LockKind LK, AttrType *Attr,
- Expr *Exp, NamedDecl *D, VarDecl *VD = 0);
- Lockset removeLocksFromSet(const Lockset &LSet,
- UnlockFunctionAttr *Attr,
- Expr *Exp, NamedDecl* FunDecl);
+ template <typename AttrType>
+ void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp,
+ const NamedDecl *D);
template <class AttrType>
- Lockset addTrylock(const Lockset &LSet,
- LockKind LK, AttrType *Attr, Expr *Exp, NamedDecl *FunDecl,
- const CFGBlock* PredBlock, const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg);
+ void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp,
+ const NamedDecl *D,
+ const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
+ Expr *BrE, bool Neg);
+
const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C,
bool &Negate);
@@ -989,32 +1003,17 @@
/// \param LDat -- the LockData for the lock
Lockset ThreadSafetyAnalyzer::addLock(const Lockset &LSet,
const MutexID &Mutex,
- const LockData &LDat,
- bool Warn) {
+ const LockData &LDat) {
// FIXME: deal with acquired before/after annotations.
// FIXME: Don't always warn when we have support for reentrant locks.
if (LSet.lookup(Mutex)) {
- if (Warn)
- Handler.handleDoubleLock(Mutex.getName(), LDat.AcquireLoc);
+ Handler.handleDoubleLock(Mutex.getName(), LDat.AcquireLoc);
return LSet;
} else {
return LocksetFactory.add(LSet, Mutex, LDat);
}
}
-/// \brief Construct a new mutex and add it to the lockset.
-Lockset ThreadSafetyAnalyzer::addLock(const Lockset &LSet,
- Expr *MutexExp, const NamedDecl *D,
- const LockData &LDat,
- bool Warn) {
- MutexID Mutex(MutexExp, 0, D);
- if (!Mutex.isValid()) {
- MutexID::warnInvalidLock(Handler, MutexExp, 0, D);
- return LSet;
- }
- return addLock(LSet, Mutex, LDat, Warn);
-}
-
/// \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
@@ -1022,129 +1021,72 @@
Lockset ThreadSafetyAnalyzer::removeLock(const Lockset &LSet,
const MutexID &Mutex,
SourceLocation UnlockLoc,
- bool Warn, bool FullyRemove) {
+ bool FullyRemove) {
const LockData *LDat = LSet.lookup(Mutex);
if (!LDat) {
- if (Warn)
- Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc);
+ Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc);
return LSet;
}
- else {
- Lockset Result = LSet;
- if (LDat->UnderlyingMutex.isValid()) {
- // For scoped-lockable vars, remove the mutex associated with this var.
- Result = removeLock(Result, LDat->UnderlyingMutex, UnlockLoc,
- false, true);
- // Fully remove the object only when the destructor is called
- if (FullyRemove)
- return LocksetFactory.remove(Result, Mutex);
- else
- return Result;
+ if (LDat->UnderlyingMutex.isValid()) {
+ // This is scoped lockable object, which manages the real mutex.
+ if (FullyRemove) {
+ // We're destroying the managing object.
+ // Remove the underlying mutex if it exists; but don't warn.
+ Lockset Result = LSet;
+ if (LSet.contains(LDat->UnderlyingMutex))
+ Result = LocksetFactory.remove(Result, LDat->UnderlyingMutex);
+ return LocksetFactory.remove(Result, Mutex);
+ } else {
+ // We're releasing the underlying mutex, but not destroying the
+ // managing object. Warn on dual release.
+ if (!LSet.contains(LDat->UnderlyingMutex)) {
+ Handler.handleUnmatchedUnlock(LDat->UnderlyingMutex.getName(),
+ UnlockLoc);
+ return LSet;
+ }
+ return LocksetFactory.remove(LSet, LDat->UnderlyingMutex);
}
- return LocksetFactory.remove(Result, Mutex);
}
+ return 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.
+/// \brief Extract the list of mutexIDs from the attribute on an expression,
+/// and push them onto Mtxs, discarding any duplicates.
template <typename AttrType>
-Lockset ThreadSafetyAnalyzer::addLocksToSet(const Lockset &LSet,
- LockKind LK, AttrType *Attr,
- Expr *Exp, NamedDecl* FunDecl,
- VarDecl *VD) {
+void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr,
+ Expr *Exp, const NamedDecl *D) {
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);
- return LSet;
- }
- else {
- return addLock(LSet, Mutex, LockData(ExpLocation, LK));
- }
+ MutexID Mu(0, Exp, D);
+ if (!Mu.isValid())
+ MutexID::warnInvalidLock(Handler, 0, Exp, D);
+ else
+ Mtxs.push_back_nodup(Mu);
+ return;
}
- Lockset Result = LSet;
for (iterator_type I=Attr->args_begin(), E=Attr->args_end(); I != E; ++I) {
- MutexID Mutex(*I, Exp, FunDecl);
- if (!Mutex.isValid())
- MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
- else {
- if (isScopedVar) {
- // Mutex is managed by scoped var -- suppress certain warnings.
- Result = addLock(Result, Mutex, LockData(ExpLocation, LK, true));
- // For scoped lockable vars, map this var to its underlying mutex.
- DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation());
- MutexID SMutex(&DRE, 0, 0);
- Result = addLock(Result, SMutex,
- LockData(VD->getLocation(), LK, Mutex));
- }
- else {
- Result = addLock(Result, Mutex, LockData(ExpLocation, LK));
- }
- }
- }
- return Result;
-}
-
-/// \brief This function removes a set of locks specified as attribute
-/// arguments from the lockset.
-Lockset ThreadSafetyAnalyzer::removeLocksFromSet(const Lockset &LSet,
- UnlockFunctionAttr *Attr,
- Expr *Exp,
- NamedDecl* FunDecl) {
- SourceLocation ExpLocation;
- if (Exp) ExpLocation = Exp->getExprLoc();
- bool Dtor = isa<CXXDestructorDecl>(FunDecl);
-
- if (Attr->args_size() == 0) {
- // The mutex held is the "this" object.
- MutexID Mu(0, Exp, FunDecl);
- if (!Mu.isValid()) {
- MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl);
- return LSet;
- } else {
- return removeLock(LSet, Mu, ExpLocation, true, Dtor);
- }
- }
-
- Lockset Result = LSet;
- for (UnlockFunctionAttr::args_iterator I = Attr->args_begin(),
- E = Attr->args_end(); I != E; ++I) {
- MutexID Mutex(*I, Exp, FunDecl);
- if (!Mutex.isValid())
- MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
+ MutexID Mu(*I, Exp, D);
+ if (!Mu.isValid())
+ MutexID::warnInvalidLock(Handler, *I, Exp, D);
else
- Result = removeLock(Result, Mutex, ExpLocation, true, Dtor);
+ Mtxs.push_back_nodup(Mu);
}
- return Result;
}
-/// \brief Add lock to set, if the current block is in the taken branch of a
-/// trylock.
+/// \brief Extract the list of mutexIDs from a trylock attribute. If the
+/// trylock applies to the given edge, then push them onto Mtxs, discarding
+/// any duplicates.
template <class AttrType>
-Lockset ThreadSafetyAnalyzer::addTrylock(const Lockset &LSet,
- LockKind LK, AttrType *Attr,
- Expr *Exp, NamedDecl *FunDecl,
- const CFGBlock *PredBlock,
- const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg) {
+void ThreadSafetyAnalyzer::getMutexIDs(MutexIDList &Mtxs, AttrType *Attr,
+ Expr *Exp, const NamedDecl *D,
+ const CFGBlock *PredBlock,
+ const CFGBlock *CurrBlock,
+ Expr *BrE, bool Neg) {
// Find out which branch has the lock
bool branch = 0;
if (CXXBoolLiteralExpr *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE)) {
@@ -1156,16 +1098,14 @@
int branchnum = branch ? 0 : 1;
if (Neg) branchnum = !branchnum;
- Lockset Result = LSet;
// If we've taken the trylock branch, then add the lock
int i = 0;
for (CFGBlock::const_succ_iterator SI = PredBlock->succ_begin(),
SE = PredBlock->succ_end(); SI != SE && i < 2; ++SI, ++i) {
if (*SI == CurrBlock && i == branchnum) {
- Result = addLocksToSet(Result, LK, Attr, Exp, FunDecl, 0);
+ getMutexIDs(Mtxs, Attr, Exp, D);
}
}
- return Result;
}
@@ -1213,8 +1153,8 @@
const CFGBlockInfo *PredBlockInfo = &BlockInfo[PredBlock->getBlockID()];
const LocalVarContext &LVarCtx = PredBlockInfo->ExitContext;
- CallExpr *Exp = const_cast<CallExpr*>(
- getTrylockCallExpr(Cond, LVarCtx, Negate));
+ CallExpr *Exp =
+ const_cast<CallExpr*>(getTrylockCallExpr(Cond, LVarCtx, Negate));
if (!Exp)
return ExitSet;
@@ -1222,7 +1162,9 @@
if(!FunDecl || !FunDecl->hasAttrs())
return ExitSet;
- Lockset Result = ExitSet;
+
+ MutexIDList ExclusiveLocksToAdd;
+ MutexIDList SharedLocksToAdd;
// If the condition is a call to a Trylock function, then grab the attributes
AttrVec &ArgAttrs = FunDecl->getAttrs();
@@ -1232,23 +1174,34 @@
case attr::ExclusiveTrylockFunction: {
ExclusiveTrylockFunctionAttr *A =
cast<ExclusiveTrylockFunctionAttr>(Attr);
- Result = addTrylock(Result, LK_Exclusive, A, Exp, FunDecl,
- PredBlock, CurrBlock,
- A->getSuccessValue(), Negate);
+ getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl,
+ PredBlock, CurrBlock, A->getSuccessValue(), Negate);
break;
}
case attr::SharedTrylockFunction: {
SharedTrylockFunctionAttr *A =
cast<SharedTrylockFunctionAttr>(Attr);
- Result = addTrylock(Result, LK_Shared, A, Exp, FunDecl,
- PredBlock, CurrBlock,
- A->getSuccessValue(), Negate);
+ getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl,
+ PredBlock, CurrBlock, A->getSuccessValue(), Negate);
break;
}
default:
break;
}
}
+
+ // Add and remove locks.
+ Lockset Result = ExitSet;
+ SourceLocation Loc = Exp->getExprLoc();
+ for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
+ Result = addLock(Result, ExclusiveLocksToAdd[i],
+ LockData(Loc, LK_Exclusive));
+ }
+ for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
+ Result = addLock(Result, SharedLocksToAdd[i],
+ LockData(Loc, LK_Shared));
+ }
+
return Result;
}
@@ -1274,7 +1227,7 @@
void checkAccess(Expr *Exp, AccessKind AK);
void checkDereference(Expr *Exp, AccessKind AK);
- void handleCall(Expr *Exp, NamedDecl *D, VarDecl *VD = 0);
+ void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = 0);
/// \brief Returns true if the lockset contains a lock, regardless of whether
/// the lock is held exclusively or shared.
@@ -1403,62 +1356,61 @@
/// and check that the appropriate locks are held. Non-const method calls with
/// the same signature as const method calls can be also treated as reads.
///
-/// FIXME: We need to also visit CallExprs to catch/check global functions.
-///
-/// FIXME: Do not flag an error for member variables accessed in constructors/
-/// destructors
-void BuildLockset::handleCall(Expr *Exp, NamedDecl *D, VarDecl *VD) {
- AttrVec &ArgAttrs = D->getAttrs();
+void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
+ const AttrVec &ArgAttrs = D->getAttrs();
+ MutexIDList ExclusiveLocksToAdd;
+ MutexIDList SharedLocksToAdd;
+ MutexIDList LocksToRemove;
+
for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
- Attr *Attr = ArgAttrs[i];
- switch (Attr->getKind()) {
+ Attr *At = const_cast<Attr*>(ArgAttrs[i]);
+ switch (At->getKind()) {
// When we encounter an exclusive lock function, we need to add the lock
// to our lockset with kind exclusive.
case attr::ExclusiveLockFunction: {
- ExclusiveLockFunctionAttr *A = cast<ExclusiveLockFunctionAttr>(Attr);
- LSet = Analyzer->addLocksToSet(LSet, LK_Exclusive, A, Exp, D, VD);
+ ExclusiveLockFunctionAttr *A = cast<ExclusiveLockFunctionAttr>(At);
+ Analyzer->getMutexIDs(ExclusiveLocksToAdd, A, Exp, D);
break;
}
// When we encounter a shared lock function, we need to add the lock
// to our lockset with kind shared.
case attr::SharedLockFunction: {
- SharedLockFunctionAttr *A = cast<SharedLockFunctionAttr>(Attr);
- LSet = Analyzer->addLocksToSet(LSet, LK_Shared, A, Exp, D, VD);
+ SharedLockFunctionAttr *A = cast<SharedLockFunctionAttr>(At);
+ Analyzer->getMutexIDs(SharedLocksToAdd, A, Exp, D);
break;
}
// When we encounter an unlock function, we need to remove unlocked
// mutexes from the lockset, and flag a warning if they are not there.
case attr::UnlockFunction: {
- UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr);
- LSet = Analyzer->removeLocksFromSet(LSet, UFAttr, Exp, D);
+ UnlockFunctionAttr *A = cast<UnlockFunctionAttr>(At);
+ Analyzer->getMutexIDs(LocksToRemove, A, Exp, D);
break;
}
case attr::ExclusiveLocksRequired: {
- ExclusiveLocksRequiredAttr *ELRAttr =
- cast<ExclusiveLocksRequiredAttr>(Attr);
+ ExclusiveLocksRequiredAttr *A = cast<ExclusiveLocksRequiredAttr>(At);
for (ExclusiveLocksRequiredAttr::args_iterator
- I = ELRAttr->args_begin(), E = ELRAttr->args_end(); I != E; ++I)
+ I = A->args_begin(), E = A->args_end(); I != E; ++I)
warnIfMutexNotHeld(D, Exp, AK_Written, *I, POK_FunctionCall);
break;
}
case attr::SharedLocksRequired: {
- SharedLocksRequiredAttr *SLRAttr = cast<SharedLocksRequiredAttr>(Attr);
+ SharedLocksRequiredAttr *A = cast<SharedLocksRequiredAttr>(At);
- for (SharedLocksRequiredAttr::args_iterator I = SLRAttr->args_begin(),
- E = SLRAttr->args_end(); I != E; ++I)
+ for (SharedLocksRequiredAttr::args_iterator I = A->args_begin(),
+ E = A->args_end(); I != E; ++I)
warnIfMutexNotHeld(D, Exp, AK_Read, *I, POK_FunctionCall);
break;
}
case attr::LocksExcluded: {
- LocksExcludedAttr *LEAttr = cast<LocksExcludedAttr>(Attr);
- for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(),
- E = LEAttr->args_end(); I != E; ++I) {
+ LocksExcludedAttr *A = cast<LocksExcludedAttr>(At);
+ for (LocksExcludedAttr::args_iterator I = A->args_begin(),
+ E = A->args_end(); I != E; ++I) {
MutexID Mutex(*I, Exp, D);
if (!Mutex.isValid())
MutexID::warnInvalidLock(Analyzer->Handler, *I, Exp, D);
@@ -1475,6 +1427,51 @@
break;
}
}
+
+ // Figure out if we're calling the constructor of scoped lockable class
+ bool isScopedVar = false;
+ if (VD) {
+ if (const CXXConstructorDecl *CD = dyn_cast<const CXXConstructorDecl>(D)) {
+ const CXXRecordDecl* PD = CD->getParent();
+ if (PD && PD->getAttr<ScopedLockableAttr>())
+ isScopedVar = true;
+ }
+ }
+
+ // Add locks.
+ SourceLocation Loc = Exp->getExprLoc();
+ for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
+ LSet = Analyzer->addLock(LSet, ExclusiveLocksToAdd[i],
+ LockData(Loc, LK_Exclusive, isScopedVar));
+ }
+ for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
+ LSet = Analyzer->addLock(LSet, SharedLocksToAdd[i],
+ LockData(Loc, LK_Shared, isScopedVar));
+ }
+
+ // Add the managing object as a dummy mutex, mapped to the underlying mutex.
+ // FIXME -- this doesn't work if we acquire multiple locks.
+ if (isScopedVar) {
+ SourceLocation MLoc = VD->getLocation();
+ DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation());
+ MutexID SMutex(&DRE, 0, 0);
+
+ for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
+ LSet = Analyzer->addLock(LSet, SMutex, LockData(MLoc, LK_Exclusive,
+ ExclusiveLocksToAdd[i]));
+ }
+ for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
+ LSet = Analyzer->addLock(LSet, SMutex, LockData(MLoc, LK_Shared,
+ SharedLocksToAdd[i]));
+ }
+ }
+
+ // Remove locks.
+ // FIXME -- should only fully remove if the attribute refers to 'this'.
+ bool Dtor = isa<CXXDestructorDecl>(D);
+ for (unsigned i=0,n=LocksToRemove.size(); i<n; ++i) {
+ LSet = Analyzer->removeLock(LSet, LocksToRemove[i], Loc, Dtor);
+ }
}
@@ -1636,6 +1633,7 @@
}
+
/// \brief Check a function's CFG for thread-safety violations.
///
/// We traverse the blocks in the CFG, compute the set of mutexes that are held
@@ -1683,23 +1681,20 @@
const CFGBlock *FirstBlock = *SortedGraph->begin();
Lockset &InitialLockset = BlockInfo[FirstBlock->getBlockID()].EntrySet;
const AttrVec &ArgAttrs = D->getAttrs();
+
+ MutexIDList ExclusiveLocksToAdd;
+ MutexIDList SharedLocksToAdd;
+
+ SourceLocation Loc = D->getLocation();
for (unsigned i = 0; i < ArgAttrs.size(); ++i) {
Attr *Attr = ArgAttrs[i];
- SourceLocation AttrLoc = Attr->getLocation();
- if (SharedLocksRequiredAttr *SLRAttr
- = dyn_cast<SharedLocksRequiredAttr>(Attr)) {
- for (SharedLocksRequiredAttr::args_iterator
- SLRIter = SLRAttr->args_begin(),
- SLREnd = SLRAttr->args_end(); SLRIter != SLREnd; ++SLRIter)
- InitialLockset = addLock(InitialLockset, *SLRIter, D,
- LockData(AttrLoc, LK_Shared), false);
- } else if (ExclusiveLocksRequiredAttr *ELRAttr
- = dyn_cast<ExclusiveLocksRequiredAttr>(Attr)) {
- for (ExclusiveLocksRequiredAttr::args_iterator
- ELRIter = ELRAttr->args_begin(),
- ELREnd = ELRAttr->args_end(); ELRIter != ELREnd; ++ELRIter)
- InitialLockset = addLock(InitialLockset, *ELRIter, D,
- LockData(AttrLoc, LK_Exclusive), false);
+ Loc = Attr->getLocation();
+ if (ExclusiveLocksRequiredAttr *A
+ = dyn_cast<ExclusiveLocksRequiredAttr>(Attr)) {
+ getMutexIDs(ExclusiveLocksToAdd, A, (Expr*) 0, D);
+ } else if (SharedLocksRequiredAttr *A
+ = dyn_cast<SharedLocksRequiredAttr>(Attr)) {
+ getMutexIDs(SharedLocksToAdd, A, (Expr*) 0, D);
} else if (isa<UnlockFunctionAttr>(Attr)) {
// Don't try to check unlock functions for now
return;
@@ -1717,6 +1712,16 @@
return;
}
}
+
+ // FIXME -- Loc can be wrong here.
+ for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
+ InitialLockset = addLock(InitialLockset, ExclusiveLocksToAdd[i],
+ LockData(Loc, LK_Exclusive));
+ }
+ for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
+ InitialLockset = addLock(InitialLockset, SharedLocksToAdd[i],
+ LockData(Loc, LK_Shared));
+ }
}
for (PostOrderCFGView::iterator I = SortedGraph->begin(),
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=159780&r1=159779&r2=159780&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Jul 5 16:16:29 2012
@@ -2395,6 +2395,8 @@
void test1();
void test2();
void test3();
+ void test4();
+ void test5();
};
@@ -2417,6 +2419,22 @@
a = 1; // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}}
}
+void Foo::test4() {
+ ReleasableMutexLock rlock(&mu_);
+ rlock.Release();
+ rlock.Release(); // expected-warning {{unlocking 'mu_' that was not locked}}
+}
+
+void Foo::test5() {
+ ReleasableMutexLock rlock(&mu_);
+ if (c) {
+ rlock.Release();
+ }
+ // no warning on join point for managed lock.
+ rlock.Release(); // expected-warning {{unlocking 'mu_' that was not locked}}
+}
+
+
} // end namespace ReleasableScopedLock
@@ -2457,15 +2475,21 @@
Mutex mu_;
int a GUARDED_BY(mu_);
- void foo() EXCLUSIVE_LOCKS_REQUIRED(mu_);
+ void foo1() EXCLUSIVE_LOCKS_REQUIRED(mu_);
+ int foo2() SHARED_LOCKS_REQUIRED(mu_);
};
-void Foo::foo() EXCLUSIVE_LOCKS_REQUIRED(mu_) {
+void Foo::foo1() EXCLUSIVE_LOCKS_REQUIRED(mu_) {
a = 0;
}
-};
+int Foo::foo2() SHARED_LOCKS_REQUIRED(mu_) {
+ return a;
+}
+
+}
+
namespace UnlockBug {
@@ -2487,6 +2511,7 @@
} // end namespace UnlockBug
+
namespace FoolishScopedLockableBug {
class SCOPED_LOCKABLE WTF_ScopedLockable {
@@ -2553,6 +2578,7 @@
} // end namespace FoolishScopedLockableBug
+
namespace TemporaryCleanupExpr {
class Foo {
@@ -2739,4 +2765,172 @@
+namespace DuplicateAttributeTest {
+
+class LOCKABLE Foo {
+public:
+ Mutex mu1_;
+ Mutex mu2_;
+ Mutex mu3_;
+ int a GUARDED_BY(mu1_);
+ int b GUARDED_BY(mu2_);
+ int c GUARDED_BY(mu3_);
+
+ void lock() EXCLUSIVE_LOCK_FUNCTION();
+ void unlock() UNLOCK_FUNCTION();
+
+ void lock1() EXCLUSIVE_LOCK_FUNCTION(mu1_);
+ void slock1() SHARED_LOCK_FUNCTION(mu1_);
+ void lock3() EXCLUSIVE_LOCK_FUNCTION(mu1_, mu2_, mu3_);
+ void locklots()
+ EXCLUSIVE_LOCK_FUNCTION(mu1_)
+ EXCLUSIVE_LOCK_FUNCTION(mu2_)
+ EXCLUSIVE_LOCK_FUNCTION(mu1_, mu2_, mu3_);
+
+ void unlock1() UNLOCK_FUNCTION(mu1_);
+ void unlock3() UNLOCK_FUNCTION(mu1_, mu2_, mu3_);
+ void unlocklots()
+ UNLOCK_FUNCTION(mu1_)
+ UNLOCK_FUNCTION(mu2_)
+ UNLOCK_FUNCTION(mu1_, mu2_, mu3_);
+};
+
+
+void Foo::lock() EXCLUSIVE_LOCK_FUNCTION() { }
+void Foo::unlock() UNLOCK_FUNCTION() { }
+
+void Foo::lock1() EXCLUSIVE_LOCK_FUNCTION(mu1_) {
+ mu1_.Lock();
+}
+
+void Foo::slock1() SHARED_LOCK_FUNCTION(mu1_) {
+ mu1_.Lock();
+}
+
+void Foo::lock3() EXCLUSIVE_LOCK_FUNCTION(mu1_, mu2_, mu3_) {
+ mu1_.Lock();
+ mu2_.Lock();
+ mu3_.Lock();
+}
+
+void Foo::locklots()
+ EXCLUSIVE_LOCK_FUNCTION(mu1_, mu2_)
+ EXCLUSIVE_LOCK_FUNCTION(mu2_, mu3_) {
+ mu1_.Lock();
+ mu2_.Lock();
+ mu3_.Lock();
+}
+
+void Foo::unlock1() UNLOCK_FUNCTION(mu1_) {
+ mu1_.Unlock();
+}
+
+void Foo::unlock3() UNLOCK_FUNCTION(mu1_, mu2_, mu3_) {
+ mu1_.Unlock();
+ mu2_.Unlock();
+ mu3_.Unlock();
+}
+
+void Foo::unlocklots()
+ UNLOCK_FUNCTION(mu1_, mu2_)
+ UNLOCK_FUNCTION(mu2_, mu3_) {
+ mu1_.Unlock();
+ mu2_.Unlock();
+ mu3_.Unlock();
+}
+
+
+void test0() {
+ Foo foo;
+ foo.lock();
+ foo.unlock();
+
+ foo.lock();
+ foo.lock(); // expected-warning {{locking 'foo' that is already locked}}
+ foo.unlock();
+ foo.unlock(); // expected-warning {{unlocking 'foo' that was not locked}}
+}
+
+
+void test1() {
+ Foo foo;
+ foo.lock1();
+ foo.a = 0;
+ foo.unlock1();
+
+ foo.lock1();
+ foo.lock1(); // expected-warning {{locking 'mu1_' that is already locked}}
+ foo.a = 0;
+ foo.unlock1();
+ foo.unlock1(); // expected-warning {{unlocking 'mu1_' that was not locked}}
+}
+
+
+int test2() {
+ Foo foo;
+ foo.slock1();
+ int d1 = foo.a;
+ foo.unlock1();
+
+ foo.slock1();
+ foo.slock1(); // expected-warning {{locking 'mu1_' that is already locked}}
+ int d2 = foo.a;
+ foo.unlock1();
+ foo.unlock1(); // expected-warning {{unlocking 'mu1_' that was not locked}}
+ return d1 + d2;
+}
+
+
+void test3() {
+ Foo foo;
+ foo.lock3();
+ foo.a = 0;
+ foo.b = 0;
+ foo.c = 0;
+ foo.unlock3();
+
+ foo.lock3();
+ foo.lock3(); // \
+ // expected-warning {{locking 'mu1_' that is already locked}} \
+ // expected-warning {{locking 'mu2_' that is already locked}} \
+ // expected-warning {{locking 'mu3_' that is already locked}}
+ foo.a = 0;
+ foo.b = 0;
+ foo.c = 0;
+ foo.unlock3();
+ foo.unlock3(); // \
+ // expected-warning {{unlocking 'mu1_' that was not locked}} \
+ // expected-warning {{unlocking 'mu2_' that was not locked}} \
+ // expected-warning {{unlocking 'mu3_' that was not locked}}
+}
+
+
+void testlots() {
+ Foo foo;
+ foo.locklots();
+ foo.a = 0;
+ foo.b = 0;
+ foo.c = 0;
+ foo.unlocklots();
+
+ foo.locklots();
+ foo.locklots(); // \
+ // expected-warning {{locking 'mu1_' that is already locked}} \
+ // expected-warning {{locking 'mu2_' that is already locked}} \
+ // expected-warning {{locking 'mu3_' that is already locked}}
+ foo.a = 0;
+ foo.b = 0;
+ foo.c = 0;
+ foo.unlocklots();
+ foo.unlocklots(); // \
+ // expected-warning {{unlocking 'mu1_' that was not locked}} \
+ // expected-warning {{unlocking 'mu2_' that was not locked}} \
+ // expected-warning {{unlocking 'mu3_' that was not locked}}
+}
+
+} // end namespace DuplicateAttributeTest
+
+
+
+
More information about the cfe-commits
mailing list