[cfe-commits] r159609 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp
DeLesley Hutchins
delesley at google.com
Mon Jul 2 15:26:29 PDT 2012
Author: delesley
Date: Mon Jul 2 17:26:29 2012
New Revision: 159609
URL: http://llvm.org/viewvc/llvm-project?rev=159609&view=rev
Log:
Thread safety analysis: fixed bug that occurs when very silly people
use scoped_lockable without putting unlock_function on the
destructor.
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=159609&r1=159608&r2=159609&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Jul 2 17:26:29 2012
@@ -1558,20 +1558,29 @@
for (Lockset::iterator I = LSet2.begin(), E = LSet2.end(); I != E; ++I) {
const MutexID &LSet2Mutex = I.getKey();
- const LockData &LSet2LockData = I.getData();
- if (const LockData *LD = LSet1.lookup(LSet2Mutex)) {
- if (LD->LKind != LSet2LockData.LKind) {
+ const LockData &LDat2 = I.getData();
+ if (const LockData *LDat1 = LSet1.lookup(LSet2Mutex)) {
+ if (LDat1->LKind != LDat2.LKind) {
Handler.handleExclusiveAndShared(LSet2Mutex.getName(),
- LSet2LockData.AcquireLoc,
- LD->AcquireLoc);
- if (LD->LKind != LK_Exclusive)
- Intersection = LocksetFactory.add(Intersection, LSet2Mutex,
- LSet2LockData);
+ LDat2.AcquireLoc,
+ LDat1->AcquireLoc);
+ if (LDat1->LKind != LK_Exclusive)
+ Intersection = LocksetFactory.add(Intersection, LSet2Mutex, LDat2);
}
} else {
- if (!LSet2LockData.Managed)
+ if (LDat2.UnderlyingMutex.isValid()) {
+ if (LSet2.lookup(LDat2.UnderlyingMutex)) {
+ // If this is a scoped lock that manages another mutex, and if the
+ // underlying mutex is still held, then warn about the underlying
+ // mutex.
+ Handler.handleMutexHeldEndOfScope(LDat2.UnderlyingMutex.getName(),
+ LDat2.AcquireLoc,
+ JoinLoc, LEK1);
+ }
+ }
+ else if (!LDat2.Managed)
Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(),
- LSet2LockData.AcquireLoc,
+ LDat2.AcquireLoc,
JoinLoc, LEK1);
}
}
@@ -1579,11 +1588,21 @@
for (Lockset::iterator I = LSet1.begin(), E = LSet1.end(); I != E; ++I) {
if (!LSet2.contains(I.getKey())) {
const MutexID &Mutex = I.getKey();
- const LockData &MissingLock = I.getData();
+ const LockData &LDat1 = I.getData();
- if (!MissingLock.Managed)
+ if (LDat1.UnderlyingMutex.isValid()) {
+ if (LSet1.lookup(LDat1.UnderlyingMutex)) {
+ // If this is a scoped lock that manages another mutex, and if the
+ // underlying mutex is still held, then warn about the underlying
+ // mutex.
+ Handler.handleMutexHeldEndOfScope(LDat1.UnderlyingMutex.getName(),
+ LDat1.AcquireLoc,
+ JoinLoc, LEK1);
+ }
+ }
+ else if (!LDat1.Managed)
Handler.handleMutexHeldEndOfScope(Mutex.getName(),
- MissingLock.AcquireLoc,
+ LDat1.AcquireLoc,
JoinLoc, LEK2);
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=159609&r1=159608&r2=159609&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Jul 2 17:26:29 2012
@@ -2470,3 +2470,68 @@
} // end namespace UnlockBug
+namespace FoolishScopedLockableBug {
+
+class SCOPED_LOCKABLE WTF_ScopedLockable {
+public:
+ WTF_ScopedLockable(Mutex* mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+
+ // have to call release() manually;
+ ~WTF_ScopedLockable();
+
+ void release() UNLOCK_FUNCTION();
+};
+
+
+class Foo {
+ Mutex mu_;
+ int a GUARDED_BY(mu_);
+ bool c;
+
+ void doSomething();
+
+ void test1() {
+ WTF_ScopedLockable wtf(&mu_);
+ wtf.release();
+ }
+
+ void test2() {
+ WTF_ScopedLockable wtf(&mu_); // expected-note {{mutex acquired here}}
+ } // expected-warning {{mutex 'mu_' is still locked at the end of function}}
+
+ void test3() {
+ if (c) {
+ WTF_ScopedLockable wtf(&mu_);
+ wtf.release();
+ }
+ }
+
+ void test4() {
+ if (c) {
+ doSomething();
+ }
+ else {
+ WTF_ScopedLockable wtf(&mu_);
+ wtf.release();
+ }
+ }
+
+ void test5() {
+ if (c) {
+ WTF_ScopedLockable wtf(&mu_); // expected-note {{mutex acquired here}}
+ }
+ } // expected-warning {{mutex 'mu_' is not locked on every path through here}}
+
+ void test6() {
+ if (c) {
+ doSomething();
+ }
+ else {
+ WTF_ScopedLockable wtf(&mu_); // expected-note {{mutex acquired here}}
+ }
+ } // expected-warning {{mutex 'mu_' is not locked on every path through here}}
+};
+
+
+} // end namespace FoolishScopedLockableBug
+
More information about the cfe-commits
mailing list