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

llvm-svn: 111832
This commit is contained in:
Tom Care 2010-08-23 19:51:57 +00:00
parent 58bd73a5a7
commit e332c3b762
7 changed files with 157 additions and 53 deletions

View 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
//
@ -13,27 +13,25 @@
//
//===----------------------------------------------------------------------===//
#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;

View 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;

View 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;
}

View 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

View 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"
@ -21,18 +21,38 @@
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())

View 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;
}

View 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;
}