From: Ted Kremenek <kremenek@apple.com>
Date: Wed, 23 Mar 2011 21:33:21 +0000 (+0000)
Subject: Fix CFG-construction bug when run from AnalysisBasedWarnings::IssueWarnings() where... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a8d459e8a68b1270b0c35fb73e8cc090b2b69e36;p=clang

Fix CFG-construction bug when run from AnalysisBasedWarnings::IssueWarnings() where block-level expressions that need
to be recorded in the Stmt*->CFGBlock* map were not always done so.  Fixes <rdar://problem/9171946>.

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

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index a1afd60fe7..c193112131 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -284,6 +284,7 @@ class CFGBuilder {
   Expr::EvalResult *switchCond;
   
   CFG::BuildOptions::ForcedBlkExprs::value_type *cachedEntry;
+  const Stmt *lastLookup;
 
 public:
   explicit CFGBuilder(ASTContext *astContext,
@@ -293,7 +294,7 @@ public:
       SwitchTerminatedBlock(NULL), DefaultCaseBlock(NULL),
       TryTerminatedBlock(NULL), badCFG(false), BuildOpts(buildOpts), 
       switchExclusivelyCovered(false), switchCond(0),
-      cachedEntry(0) {}
+      cachedEntry(0), lastLookup(0) {}
 
   // buildCFG - Used by external clients to construct the CFG.
   CFG* buildCFG(const Decl *D, Stmt *Statement);
@@ -393,11 +394,8 @@ private:
 
   // Interface to CFGBlock - adding CFGElements.
   void appendStmt(CFGBlock *B, Stmt *S) {
-    if (cachedEntry) {
-      assert(cachedEntry->first == S);
+    if (alwaysAdd(S))
       cachedEntry->second = B;
-      cachedEntry = 0;
-    }    
 
     B->appendStmt(S, cfg->getBumpVectorContext());
   }
@@ -459,20 +457,36 @@ inline bool AddStmtChoice::alwaysAdd(CFGBuilder &builder,
                                      const Stmt *stmt) const {
   return builder.alwaysAdd(stmt) || kind == AlwaysAdd;
 }
-  
+
 bool CFGBuilder::alwaysAdd(const Stmt *stmt) {
   if (!BuildOpts.forcedBlkExprs)
     return false;
+
+  if (lastLookup == stmt) {  
+    if (cachedEntry) {
+      assert(cachedEntry->first == stmt);
+      return true;
+    }
+    return false;
+  }
   
+  lastLookup = stmt;
+
+  // Perform the lookup!
   CFG::BuildOptions::ForcedBlkExprs *fb = *BuildOpts.forcedBlkExprs;
 
-  if (!fb)
+  if (!fb) {
+    // No need to update 'cachedEntry', since it will always be null.
+    assert(cachedEntry == 0);
     return false;
+  }
 
   CFG::BuildOptions::ForcedBlkExprs::iterator itr = fb->find(stmt);
-  if (itr == fb->end())
+  if (itr == fb->end()) {
+    cachedEntry = 0;
     return false;
-  
+  }
+
   cachedEntry = &*itr;
   return true;
 }
diff --git a/test/Sema/exprs.c b/test/Sema/exprs.c
index 0d6c5488de..86cd52ec63 100644
--- a/test/Sema/exprs.c
+++ b/test/Sema/exprs.c
@@ -12,6 +12,14 @@
   } while (0)
 
 
+// Test that we don't report divide-by-zero errors in unreachable code.
+// This test should be left as is, as it also tests CFG functionality.
+void radar9171946() {
+  if (0) {
+    0 / (0 ? 1 : 0); // expected-warning {{expression result unused}}
+  }
+}
+
 int test_pr8876() {
   PR8876(0); // no-warning
   PR8876_pos(0); // expected-warning{{indirection of non-volatile null pointer will be deleted, not trap}} expected-note{{consider using __builtin_trap() or qualifying pointer with 'volatile'}}