From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Fri, 3 Feb 2012 03:30:07 +0000 (+0000)
Subject: Thread safety analysis: at a CFG join point between a block terminating in a
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=aacde7174af6c5759b52dc0ceb7b167d323afb6a;p=clang

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.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@149666 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 6f76f1b947..69ea830bdd 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -1429,6 +1429,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
     // 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 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
       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 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
       }
     }
 
+    // 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();
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index dfec94ca91..634a233430 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -208,8 +208,7 @@ void sls_fun_bad_6() {
 }
 
 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 @@ void sls_fun_bad_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 @@ void sls_fun_bad_11() {
     // 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 @@ void test() {
 }
 
 };
-