]> granicus.if.org Git - clang/commitdiff
Several small changes to PseudoConstantAnalysis and the way IdempotentOperationChecke...
authorTom Care <tom.care@uqconnect.edu.au>
Mon, 23 Aug 2010 19:51:57 +0000 (19:51 +0000)
committerTom Care <tom.care@uqconnect.edu.au>
Mon, 23 Aug 2010 19:51:57 +0000 (19:51 +0000)
- 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

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

include/clang/Analysis/Analyses/PseudoConstantAnalysis.h [moved from include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h with 60% similarity]
include/clang/Analysis/AnalysisContext.h
lib/Analysis/AnalysisContext.cpp
lib/Analysis/CMakeLists.txt
lib/Analysis/PseudoConstantAnalysis.cpp [moved from lib/Analysis/PsuedoConstantAnalysis.cpp with 60% similarity]
lib/Checker/IdempotentOperationChecker.cpp
test/Analysis/idempotent-operations.c

similarity index 60%
rename from include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h
rename to include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
index 080f6c158757fb4027eac46c736c44a377eba77d..0dd534c2a85a34b782ef46ec7269c1260a539994 100644 (file)
@@ -1,4 +1,4 @@
-//== PsuedoConstantAnalysis.h - Find Psuedo-constants in the AST -*- C++ -*-==//
+//== PseudoConstantAnalysis.h - Find Pseudo-constants in the AST -*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CLANG_ANALYSIS_PSUEDOCONSTANTANALYSIS
-#define LLVM_CLANG_ANALYSIS_PSUEDOCONSTANTANALYSIS
+#ifndef LLVM_CLANG_ANALYSIS_PSEUDOCONSTANTANALYSIS
+#define LLVM_CLANG_ANALYSIS_PSEUDOCONSTANTANALYSIS
 
 #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 {
+class PseudoConstantAnalysis {
 public:
-  PsuedoConstantAnalysis(const Stmt *DeclBody) :
-      DeclBody(DeclBody), Analyzed(false) {}
-  bool isPsuedoConstant(const ValueDecl *VD);
+  PseudoConstantAnalysis(const Stmt *DeclBody);
+  ~PseudoConstantAnalysis();
+
+  bool isPseudoConstant(const VarDecl *VD);
 
 private:
   void RunAnalysis();
 
   // for storing the result of analyzed ValueDecls
-  llvm::SmallPtrSet<const ValueDecl*, VALUEDECL_SET_SIZE> NonConstants;
+  void *NonConstantsImpl;
 
   const Stmt *DeclBody;
   bool Analyzed;
index 124a08e0739559c3efddfc754ad7247a4e3214a1..6091db4be923ef6edf757721493f579feb4f7ade 100644 (file)
@@ -30,7 +30,7 @@ class CFG;
 class CFGBlock;
 class LiveVariables;
 class ParentMap;
-class PsuedoConstantAnalysis;
+class PseudoConstantAnalysis;
 class ImplicitParamDecl;
 class LocationContextManager;
 class StackFrameContext;
@@ -50,7 +50,7 @@ class AnalysisContext {
   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 @@ public:
   CFG *getUnoptimizedCFG();
 
   ParentMap &getParentMap();
-  PsuedoConstantAnalysis *getPsuedoConstantAnalysis();
+  PseudoConstantAnalysis *getPseudoConstantAnalysis();
   LiveVariables *getLiveVariables();
 
   typedef const VarDecl * const * referenced_decls_iterator;
index 934a031b3ab5c1239c4172cd0665e7a7afd4dab5..2c337f07c3c9c4786318693d545158ff28d9aee4 100644 (file)
@@ -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 @@ ParentMap &AnalysisContext::getParentMap() {
   return *PM;
 }
 
-PsuedoConstantAnalysis *AnalysisContext::getPsuedoConstantAnalysis() {
+PseudoConstantAnalysis *AnalysisContext::getPseudoConstantAnalysis() {
   if (!PCA)
-    PCA = new PsuedoConstantAnalysis(getBody());
+    PCA = new PseudoConstantAnalysis(getBody());
   return PCA;
 }
 
index 514042bf9a1b9b0cb94e402c02af0e8be1854e36..850e9b468100bb661fe1c61a0450ea900589067e 100644 (file)
@@ -7,7 +7,7 @@ add_clang_library(clangAnalysis
   FormatString.cpp
   LiveVariables.cpp
   PrintfFormatString.cpp
-  PsuedoConstantAnalysis.cpp
+  PseudoConstantAnalysis.cpp
   ReachableCode.cpp
   ScanfFormatString.cpp
   UninitializedValues.cpp
similarity index 60%
rename from lib/Analysis/PsuedoConstantAnalysis.cpp
rename to lib/Analysis/PseudoConstantAnalysis.cpp
index a169f89fe1fc09cb58c17121c9e0ddcbc7e43213..7a2fa8a8bd23aee307952b092dfd848fce13f8ef 100644 (file)
@@ -1,4 +1,4 @@
-//== PsuedoConstantAnalysis.cpp - Find Psuedoconstants in the AST-*- C++ -*-==//
+//== PseudoConstantAnalysis.cpp - Find Pseudoconstants in the AST-*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -13,7 +13,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/Analysis/Analyses/PsuedoConstantAnalysis.h"
+#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Stmt.h"
 
 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 PsuedoConstantAnalysis::isPsuedoConstant(const ValueDecl *VD) {
+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;
   }
 
-  return !NonConstants.count(VD);
+  VarDeclSet *NonConstants = (VarDeclSet*)NonConstantsImpl;
+
+  return !NonConstants->count(VD);
 }
 
-void PsuedoConstantAnalysis::RunAnalysis() {
+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);
@@ -65,10 +85,13 @@ void PsuedoConstantAnalysis::RunAnalysis() {
       case BinaryOperator::OrAssign:
       case BinaryOperator::XorAssign:
       case BinaryOperator::ShlAssign:
-      case BinaryOperator::ShrAssign:
+      case BinaryOperator::ShrAssign: {
         // The DeclRefExpr is being assigned to - mark it as non-constant
-        NonConstants.insert(DR->getDecl());
-        continue; // Continue without looking at children
+        const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+        if (VD)
+          NonConstants->insert(VD);
+        break;
+      }
 
       default:
         break;
@@ -95,10 +118,14 @@ void PsuedoConstantAnalysis::RunAnalysis() {
       case UnaryOperator::PreDec:
       case UnaryOperator::PreInc:
         // The DeclRefExpr is being changed - mark it as non-constant
-      case UnaryOperator::AddrOf:
+      case UnaryOperator::AddrOf: {
         // If we are taking the address of the DeclRefExpr, assume it is
         // non-constant.
-        NonConstants.insert(DR->getDecl());
+        const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+        if (VD)
+          NonConstants->insert(VD);
+        break;
+      }
 
       default:
         break;
@@ -106,6 +133,33 @@ void PsuedoConstantAnalysis::RunAnalysis() {
       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())
index 885123ae241a7d059926510f146fd2b284fa4cda..2cfcaa44d4b905da3ca89831b56ebf7fa525c97c 100644 (file)
@@ -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 @@ class IdempotentOperationChecker
     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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
   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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
   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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
    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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
   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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
   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 @@ void IdempotentOperationChecker::PreVisitBinaryOperator(
   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 @@ bool IdempotentOperationChecker::isConstantOrPseudoConstant(
   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;
+}
index abf67103c11f278c2d9579c882f30b686e67a820..179c7a4da3f8c4a0896be555be3a533ee82d813e 100644 (file)
@@ -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 @@ unsigned basic() {
   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 @@ unsigned false3(int param) {
   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;
+}