[cfe-commits] r111832 - in /cfe/trunk: include/clang/Analysis/Analyses/PseudoConstantAnalysis.h include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h include/clang/Analysis/AnalysisContext.h lib/Analysis/AnalysisContext.cpp lib/Analysis/CMakeLists.txt lib/Analysis/PseudoConstantAnalysis.cpp lib/Analysis/PsuedoConstantAnalysis.cpp lib/Checker/IdempotentOperationChecker.cpp test/Analysis/idempotent-operations.c

Tom Care tcare at apple.com
Mon Aug 23 12:51:58 PDT 2010


Author: tcare
Date: Mon Aug 23 14:51:57 2010
New Revision: 111832

URL: http://llvm.org/viewvc/llvm-project?rev=111832&view=rev
Log:
Several small changes to PseudoConstantAnalysis and the way IdempotentOperationChecker uses it.
- Psuedo -> Pseudo (doh...)
- C++ reference support
- Added pseudoconstant test case for __block vars
- Separated out static local checking from pseudoconstant analysis and generalized to non-local checking
- Added missing test cases for storage false positives

Added:
    cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
    cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
Removed:
    cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h
    cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp
Modified:
    cfe/trunk/include/clang/Analysis/AnalysisContext.h
    cfe/trunk/lib/Analysis/AnalysisContext.cpp
    cfe/trunk/lib/Analysis/CMakeLists.txt
    cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
    cfe/trunk/test/Analysis/idempotent-operations.c

Added: cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h?rev=111832&view=auto
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h (added)
+++ cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h Mon Aug 23 14:51:57 2010
@@ -0,0 +1,42 @@
+//== PseudoConstantAnalysis.h - Find Pseudo-constants in the AST -*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file tracks the usage of variables in a Decl body to see if they are
+// never written to, implying that they constant. This is useful in static
+// analysis to see if a developer might have intended a variable to be const.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_PSEUDOCONSTANTANALYSIS
+#define LLVM_CLANG_ANALYSIS_PSEUDOCONSTANTANALYSIS
+
+#include "clang/AST/Stmt.h"
+
+namespace clang {
+
+class PseudoConstantAnalysis {
+public:
+  PseudoConstantAnalysis(const Stmt *DeclBody);
+  ~PseudoConstantAnalysis();
+
+  bool isPseudoConstant(const VarDecl *VD);
+
+private:
+  void RunAnalysis();
+
+  // for storing the result of analyzed ValueDecls
+  void *NonConstantsImpl;
+
+  const Stmt *DeclBody;
+  bool Analyzed;
+};
+
+}
+
+#endif

Removed: cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h?rev=111831&view=auto
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h (removed)
@@ -1,44 +0,0 @@
-//== PsuedoConstantAnalysis.h - Find Psuedo-constants in the AST -*- C++ -*-==//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file tracks the usage of variables in a Decl body to see if they are
-// never written to, implying that they constant. This is useful in static
-// analysis to see if a developer might have intended a variable to be const.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_ANALYSIS_PSUEDOCONSTANTANALYSIS
-#define LLVM_CLANG_ANALYSIS_PSUEDOCONSTANTANALYSIS
-
-#include "clang/AST/Stmt.h"
-
-// The number of ValueDecls we want to keep track of by default (per-function)
-#define VALUEDECL_SET_SIZE 256
-
-namespace clang {
-
-class PsuedoConstantAnalysis {
-public:
-  PsuedoConstantAnalysis(const Stmt *DeclBody) :
-      DeclBody(DeclBody), Analyzed(false) {}
-  bool isPsuedoConstant(const ValueDecl *VD);
-
-private:
-  void RunAnalysis();
-
-  // for storing the result of analyzed ValueDecls
-  llvm::SmallPtrSet<const ValueDecl*, VALUEDECL_SET_SIZE> NonConstants;
-
-  const Stmt *DeclBody;
-  bool Analyzed;
-};
-
-}
-
-#endif

Modified: cfe/trunk/include/clang/Analysis/AnalysisContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisContext.h?rev=111832&r1=111831&r2=111832&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/AnalysisContext.h (original)
+++ cfe/trunk/include/clang/Analysis/AnalysisContext.h Mon Aug 23 14:51:57 2010
@@ -30,7 +30,7 @@
 class CFGBlock;
 class LiveVariables;
 class ParentMap;
-class PsuedoConstantAnalysis;
+class PseudoConstantAnalysis;
 class ImplicitParamDecl;
 class LocationContextManager;
 class StackFrameContext;
@@ -50,7 +50,7 @@
   bool builtCFG, builtCompleteCFG;
   LiveVariables *liveness;
   ParentMap *PM;
-  PsuedoConstantAnalysis *PCA;
+  PseudoConstantAnalysis *PCA;
   llvm::DenseMap<const BlockDecl*,void*> *ReferencedBlockVars;
   llvm::BumpPtrAllocator A;
   bool UseUnoptimizedCFG;  
@@ -87,7 +87,7 @@
   CFG *getUnoptimizedCFG();
 
   ParentMap &getParentMap();
-  PsuedoConstantAnalysis *getPsuedoConstantAnalysis();
+  PseudoConstantAnalysis *getPseudoConstantAnalysis();
   LiveVariables *getLiveVariables();
 
   typedef const VarDecl * const * referenced_decls_iterator;

Modified: cfe/trunk/lib/Analysis/AnalysisContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisContext.cpp?rev=111832&r1=111831&r2=111832&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/AnalysisContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisContext.cpp Mon Aug 23 14:51:57 2010
@@ -18,7 +18,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
-#include "clang/Analysis/Analyses/PsuedoConstantAnalysis.h"
+#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
 #include "clang/Analysis/AnalysisContext.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/Support/BumpVector.h"
@@ -84,9 +84,9 @@
   return *PM;
 }
 
-PsuedoConstantAnalysis *AnalysisContext::getPsuedoConstantAnalysis() {
+PseudoConstantAnalysis *AnalysisContext::getPseudoConstantAnalysis() {
   if (!PCA)
-    PCA = new PsuedoConstantAnalysis(getBody());
+    PCA = new PseudoConstantAnalysis(getBody());
   return PCA;
 }
 

Modified: cfe/trunk/lib/Analysis/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CMakeLists.txt?rev=111832&r1=111831&r2=111832&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CMakeLists.txt (original)
+++ cfe/trunk/lib/Analysis/CMakeLists.txt Mon Aug 23 14:51:57 2010
@@ -7,7 +7,7 @@
   FormatString.cpp
   LiveVariables.cpp
   PrintfFormatString.cpp
-  PsuedoConstantAnalysis.cpp
+  PseudoConstantAnalysis.cpp
   ReachableCode.cpp
   ScanfFormatString.cpp
   UninitializedValues.cpp

Added: cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp?rev=111832&view=auto
==============================================================================
--- cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp (added)
+++ cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp Mon Aug 23 14:51:57 2010
@@ -0,0 +1,173 @@
+//== PseudoConstantAnalysis.cpp - Find Pseudoconstants in the AST-*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file tracks the usage of variables in a Decl body to see if they are
+// never written to, implying that they constant. This is useful in static
+// analysis to see if a developer might have intended a variable to be const.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include <deque>
+
+using namespace clang;
+
+// The number of ValueDecls we want to keep track of by default (per-function)
+#define VARDECL_SET_SIZE 256
+typedef llvm::SmallPtrSet<const VarDecl*, VARDECL_SET_SIZE> VarDeclSet;
+
+PseudoConstantAnalysis::PseudoConstantAnalysis(const Stmt *DeclBody) :
+      DeclBody(DeclBody), Analyzed(false) {
+  NonConstantsImpl = new VarDeclSet;
+}
+
+PseudoConstantAnalysis::~PseudoConstantAnalysis() {
+  delete (VarDeclSet*)NonConstantsImpl;
+}
+
+// Returns true if the given ValueDecl is never written to in the given DeclBody
+bool PseudoConstantAnalysis::isPseudoConstant(const VarDecl *VD) {
+  // Only local and static variables can be pseudoconstants
+  if (!VD->hasLocalStorage() && !VD->isStaticLocal())
+    return false;
+
+  if (!Analyzed) {
+    RunAnalysis();
+    Analyzed = true;
+  }
+
+  VarDeclSet *NonConstants = (VarDeclSet*)NonConstantsImpl;
+
+  return !NonConstants->count(VD);
+}
+
+void PseudoConstantAnalysis::RunAnalysis() {
+  std::deque<const Stmt *> WorkList;
+  VarDeclSet *NonConstants = (VarDeclSet*)NonConstantsImpl;
+
+  // Start with the top level statement of the function
+  WorkList.push_back(DeclBody);
+
+  while (!WorkList.empty()) {
+    const Stmt* Head = WorkList.front();
+    WorkList.pop_front();
+
+    switch (Head->getStmtClass()) {
+    // Case 1: Assignment operators modifying ValueDecl
+    case Stmt::BinaryOperatorClass: {
+      const BinaryOperator *BO = cast<BinaryOperator>(Head);
+      const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts();
+      const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS);
+
+      // We only care about DeclRefExprs on the LHS
+      if (!DR)
+        break;
+
+      // We found a binary operator with a DeclRefExpr on the LHS. We now check
+      // for any of the assignment operators, implying that this Decl is being
+      // written to.
+      switch (BO->getOpcode()) {
+      case BinaryOperator::Assign:
+      case BinaryOperator::AddAssign:
+      case BinaryOperator::SubAssign:
+      case BinaryOperator::MulAssign:
+      case BinaryOperator::DivAssign:
+      case BinaryOperator::AndAssign:
+      case BinaryOperator::OrAssign:
+      case BinaryOperator::XorAssign:
+      case BinaryOperator::ShlAssign:
+      case BinaryOperator::ShrAssign: {
+        // The DeclRefExpr is being assigned to - mark it as non-constant
+        const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+        if (VD)
+          NonConstants->insert(VD);
+        break;
+      }
+
+      default:
+        break;
+      }
+      break;
+    }
+
+    // Case 2: Pre/post increment/decrement and address of
+    case Stmt::UnaryOperatorClass: {
+      const UnaryOperator *UO = cast<UnaryOperator>(Head);
+      const Expr *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
+      const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(SubExpr);
+
+      // We only care about DeclRefExprs in the subexpression
+      if (!DR)
+        break;
+
+      // We found a unary operator with a DeclRefExpr as a subexpression. We now
+      // check for any of the increment/decrement operators, as well as
+      // addressOf.
+      switch (UO->getOpcode()) {
+      case UnaryOperator::PostDec:
+      case UnaryOperator::PostInc:
+      case UnaryOperator::PreDec:
+      case UnaryOperator::PreInc:
+        // The DeclRefExpr is being changed - mark it as non-constant
+      case UnaryOperator::AddrOf: {
+        // If we are taking the address of the DeclRefExpr, assume it is
+        // non-constant.
+        const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+        if (VD)
+          NonConstants->insert(VD);
+        break;
+      }
+
+      default:
+        break;
+      }
+      break;
+    }
+
+    // Case 3: Reference Declarations
+    case Stmt::DeclStmtClass: {
+      const DeclStmt *DS = cast<DeclStmt>(Head);
+      // Iterate over each decl and see if any of them contain reference decls
+      for (DeclStmt::const_decl_iterator I = DS->decl_begin(), E = DS->decl_end();
+          I != E; ++I) {
+        // We only care about VarDecls
+        const VarDecl *VD = dyn_cast<VarDecl>(*I);
+        if (!VD)
+          continue;
+
+        // We found a VarDecl; make sure it is a reference type
+        if (!VD->getType().getTypePtr()->isReferenceType())
+          continue;
+
+        // Ignore VarDecls without a body
+        if (!VD->getBody())
+          continue;
+
+        // If the reference is to another var, add the var to the non-constant
+        // list
+        if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(VD->getBody()))
+          if (const VarDecl *RefVD = dyn_cast<VarDecl>(DR->getDecl()))
+            NonConstants->insert(RefVD);
+      }
+    }
+
+      default:
+        break;
+    } // switch (head->getStmtClass())
+
+    // Add all substatements to the worklist
+    for (Stmt::const_child_iterator I = Head->child_begin(),
+        E = Head->child_end(); I != E; ++I)
+      if (*I)
+        WorkList.push_back(*I);
+  } // while (!WorkList.empty())
+}

Removed: cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp?rev=111831&view=auto
==============================================================================
--- cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp (original)
+++ cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp (removed)
@@ -1,119 +0,0 @@
-//== PsuedoConstantAnalysis.cpp - Find Psuedoconstants in the AST-*- C++ -*-==//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file tracks the usage of variables in a Decl body to see if they are
-// never written to, implying that they constant. This is useful in static
-// analysis to see if a developer might have intended a variable to be const.
-//
-//===----------------------------------------------------------------------===//
-
-#include "clang/Analysis/Analyses/PsuedoConstantAnalysis.h"
-#include "clang/AST/Decl.h"
-#include "clang/AST/Expr.h"
-#include "clang/AST/Stmt.h"
-#include <deque>
-
-using namespace clang;
-
-// Returns true if the given ValueDecl is never written to in the given DeclBody
-bool PsuedoConstantAnalysis::isPsuedoConstant(const ValueDecl *VD) {
-  if (!Analyzed) {
-    RunAnalysis();
-    Analyzed = true;
-  }
-
-  return !NonConstants.count(VD);
-}
-
-void PsuedoConstantAnalysis::RunAnalysis() {
-  std::deque<const Stmt *> WorkList;
-
-  // Start with the top level statement of the function
-  WorkList.push_back(DeclBody);
-
-  while (!WorkList.empty()) {
-    const Stmt* Head = WorkList.front();
-    WorkList.pop_front();
-
-    switch (Head->getStmtClass()) {
-    // Case 1: Assignment operators modifying ValueDecl
-    case Stmt::BinaryOperatorClass: {
-      const BinaryOperator *BO = cast<BinaryOperator>(Head);
-      const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts();
-      const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS);
-
-      // We only care about DeclRefExprs on the LHS
-      if (!DR)
-        break;
-
-      // We found a binary operator with a DeclRefExpr on the LHS. We now check
-      // for any of the assignment operators, implying that this Decl is being
-      // written to.
-      switch (BO->getOpcode()) {
-      case BinaryOperator::Assign:
-      case BinaryOperator::AddAssign:
-      case BinaryOperator::SubAssign:
-      case BinaryOperator::MulAssign:
-      case BinaryOperator::DivAssign:
-      case BinaryOperator::AndAssign:
-      case BinaryOperator::OrAssign:
-      case BinaryOperator::XorAssign:
-      case BinaryOperator::ShlAssign:
-      case BinaryOperator::ShrAssign:
-        // The DeclRefExpr is being assigned to - mark it as non-constant
-        NonConstants.insert(DR->getDecl());
-        continue; // Continue without looking at children
-
-      default:
-        break;
-      }
-      break;
-    }
-
-    // Case 2: Pre/post increment/decrement and address of
-    case Stmt::UnaryOperatorClass: {
-      const UnaryOperator *UO = cast<UnaryOperator>(Head);
-      const Expr *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
-      const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(SubExpr);
-
-      // We only care about DeclRefExprs in the subexpression
-      if (!DR)
-        break;
-
-      // We found a unary operator with a DeclRefExpr as a subexpression. We now
-      // check for any of the increment/decrement operators, as well as
-      // addressOf.
-      switch (UO->getOpcode()) {
-      case UnaryOperator::PostDec:
-      case UnaryOperator::PostInc:
-      case UnaryOperator::PreDec:
-      case UnaryOperator::PreInc:
-        // The DeclRefExpr is being changed - mark it as non-constant
-      case UnaryOperator::AddrOf:
-        // If we are taking the address of the DeclRefExpr, assume it is
-        // non-constant.
-        NonConstants.insert(DR->getDecl());
-
-      default:
-        break;
-      }
-      break;
-    }
-
-      default:
-        break;
-    } // switch (head->getStmtClass())
-
-    // Add all substatements to the worklist
-    for (Stmt::const_child_iterator I = Head->child_begin(),
-        E = Head->child_end(); I != E; ++I)
-      if (*I)
-        WorkList.push_back(*I);
-  } // while (!WorkList.empty())
-}

Modified: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=111832&r1=111831&r2=111832&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Mon Aug 23 14:51:57 2010
@@ -44,7 +44,7 @@
 
 #include "GRExprEngineExperimentalChecks.h"
 #include "clang/Analysis/CFGStmtMap.h"
-#include "clang/Analysis/Analyses/PsuedoConstantAnalysis.h"
+#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
 #include "clang/Checker/BugReporter/BugType.h"
 #include "clang/Checker/PathSensitive/CheckerHelpers.h"
 #include "clang/Checker/PathSensitive/CheckerVisitor.h"
@@ -83,6 +83,7 @@
     static bool CanVary(const Expr *Ex, AnalysisContext *AC);
     static bool isConstantOrPseudoConstant(const DeclRefExpr *DR,
                                            AnalysisContext *AC);
+    static bool containsNonLocalVarDecl(const Stmt *S);
 
     // Hash table
     typedef llvm::DenseMap<const BinaryOperator *,
@@ -122,12 +123,17 @@
   const Expr *LHS = B->getLHS();
   const Expr *RHS = B->getRHS();
 
-  // Check if either side can vary. We only need to calculate this when we have
-  // no assumption.
-  bool LHSCanVary = true, RHSCanVary = true;
+  // At this stage we can calculate whether each side contains a false positive
+  // that applies to all operators. We only need to calculate this the first
+  // time.
+  bool LHSContainsFalsePositive = false, RHSContainsFalsePositive = false;
   if (A == Possible) {
-    LHSCanVary = CanVary(LHS, AC);
-    RHSCanVary = CanVary(RHS, AC);
+    // An expression contains a false positive if it can't vary, or if it
+    // contains a known false positive VarDecl.
+    LHSContainsFalsePositive = !CanVary(LHS, AC)
+        || containsNonLocalVarDecl(LHS);
+    RHSContainsFalsePositive = !CanVary(RHS, AC)
+        || containsNonLocalVarDecl(RHS);
   }
 
   const GRState *state = C.getState();
@@ -195,7 +201,8 @@
   case BinaryOperator::Xor:
   case BinaryOperator::LOr:
   case BinaryOperator::LAnd:
-    if (LHSVal != RHSVal || !LHSCanVary || !RHSCanVary)
+    if (LHSVal != RHSVal || LHSContainsFalsePositive
+        || RHSContainsFalsePositive)
       break;
     UpdateAssumption(A, Equal);
     return;
@@ -213,7 +220,7 @@
    case BinaryOperator::Div:
    case BinaryOperator::LOr:
    case BinaryOperator::LAnd:
-     if (!RHSVal.isConstant(1) || !RHSCanVary)
+     if (!RHSVal.isConstant(1) || RHSContainsFalsePositive)
        break;
      UpdateAssumption(A, RHSis1);
      return;
@@ -229,7 +236,7 @@
   case BinaryOperator::Mul:
   case BinaryOperator::LOr:
   case BinaryOperator::LAnd:
-    if (!LHSVal.isConstant(1) || !LHSCanVary)
+    if (!LHSVal.isConstant(1) || LHSContainsFalsePositive)
       break;
     UpdateAssumption(A, LHSis1);
     return;
@@ -257,7 +264,7 @@
   case BinaryOperator::Shr:
   case BinaryOperator::LOr:
   case BinaryOperator::LAnd:
-    if (!RHSVal.isConstant(0) || !RHSCanVary)
+    if (!RHSVal.isConstant(0) || RHSContainsFalsePositive)
       break;
     UpdateAssumption(A, RHSis0);
     return;
@@ -289,7 +296,7 @@
   case BinaryOperator::Shr:
   case BinaryOperator::LOr:
   case BinaryOperator::LAnd:
-    if (!LHSVal.isConstant(0) || !LHSCanVary)
+    if (!LHSVal.isConstant(0) || LHSContainsFalsePositive)
       break;
     UpdateAssumption(A, LHSis0);
     return;
@@ -578,16 +585,35 @@
   if (isa<EnumConstantDecl>(DR->getDecl()))
     return true;
 
-  // Check for a static variable
-  // FIXME: Analysis should model static vars
-  if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()))
-    if (VD->isStaticLocal())
-      return true;
+  const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+  if (!VD)
+    return true;
 
-  // Check if the Decl behaves like a constant
-  PsuedoConstantAnalysis *PCA = AC->getPsuedoConstantAnalysis();
-  if (PCA->isPsuedoConstant(DR->getDecl()))
+  // Check if the Decl behaves like a constant. This check also takes care of
+  // static variables, which can only change between function calls if they are
+  // modified in the AST.
+  PseudoConstantAnalysis *PCA = AC->getPseudoConstantAnalysis();
+  if (PCA->isPseudoConstant(VD))
     return true;
 
   return false;
 }
+
+// Recursively find any substatements containing VarDecl's with storage other
+// than local
+bool IdempotentOperationChecker::containsNonLocalVarDecl(const Stmt *S) {
+  const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);
+
+  if (DR)
+    if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()))
+      if (!VD->hasLocalStorage())
+        return true;
+
+  for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();
+      ++I)
+    if (const Stmt *child = *I)
+      if (containsNonLocalVarDecl(child))
+        return true;
+
+  return false;
+}

Modified: cfe/trunk/test/Analysis/idempotent-operations.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=111832&r1=111831&r2=111832&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.c (original)
+++ cfe/trunk/test/Analysis/idempotent-operations.c Mon Aug 23 14:51:57 2010
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -verify -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-check-idempotent-operations -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-check-idempotent-operations -verify %s
 
 // Basic tests
 
@@ -51,7 +51,7 @@
   test(zero << x); // expected-warning {{The left operand to '<<' is always 0}}
   test(zero >> x); // expected-warning {{The left operand to '>>' is always 0}}
 
-  // Overwrite the values so these aren't marked as psuedoconstants
+  // Overwrite the values so these aren't marked as Pseudoconstants
   x = 1;
   zero = 2;
   one = 3;
@@ -102,12 +102,38 @@
   return nonparam;
 }
 
-// Psuedo-constants (vars only read) and constants should not be reported
+// Pseudo-constants (vars only read) and constants should not be reported
 unsigned false4() {
   // Trivial constant
-  const int height = 1; // no-warning
-  // Psuedo-constant (never changes after decl)
-  int width = height; // no-warning
+  const int height = 1;
+
+  // Pseudo-constant (never changes after decl)
+  int width = height;
+
+  // Pseudo-constant (blockvar)
+  __block int a = 0;
+  int b = 10;
+  a *= b; // no-warning
+  test(a);
 
   return width * 10; // no-warning
 }
+
+// Static vars are common false positives
+int false5() {
+  static int test = 0;
+  int a = 56;
+  a *= test; // no-warning
+  test++;
+  return a;
+}
+
+// Non-local storage vars are considered false positives
+int globalInt = 1;
+int false6() {
+  int localInt = 23;
+
+  localInt /= globalInt;
+
+  return localInt;
+}





More information about the cfe-commits mailing list