[cfe-commits] r159609 - in /cfe/trunk: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp

[DeLesley Hutchins]

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
+