1 files changed, 76 insertions, 9 deletions

diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp
index d8fca00681b4..4a5d25fc5634 100644
--- a/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -3310,6 +3310,78 @@ static const CFGBlock *findBlockForNode(const ExplodedNode *N) {
   return nullptr;
 }
 
+// Returns true if by simply looking at the block, we can be sure that it
+// results in a sink during analysis. This is useful to know when the analysis
+// was interrupted, and we try to figure out if it would sink eventually.
+// There may be many more reasons why a sink would appear during analysis
+// (eg. checkers may generate sinks arbitrarily), but here we only consider
+// sinks that would be obvious by looking at the CFG.
+static bool isImmediateSinkBlock(const CFGBlock *Blk) {
+  if (Blk->hasNoReturnElement())
+    return true;
+
+  // FIXME: Throw-expressions are currently generating sinks during analysis:
+  // they're not supported yet, and also often used for actually terminating
+  // the program. So we should treat them as sinks in this analysis as well,
+  // at least for now, but once we have better support for exceptions,
+  // we'd need to carefully handle the case when the throw is being
+  // immediately caught.
+  if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement &Elm) {
+        if (Optional<CFGStmt> StmtElm = Elm.getAs<CFGStmt>())
+          if (isa<CXXThrowExpr>(StmtElm->getStmt()))
+            return true;
+        return false;
+      }))
+    return true;
+
+  return false;
+}
+
+// Returns true if by looking at the CFG surrounding the node's program
+// point, we can be sure that any analysis starting from this point would
+// eventually end with a sink. We scan the child CFG blocks in a depth-first
+// manner and see if all paths eventually end up in an immediate sink block.
+static bool isInevitablySinking(const ExplodedNode *N) {
+  const CFG &Cfg = N->getCFG();
+
+  const CFGBlock *StartBlk = findBlockForNode(N);
+  if (!StartBlk)
+    return false;
+  if (isImmediateSinkBlock(StartBlk))
+    return true;
+
+  llvm::SmallVector<const CFGBlock *, 32> DFSWorkList;
+  llvm::SmallPtrSet<const CFGBlock *, 32> Visited;
+
+  DFSWorkList.push_back(StartBlk);
+  while (!DFSWorkList.empty()) {
+    const CFGBlock *Blk = DFSWorkList.back();
+    DFSWorkList.pop_back();
+    Visited.insert(Blk);
+
+    for (const auto &Succ : Blk->succs()) {
+      if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) {
+        if (SuccBlk == &Cfg.getExit()) {
+          // If at least one path reaches the CFG exit, it means that control is
+          // returned to the caller. For now, say that we are not sure what
+          // happens next. If necessary, this can be improved to analyze
+          // the parent StackFrameContext's call site in a similar manner.
+          return false;
+        }
+
+        if (!isImmediateSinkBlock(SuccBlk) && !Visited.count(SuccBlk)) {
+          // If the block has reachable child blocks that aren't no-return,
+          // add them to the worklist.
+          DFSWorkList.push_back(SuccBlk);
+        }
+      }
+    }
+  }
+
+  // Nothing reached the exit. It can only mean one thing: there's no return.
+  return true;
+}
+
 static BugReport *
 FindReportInEquivalenceClass(BugReportEquivClass& EQ,
                              SmallVectorImpl<BugReport*> &bugReports) {
@@ -3360,15 +3432,10 @@ FindReportInEquivalenceClass(BugReportEquivClass& EQ,
 
     // See if we are in a no-return CFG block. If so, treat this similarly
     // to being post-dominated by a sink. This works better when the analysis
-    // is incomplete and we have never reached a no-return function
-    // we're post-dominated by.
-    // This is not quite enough to handle the incomplete analysis case.
-    // We may be post-dominated in subsequent blocks, or even
-    // inter-procedurally. However, it is not clear if more complicated
-    // cases are generally worth suppressing.
-    if (const CFGBlock *B = findBlockForNode(errorNode))
-      if (B->hasNoReturnElement())
-        continue;
+    // is incomplete and we have never reached the no-return function call(s)
+    // that we'd inevitably bump into on this path.
+    if (isInevitablySinking(errorNode))
+      continue;
 
     // At this point we know that 'N' is not a sink and it has at least one
     // successor.  Use a DFS worklist to find a non-sink end-of-path node.