From: DeLesley Hutchins Date: Thu, 28 Jun 2012 22:42:48 +0000 (+0000) Subject: Thread safety analysis: support release() function on scoped X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c99a5d820ead4e4f1f4b4a8ab61007b8da0307e6;p=clang Thread safety analysis: support release() function on scoped lockable objects. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159387 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 2668de1b05..fd4551b975 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -349,14 +349,18 @@ struct LockData { /// /// 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 @@ public: 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 Lockset addLocksToSet(const Lockset &LSet, LockKind LK, AttrType *Attr, @@ -986,21 +991,31 @@ Lockset ThreadSafetyAnalyzer::addLock(const Lockset &LSet, /// \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 @@ -1040,14 +1055,18 @@ Lockset ThreadSafetyAnalyzer::addLocksToSet(const Lockset &LSet, 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 @@ Lockset ThreadSafetyAnalyzer::addLocksToSet(const Lockset &LSet, /// 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(FunDecl); if (Attr->args_size() == 0) { // The mutex held is the "this" object. @@ -1068,7 +1089,7 @@ Lockset ThreadSafetyAnalyzer::removeLocksFromSet(const Lockset &LSet, 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 @@ Lockset ThreadSafetyAnalyzer::removeLocksFromSet(const Lockset &LSet, 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 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset &LSet1, 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 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset &LSet1, 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); } } diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 8d231bd7f8..1c47035c2b 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -49,6 +49,13 @@ class __attribute__((scoped_lockable)) ReaderMutexLock { ~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 @@ struct TestScopedLockable { 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); @@ -2361,3 +2368,42 @@ void test3(Bar* b1, Bar* b2) { } // 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 + + + + + +