[cfe-commits] r128170 - in /cfe/trunk: lib/Analysis/CFG.cpp test/Sema/exprs.c

[Ted Kremenek]

Author: kremenek
Date: Wed Mar 23 16:33:21 2011
New Revision: 128170

URL: http://llvm.org/viewvc/llvm-project?rev=128170&view=rev
Log:
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>.

Modified:
    cfe/trunk/lib/Analysis/CFG.cpp
    cfe/trunk/test/Sema/exprs.c

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=128170&r1=128169&r2=128170&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Wed Mar 23 16:33:21 2011
@@ -284,6 +284,7 @@
   Expr::EvalResult *switchCond;
   
   CFG::BuildOptions::ForcedBlkExprs::value_type *cachedEntry;
+  const Stmt *lastLookup;
 
 public:
   explicit CFGBuilder(ASTContext *astContext,
@@ -293,7 +294,7 @@
       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 @@
 
   // 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 @@
                                      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;
 }

Modified: cfe/trunk/test/Sema/exprs.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/exprs.c?rev=128170&r1=128169&r2=128170&view=diff
==============================================================================
--- cfe/trunk/test/Sema/exprs.c (original)
+++ cfe/trunk/test/Sema/exprs.c Wed Mar 23 16:33:21 2011
@@ -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'}}