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

[Richard Smith]

Author: rsmith
Date: Thu Feb  2 21:30:07 2012
New Revision: 149666

URL: http://llvm.org/viewvc/llvm-project?rev=149666&view=rev
Log:
Thread safety analysis: at a CFG join point between a block terminating in a
'continue' and another block, prefer the lockset from the other block, and
diagnose the 'continue' block as being the end of a loop.

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=149666&r1=149665&r2=149666&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Feb  2 21:30:07 2012
@@ -1429,6 +1429,7 @@
     // union because the real error is probably that we forgot to unlock M on
     // all code paths.
     bool LocksetInitialized = false;
+    llvm::SmallVector<CFGBlock*, 8> SpecialBlocks;
     for (CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(),
          PE  = CurrBlock->pred_end(); PI != PE; ++PI) {
 
@@ -1436,6 +1437,17 @@
       if (*PI == 0 || !VisitedBlocks.alreadySet(*PI))
         continue;
 
+      // If the previous block ended in a 'continue' or 'break' statement, then
+      // a difference in locksets is probably due to a bug in that block, rather
+      // than in some other predecessor. In that case, keep the other
+      // predecessor's lockset.
+      if (const Stmt *Terminator = (*PI)->getTerminator()) {
+        if (isa<ContinueStmt>(Terminator) || isa<BreakStmt>(Terminator)) {
+          SpecialBlocks.push_back(*PI);
+          continue;
+        }
+      }
+
       int PrevBlockID = (*PI)->getBlockID();
       CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
 
@@ -1449,6 +1461,33 @@
       }
     }
 
+    // Process continue and break blocks. Assume that the lockset for the
+    // resulting block is unaffected by any discrepancies in them.
+    for (unsigned SpecialI = 0, SpecialN = SpecialBlocks.size();
+         SpecialI < SpecialN; ++SpecialI) {
+      CFGBlock *PrevBlock = SpecialBlocks[SpecialI];
+      int PrevBlockID = PrevBlock->getBlockID();
+      CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
+
+      if (!LocksetInitialized) {
+        CurrBlockInfo->EntrySet = PrevBlockInfo->ExitSet;
+        LocksetInitialized = true;
+      } else {
+        // Determine whether this edge is a loop terminator for diagnostic
+        // purposes. FIXME: A 'break' statement might be a loop terminator, but
+        // it might also be part of a switch. Also, a subsequent destructor
+        // might add to the lockset, in which case the real issue might be a
+        // double lock on the other path.
+        const Stmt *Terminator = PrevBlock->getTerminator();
+        bool IsLoop = Terminator && isa<ContinueStmt>(Terminator);
+
+        // Do not update EntrySet.
+        intersectAndWarn(CurrBlockInfo->EntrySet, PrevBlockInfo->ExitSet,
+                         IsLoop ? LEK_LockedSomeLoopIterations
+                                : LEK_LockedSomePredecessors);
+      }
+    }
+
     BuildLockset LocksetBuilder(this, *CurrBlockInfo);
     CFGBlock::const_pred_iterator PI = CurrBlock->pred_begin(),
                                   PE = CurrBlock->pred_end();

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=149666&r1=149665&r2=149666&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Feb  2 21:30:07 2012
@@ -208,8 +208,7 @@
 }
 
 void sls_fun_bad_7() {
-  sls_mu.Lock(); // \
-    // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+  sls_mu.Lock();
   while (getBool()) {
     sls_mu.Unlock();
     if (getBool()) {
@@ -218,7 +217,7 @@
       }
     }
     sls_mu.Lock(); // \
-      // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+      // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
   }
   sls_mu.Unlock();
 }
@@ -257,6 +256,21 @@
     // expected-warning{{unlocking 'sls_mu' that was not locked}}
 }
 
+void sls_fun_bad_12() {
+  sls_mu.Lock(); // \
+    // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+  while (getBool()) {
+    sls_mu.Unlock();
+    if (getBool()) {
+      if (getBool()) {
+        break;
+      }
+    }
+    sls_mu.Lock();
+  }
+  sls_mu.Unlock();
+}
+
 //-----------------------------------------//
 // Handling lock expressions in attribute args
 // -------------------------------------------//
@@ -1900,4 +1914,3 @@
 }
 
 };
-