[cfe-commits] r125419 - in /cfe/trunk: include/clang/AST/ParentMap.h lib/AST/ParentMap.cpp lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp test/Analysis/dead-stores.c test/Analysis/dead-stores.cpp

[Ted Kremenek]

Author: kremenek
Date: Fri Feb 11 18:17:19 2011
New Revision: 125419

URL: http://llvm.org/viewvc/llvm-project?rev=125419&view=rev
Log:
Don't emit a dead store for '++' operations unless it occurs with a return statement.  We've never seen any other cases that were real bugs.

Fixes <rdar://problem/6962292>.

Modified:
    cfe/trunk/include/clang/AST/ParentMap.h
    cfe/trunk/lib/AST/ParentMap.cpp
    cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
    cfe/trunk/test/Analysis/dead-stores.c
    cfe/trunk/test/Analysis/dead-stores.cpp

Modified: cfe/trunk/include/clang/AST/ParentMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ParentMap.h?rev=125419&r1=125418&r2=125419&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ParentMap.h (original)
+++ cfe/trunk/include/clang/AST/ParentMap.h Fri Feb 11 18:17:19 2011
@@ -31,6 +31,7 @@
 
   Stmt *getParent(Stmt*) const;
   Stmt *getParentIgnoreParens(Stmt *) const;
+  Stmt *getParentIgnoreParenCasts(Stmt *) const;
 
   const Stmt *getParent(const Stmt* S) const {
     return getParent(const_cast<Stmt*>(S));
@@ -40,6 +41,10 @@
     return getParentIgnoreParens(const_cast<Stmt*>(S));
   }
 
+  const Stmt *getParentIgnoreParenCasts(const Stmt *S) const {
+    return getParentIgnoreParenCasts(const_cast<Stmt*>(S));
+  }
+
   bool hasParent(Stmt* S) const {
     return getParent(S) != 0;
   }

Modified: cfe/trunk/lib/AST/ParentMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ParentMap.cpp?rev=125419&r1=125418&r2=125419&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ParentMap.cpp (original)
+++ cfe/trunk/lib/AST/ParentMap.cpp Fri Feb 11 18:17:19 2011
@@ -57,6 +57,15 @@
   return S;
 }
 
+Stmt *ParentMap::getParentIgnoreParenCasts(Stmt *S) const {
+  do {
+    S = getParent(S);
+  }
+  while (S && (isa<ParenExpr>(S) || isa<CastExpr>(S)));
+
+  return S;  
+}
+
 bool ParentMap::isConsumedExpr(Expr* E) const {
   Stmt *P = getParent(E);
   Stmt *DirectChild = E;

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp?rev=125419&r1=125418&r2=125419&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp Fri Feb 11 18:17:19 2011
@@ -191,6 +191,8 @@
     if (S->getLocStart().isMacroID())
       return;
 
+    // Only cover dead stores from regular assignments.  ++/-- dead stores
+    // have never flagged a real bug.
     if (BinaryOperator* B = dyn_cast<BinaryOperator>(S)) {
       if (!B->isAssignmentOp()) return; // Skip non-assignments.
 
@@ -221,14 +223,11 @@
         }
     }
     else if (UnaryOperator* U = dyn_cast<UnaryOperator>(S)) {
-      if (!U->isIncrementOp())
+      if (!U->isIncrementOp() || U->isPrefix())
         return;
 
-      // Handle: ++x within a subexpression.  The solution is not warn
-      //  about preincrements to dead variables when the preincrement occurs
-      //  as a subexpression.  This can lead to false negatives, e.g. "(++x);"
-      //  A generalized dead code checker should find such issues.
-      if (U->isPrefix() && Parents.isConsumedExpr(U))
+      Stmt *parent = Parents.getParentIgnoreParenCasts(U);
+      if (!parent || !isa<ReturnStmt>(parent))
         return;
 
       Expr *Ex = U->getSubExpr()->IgnoreParenCasts();

Modified: cfe/trunk/test/Analysis/dead-stores.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.c?rev=125419&r1=125418&r2=125419&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.c (original)
+++ cfe/trunk/test/Analysis/dead-stores.c Fri Feb 11 18:17:19 2011
@@ -44,10 +44,11 @@
 
 }
 
+//
 int f6() {
   
   int x = 4;
-  ++x; // expected-warning{{never read}}
+  ++x; // no-warning
   return 1;
 }
 
@@ -231,7 +232,7 @@
 int f21() {
   int x = 4;
   
-  ++x; // expected-warning{{never read}}
+  x = x + 1; // expected-warning{{never read}}
   if (1) {
     halt();
     (void)x;
@@ -263,7 +264,7 @@
   int y19 = 4;
   int y20 = 4;
 
-  ++x; // expected-warning{{never read}}
+  x = x + 1; // expected-warning{{never read}}
   ++y1;
   ++y2;
   ++y3;

Modified: cfe/trunk/test/Analysis/dead-stores.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.cpp?rev=125419&r1=125418&r2=125419&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.cpp (original)
+++ cfe/trunk/test/Analysis/dead-stores.cpp Fri Feb 11 18:17:19 2011
@@ -12,7 +12,7 @@
 void test1() {
   int x = 4;
 
-  ++x; // expected-warning{{never read}}
+  x = x + 1; // expected-warning{{never read}}
 
   switch (j) {
   case 1:
@@ -69,11 +69,11 @@
 //===----------------------------------------------------------------------===//
 
 void test3_a(int x) {
-  ++x; // expected-warning{{never read}}
+   x = x + 1; // expected-warning{{never read}}
 }
 
 void test3_b(int &x) {
-  ++x; // no-warninge
+  x = x + 1; // no-warninge
 }
 
 void test3_c(int x) {