From: Tom Care <tom.care@uqconnect.edu.au>
Date: Wed, 6 Oct 2010 23:02:25 +0000 (+0000)
Subject: UnreachableCodeChecker cleanup and improvements
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4895b9cf34b26b20e674a88fa8104489e1d06812;p=clang

UnreachableCodeChecker cleanup and improvements
- Fixed some iterator style issues
- Don't process blocks that have been visited already
- Fixed a case where a unreachable block cycle was not reported
- Minor test case changes
- Added one test case from flow-sensitive version of the check. More coming.

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

diff --git a/lib/Checker/UnreachableCodeChecker.cpp b/lib/Checker/UnreachableCodeChecker.cpp
index 52fb0caba0..6854219714 100644
--- a/lib/Checker/UnreachableCodeChecker.cpp
+++ b/lib/Checker/UnreachableCodeChecker.cpp
@@ -91,7 +91,7 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
   ASTContext &Ctx = B.getContext();
 
   // Find CFGBlocks that were not covered by any node
-  for (CFG::const_iterator I = C->begin(); I != C->end(); ++I) {
+  for (CFG::const_iterator I = C->begin(), E = C->end(); I != E; ++I) {
     const CFGBlock *CB = *I;
     // Check if the block is unreachable
     if (reachable.count(CB->getBlockID()))
@@ -102,7 +102,8 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
       continue;
 
     // Find the entry points for this block
-    FindUnreachableEntryPoints(CB);
+    if (!visited.count(CB->getBlockID()))
+      FindUnreachableEntryPoints(CB);
 
     // This block may have been pruned; check if we still want to report it
     if (reachable.count(CB->getBlockID()))
@@ -149,28 +150,19 @@ void UnreachableCodeChecker::VisitEndAnalysis(ExplodedGraph &G,
 
 // Recursively finds the entry point(s) for this dead CFGBlock.
 void UnreachableCodeChecker::FindUnreachableEntryPoints(const CFGBlock *CB) {
-  bool allPredecessorsReachable = true;
-
   visited.insert(CB->getBlockID());
 
-  for (CFGBlock::const_pred_iterator I = CB->pred_begin(); I != CB->pred_end();
-      ++I) {
-    // Recurse over all unreachable blocks
+  for (CFGBlock::const_pred_iterator I = CB->pred_begin(), E = CB->pred_end();
+      I != E; ++I) {
     if (!reachable.count((*I)->getBlockID())) {
-      // At least one predeccessor was unreachable
-      allPredecessorsReachable = false;
-
-      // Only visit the block once
+      // If we find an unreachable predecessor, mark this block as reachable so
+      // we don't report this block
+      reachable.insert(CB->getBlockID());
       if (!visited.count((*I)->getBlockID()))
+        // If we haven't previously visited the unreachable predecessor, recurse
         FindUnreachableEntryPoints(*I);
     }
   }
-
-  // If at least one predecessor is unreachable, mark this block as reachable
-  // so we don't report this block.
-  if (!allPredecessorsReachable) {
-    reachable.insert(CB->getBlockID());
-  }
 }
 
 // Find the Stmt* in a CFGBlock for reporting a warning
diff --git a/test/Analysis/unreachable-code-path.c b/test/Analysis/unreachable-code-path.c
index 071532739c..acc30f4334 100644
--- a/test/Analysis/unreachable-code-path.c
+++ b/test/Analysis/unreachable-code-path.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-experimental-checks -analyzer-check-objc-mem -analyzer-check-dead-stores -verify -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_cc1 -analyze -analyzer-experimental-checks -analyzer-check-objc-mem -analyzer-check-dead-stores -verify -analyzer-opt-analyze-nested-blocks -Wno-unused-value %s
 
 extern void foo(int a);
 
@@ -93,8 +93,8 @@ void test9(unsigned a) {
   switch (a) {
     if (a) // expected-warning{{never executed}}
       foo(a + 5); // no-warning
-    else // no-warning
-      foo(a); // no-warning
+    else          // no-warning
+      foo(a);     // no-warning
     case 1:
     case 2:
       break;
@@ -102,3 +102,23 @@ void test9(unsigned a) {
       break;
   }
 }
+
+// Tests from flow-sensitive version
+void test10() {
+  goto c;
+  d:
+  goto e; // expected-warning {{never executed}}
+  c: ;
+  int i;
+  return;
+  goto b; // expected-warning {{never executed}}
+  goto a; // expected-warning {{never executed}}
+  b:
+  i = 1; // expected-warning {{Value stored to 'i' is never read}}
+  a:
+  i = 2; // expected-warning {{Value stored to 'i' is never read}}
+  goto f;
+  e:
+  goto d;
+  f: ;
+}