Thread-safety analysis: better handling of unreachable blocks. Fixes a bug

where a call to function marked 'noreturn' is followed by unreachable
implicit destructor calls.

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

lib/Analysis/ThreadSafety.cpp

@@ -892,6 +892,7 @@ struct CFGBlockInfo {
   SourceLocation EntryLoc;      // Location of first statement in block
   SourceLocation ExitLoc;       // Location of last statement in block.
   unsigned EntryIndex;          // Used to replay contexts later
+  bool Reachable;               // Is this block reachable?
 
   const FactSet &getSet(CFGBlockSide Side) const {
     return Side == CBS_Entry ? EntrySet : ExitSet;
@@ -902,7 +903,7 @@ struct CFGBlockInfo {
 
 private:
   CFGBlockInfo(LocalVarContext EmptyCtx)
-    : EntryContext(EmptyCtx), ExitContext(EmptyCtx)
+    : EntryContext(EmptyCtx), ExitContext(EmptyCtx), Reachable(false)
   { }
 
 public:
@@ -2183,6 +2184,9 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   PostOrderCFGView *SortedGraph = AC.getAnalysis<PostOrderCFGView>();
   PostOrderCFGView::CFGBlockSet VisitedBlocks(CFGraph);
 
+  // Mark entry block as reachable
+  BlockInfo[CFGraph->getEntry().getBlockID()].Reachable = true;
+
   // Compute SSA names for local variables
   LocalVarMap.traverseCFG(CFGraph, SortedGraph, BlockInfo);
 
@@ -2270,10 +2274,16 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
       if (*PI == 0 || !VisitedBlocks.alreadySet(*PI))
         continue;
 
+      int PrevBlockID = (*PI)->getBlockID();
+      CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
+
       // Ignore edges from blocks that can't return.
-      if ((*PI)->hasNoReturnElement())
+      if ((*PI)->hasNoReturnElement() || !PrevBlockInfo->Reachable)
         continue;
 
+      // Okay, we can reach this block from the entry.
+      CurrBlockInfo->Reachable = true;
+
       // 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
@@ -2285,8 +2295,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         }
       }
 
-      int PrevBlockID = (*PI)->getBlockID();
-      CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
+
       FactSet PrevLockset;
       getEdgeLockset(PrevLockset, PrevBlockInfo->ExitSet, *PI, CurrBlock);
 
@@ -2300,6 +2309,10 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
       }
     }
 
+    // Skip rest of block if it's not reachable.
+    if (!CurrBlockInfo->Reachable)
+      continue;
+
     // 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();
@@ -2386,23 +2399,13 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
     }
   }
 
-
-  // Check to make sure that the exit block is reachable
-  bool ExitUnreachable = true;
-  for (CFGBlock::const_pred_iterator PI = CFGraph->getExit().pred_begin(),
-       PE  = CFGraph->getExit().pred_end(); PI != PE; ++PI) {
-    if (!(*PI)->hasNoReturnElement()) {
-      ExitUnreachable = false;
-      break;
-    }
-  }
-  // Skip the final check if the exit block is unreachable.
-  if (ExitUnreachable)
-    return;
-
   CFGBlockInfo *Initial = &BlockInfo[CFGraph->getEntry().getBlockID()];
   CFGBlockInfo *Final   = &BlockInfo[CFGraph->getExit().getBlockID()];
 
+  // Skip the final check if the exit block is unreachable.
+  if (!Final->Reachable)
+    return;
+
   // FIXME: Should we call this function for all blocks which exit the function?
   intersectAndWarn(Initial->EntrySet, Final->ExitSet,
                    Final->ExitLoc,

test/SemaCXX/warn-thread-safety-analysis.cpp

@@ -64,6 +64,7 @@ void beginNoWarnOnWrites() EXCLUSIVE_LOCK_FUNCTION("*");
 void endNoWarnOnWrites()   UNLOCK_FUNCTION("*");
 
 
+// For testing handling of smart pointers.
 template<class T>
 class SmartPtr {
 public:
@@ -80,6 +81,15 @@ private:
 };
 
 
+// For testing destructor calls and cleanup.
+class MyString {
+public:
+  MyString(const char* s);
+  ~MyString();
+};
+
+
+
 Mutex sls_mu;
 
 Mutex sls_mu2 __attribute__((acquired_after(sls_mu)));
@@ -3250,12 +3260,6 @@ void Base::bar(Inner* i) {
 
 namespace TrylockWithCleanups {
 
-class MyString {
-public:
-  MyString(const char* s);
-  ~MyString();
-};
-
 struct Foo {
   Mutex mu_;
   int a GUARDED_BY(mu_);
@@ -3460,6 +3464,7 @@ public:
 };
 
 void exitNow() __attribute__((noreturn));
+void exitDestruct(const MyString& ms) __attribute__((noreturn));
 
 Mutex fatalmu_;
 
@@ -3482,6 +3487,10 @@ void test3() EXCLUSIVE_LOCKS_REQUIRED(fatalmu_) {
   }
 }
 
+void test4() EXCLUSIVE_LOCKS_REQUIRED(fatalmu_) {
+  exitDestruct("foo");
+}
+
 }   // end namespace UnreachableExitTest