[cfe-commits] r159387 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
Chandler Carruth
chandlerc at google.com
Thu Jun 28 15:48:12 PDT 2012
On Thu, Jun 28, 2012 at 3:42 PM, DeLesley Hutchins <delesley at google.com>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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120628/49ee6131/attachment.html>
More information about the cfe-commits
mailing list