On Thu, Jun 28, 2012 at 3:42 PM, DeLesley Hutchins <[email protected]>wrote:
> Author: delesley > Date: Thu Jun 28 17:42:48 2012 > New Revision: 159387 > > URL: http://llvm.org/viewvc/llvm-project?rev=159387&view=rev > Log: > Thread safety analysis: support release() function on scoped > lockable objects. > > 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=159387&r1=159386&r2=159387&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original) > +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Jun 28 17:42:48 2012 > @@ -349,14 +349,18 @@ > /// > /// FIXME: add support for re-entrant locking and lock up/downgrading > LockKind LKind; > - MutexID UnderlyingMutex; // for ScopedLockable objects > + bool Managed; // for ScopedLockable objects > + MutexID UnderlyingMutex; // for ScopedLockable objects > > - LockData(SourceLocation AcquireLoc, LockKind LKind) > - : AcquireLoc(AcquireLoc), LKind(LKind), > UnderlyingMutex(Decl::EmptyShell()) > + LockData(SourceLocation AcquireLoc, LockKind LKind, bool M = false) > + : AcquireLoc(AcquireLoc), LKind(LKind), Managed(M), > + UnderlyingMutex(Decl::EmptyShell()) > {} > > LockData(SourceLocation AcquireLoc, LockKind LKind, const MutexID &Mu) > - : AcquireLoc(AcquireLoc), LKind(LKind), UnderlyingMutex(Mu) {} > + : AcquireLoc(AcquireLoc), LKind(LKind), Managed(false), > + UnderlyingMutex(Mu) > + {} > > bool operator==(const LockData &other) const { > return AcquireLoc == other.AcquireLoc && LKind == other.LKind; > @@ -924,7 +928,8 @@ > Lockset addLock(const Lockset &LSet, Expr *MutexExp, const NamedDecl *D, > const LockData &LDat); > Lockset removeLock(const Lockset &LSet, const MutexID &Mutex, > - SourceLocation UnlockLoc); > + SourceLocation UnlockLoc, > + bool Warn=true, bool FullyRemove=false); > > template <class AttrType> > Lockset addLocksToSet(const Lockset &LSet, LockKind LK, AttrType *Attr, > @@ -986,21 +991,31 @@ > /// \param UnlockLoc The source location of the unlock (only used in > error msg) > Lockset ThreadSafetyAnalyzer::removeLock(const Lockset &LSet, > const MutexID &Mutex, > - SourceLocation UnlockLoc) { > + SourceLocation UnlockLoc, > + bool Warn, bool FullyRemove) { > const LockData *LDat = LSet.lookup(Mutex); > if (!LDat) { > - Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc); > + if (Warn) > + Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc); > return LSet; > } > else { > Lockset Result = LSet; > - // For scoped-lockable vars, remove the mutex associated with this > var. > - if (LDat->UnderlyingMutex.isValid()) > - Result = removeLock(Result, LDat->UnderlyingMutex, UnlockLoc); > + 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; > + } > return LocksetFactory.remove(Result, 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> > @@ -1040,14 +1055,18 @@ > if (!Mutex.isValid()) > MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl); > else { > - Result = addLock(Result, Mutex, LockData(ExpLocation, LK)); > 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; > @@ -1057,9 +1076,11 @@ > /// arguments from the lockset. > Lockset ThreadSafetyAnalyzer::removeLocksFromSet(const Lockset &LSet, > UnlockFunctionAttr *Attr, > - Expr *Exp, NamedDecl* > FunDecl) { > + 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. > @@ -1068,7 +1089,7 @@ > MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl); > return LSet; > } else { > - return removeLock(LSet, Mu, ExpLocation); > + return removeLock(LSet, Mu, ExpLocation, true, Dtor); > } > } > > @@ -1079,7 +1100,7 @@ > if (!Mutex.isValid()) > MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl); > else > - Result = removeLock(Result, Mutex, ExpLocation); > + Result = removeLock(Result, Mutex, ExpLocation, true, Dtor); > } > return Result; > } > @@ -1537,9 +1558,10 @@ > LSet2LockData); > } > } else { > - Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(), > - LSet2LockData.AcquireLoc, > - JoinLoc, LEK); > + if (!LSet2LockData.Managed) > + Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(), > + LSet2LockData.AcquireLoc, > + JoinLoc, LEK); > } > } > > @@ -1547,9 +1569,11 @@ > if (!LSet2.contains(I.getKey())) { > const MutexID &Mutex = I.getKey(); > const LockData &MissingLock = I.getData(); > - Handler.handleMutexHeldEndOfScope(Mutex.getName(), > - MissingLock.AcquireLoc, > - JoinLoc, LEK); > + > + if (!MissingLock.Managed) > + Handler.handleMutexHeldEndOfScope(Mutex.getName(), > + MissingLock.AcquireLoc, > + JoinLoc, LEK); > Intersection = LocksetFactory.remove(Intersection, Mutex); > } > } > > 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=159387&r1=159386&r2=159387&view=diff > > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original) > +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Jun 28 > 17:42:48 2012 > @@ -49,6 +49,13 @@ > ~ReaderMutexLock() __attribute__((unlock_function)); > }; > > +class SCOPED_LOCKABLE ReleasableMutexLock { > + public: > + ReleasableMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu); > + ~ReleasableMutexLock() UNLOCK_FUNCTION(); > + > + void Release() UNLOCK_FUNCTION(); > +}; > > Mutex sls_mu; > > @@ -1578,7 +1585,7 @@ > MutexLock mulock_a(&mu1); > MutexLock mulock_b(&mu1); // \ > // expected-warning {{locking 'mu1' that is already locked}} > - } // expected-warning {{unlocking 'mu1' that was not locked}} > + } > This seems like a distinct bug fix from the new feature in the commit log? Or am I misunderstanding what's changing here? (entirely possible) > > void foo4() { > MutexLock mulock1(&mu1), mulock2(&mu2); > @@ -2361,3 +2368,42 @@ > } // end namespace LockReturned > > > +namespace ReleasableScopedLock { > + > +class Foo { > + Mutex mu_; > + bool c; > + int a GUARDED_BY(mu_); > + > + void test1(); > + void test2(); > + void test3(); > +}; > + > + > +void Foo::test1() { > + ReleasableMutexLock rlock(&mu_); > + rlock.Release(); > +} > + > +void Foo::test2() { > + ReleasableMutexLock rlock(&mu_); > + if (c) { // test join point -- held/not held during release > + rlock.Release(); > + } > +} > + > +void Foo::test3() { > + ReleasableMutexLock rlock(&mu_); > + a = 0; > + rlock.Release(); > + a = 1; // expected-warning {{writing variable 'a' requires locking > 'mu_' exclusively}} > +} > + > +} // end namespace ReleasableScopedLock > + > + > + > + > + > + > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
