From 5381c05f51e5b7c7627f1d95b9a3425303ce086a Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Thu, 5 Jul 2012 21:16:29 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159780 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ThreadSafety.cpp | 371 ++++++++++--------- test/SemaCXX/warn-thread-safety-analysis.cpp | 200 +++++++++- 2 files changed, 385 insertions(+), 186 deletions(-) diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index fb53d076a4..a912aa9796 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -356,6 +356,25 @@ public: }; +/// \brief A short list of MutexIDs +class MutexIDList : public SmallVector { +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 @@ public: 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 - 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 + void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp, + const NamedDecl *D); template - 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 @@ public: /// \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::addLock(const Lockset &LSet, 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 -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(FunDecl)) { - CXXRecordDecl* PD = CD->getParent(); - if (PD && PD->getAttr()) - 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(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 -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(BrE)) { @@ -1156,16 +1098,14 @@ Lockset ThreadSafetyAnalyzer::addTrylock(const Lockset &LSet, 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 @@ Lockset ThreadSafetyAnalyzer::getEdgeLockset(const Lockset &ExitSet, const CFGBlockInfo *PredBlockInfo = &BlockInfo[PredBlock->getBlockID()]; const LocalVarContext &LVarCtx = PredBlockInfo->ExitContext; - CallExpr *Exp = const_cast( - getTrylockCallExpr(Cond, LVarCtx, Negate)); + CallExpr *Exp = + const_cast(getTrylockCallExpr(Cond, LVarCtx, Negate)); if (!Exp) return ExitSet; @@ -1222,7 +1162,9 @@ Lockset ThreadSafetyAnalyzer::getEdgeLockset(const Lockset &ExitSet, 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 @@ Lockset ThreadSafetyAnalyzer::getEdgeLockset(const Lockset &ExitSet, case attr::ExclusiveTrylockFunction: { ExclusiveTrylockFunctionAttr *A = cast(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(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 { 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 @@ void BuildLockset::checkAccess(Expr *Exp, AccessKind AK) { /// 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(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(Attr); - LSet = Analyzer->addLocksToSet(LSet, LK_Exclusive, A, Exp, D, VD); + ExclusiveLockFunctionAttr *A = cast(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(Attr); - LSet = Analyzer->addLocksToSet(LSet, LK_Shared, A, Exp, D, VD); + SharedLockFunctionAttr *A = cast(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(Attr); - LSet = Analyzer->removeLocksFromSet(LSet, UFAttr, Exp, D); + UnlockFunctionAttr *A = cast(At); + Analyzer->getMutexIDs(LocksToRemove, A, Exp, D); break; } case attr::ExclusiveLocksRequired: { - ExclusiveLocksRequiredAttr *ELRAttr = - cast(Attr); + ExclusiveLocksRequiredAttr *A = cast(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(Attr); + SharedLocksRequiredAttr *A = cast(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(Attr); - for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(), - E = LEAttr->args_end(); I != E; ++I) { + LocksExcludedAttr *A = cast(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 @@ void BuildLockset::handleCall(Expr *Exp, NamedDecl *D, VarDecl *VD) { break; } } + + // Figure out if we're calling the constructor of scoped lockable class + bool isScopedVar = false; + if (VD) { + if (const CXXConstructorDecl *CD = dyn_cast(D)) { + const CXXRecordDecl* PD = CD->getParent(); + if (PD && PD->getAttr()) + isScopedVar = true; + } + } + + // Add locks. + SourceLocation Loc = Exp->getExprLoc(); + for (unsigned i=0,n=ExclusiveLocksToAdd.size(); iaddLock(LSet, ExclusiveLocksToAdd[i], + LockData(Loc, LK_Exclusive, isScopedVar)); + } + for (unsigned i=0,n=SharedLocksToAdd.size(); iaddLock(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(); iaddLock(LSet, SMutex, LockData(MLoc, LK_Exclusive, + ExclusiveLocksToAdd[i])); + } + for (unsigned i=0,n=SharedLocksToAdd.size(); iaddLock(LSet, SMutex, LockData(MLoc, LK_Shared, + SharedLocksToAdd[i])); + } + } + + // Remove locks. + // FIXME -- should only fully remove if the attribute refers to 'this'. + bool Dtor = isa(D); + for (unsigned i=0,n=LocksToRemove.size(); iremoveLock(LSet, LocksToRemove[i], Loc, Dtor); + } } @@ -1636,6 +1633,7 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset &LSet1, } + /// \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 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { 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(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(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(Attr)) { + getMutexIDs(ExclusiveLocksToAdd, A, (Expr*) 0, D); + } else if (SharedLocksRequiredAttr *A + = dyn_cast(Attr)) { + getMutexIDs(SharedLocksToAdd, A, (Expr*) 0, D); } else if (isa(Attr)) { // Don't try to check unlock functions for now return; @@ -1717,6 +1712,16 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { return; } } + + // FIXME -- Loc can be wrong here. + for (unsigned i=0,n=ExclusiveLocksToAdd.size(); ibegin(), diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 9f01223f03..10f9ad2736 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -2395,6 +2395,8 @@ class Foo { void test1(); void test2(); void test3(); + void test4(); + void test5(); }; @@ -2417,6 +2419,22 @@ void Foo::test3() { 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 @@ public: 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 @@ public: } // end namespace UnlockBug + namespace FoolishScopedLockableBug { class SCOPED_LOCKABLE WTF_ScopedLockable { @@ -2553,6 +2578,7 @@ class Foo { } // end namespace FoolishScopedLockableBug + namespace TemporaryCleanupExpr { class Foo { @@ -2739,4 +2765,172 @@ void Bar::test3() { +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 + + + + -- 2.40.0