[cfe-commits] r111426 - in /cfe/trunk: include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h include/clang/Analysis/AnalysisContext.h lib/Analysis/AnalysisContext.cpp lib/Analysis/CMakeLists.txt lib/Analysis/PsuedoConstantAnalysis.cpp lib/Checker/IdempotentOperationChecker.cpp test/Analysis/constant-folding.c test/Analysis/idempotent-operations.c test/Analysis/null-deref-ps.c
Tom Care
tcare at apple.com
Wed Aug 18 14:17:24 PDT 2010
Author: tcare
Date: Wed Aug 18 16:17:24 2010
New Revision: 111426
URL: http://llvm.org/viewvc/llvm-project?rev=111426&view=rev
Log:
Added psuedo-constant analysis and integrated it into the false positive reduction stage in IdempotentOperationChecker.
- Renamed IdempotentOperationChecker::isConstant to isConstantOrPseudoConstant to better reflect the function
- Changed IdempotentOperationChecker::PreVisitBinaryOperator to only run 'CanVary' once on undefined assumptions
- Created new PsuedoConstantAnalysis class and added it to AnalysisContext
- Changed IdempotentOperationChecker to exploit the new analysis
- Updated tests with psuedo-constants
- Added check to IdempotentOperationChecker to see if a Decl is const qualified
Added:
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/constant-folding.c
cfe/trunk/test/Analysis/idempotent-operations.c
cfe/trunk/test/Analysis/null-deref-ps.c
Added: cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h?rev=111426&view=auto
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h (added)
+++ cfe/trunk/include/clang/Analysis/Analyses/PsuedoConstantAnalysis.h Wed Aug 18 16:17:24 2010
@@ -0,0 +1,44 @@
+//== 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=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/AnalysisContext.h (original)
+++ cfe/trunk/include/clang/Analysis/AnalysisContext.h Wed Aug 18 16:17:24 2010
@@ -30,6 +30,7 @@
class CFGBlock;
class LiveVariables;
class ParentMap;
+class PsuedoConstantAnalysis;
class ImplicitParamDecl;
class LocationContextManager;
class StackFrameContext;
@@ -49,6 +50,7 @@
bool builtCFG, builtCompleteCFG;
LiveVariables *liveness;
ParentMap *PM;
+ PsuedoConstantAnalysis *PCA;
llvm::DenseMap<const BlockDecl*,void*> *ReferencedBlockVars;
llvm::BumpPtrAllocator A;
bool UseUnoptimizedCFG;
@@ -59,7 +61,7 @@
bool addehedges = false)
: D(d), TU(tu), cfg(0), completeCFG(0),
builtCFG(false), builtCompleteCFG(false),
- liveness(0), PM(0),
+ liveness(0), PM(0), PCA(0),
ReferencedBlockVars(0), UseUnoptimizedCFG(useUnoptimizedCFG),
AddEHEdges(addehedges) {}
@@ -85,6 +87,7 @@
CFG *getUnoptimizedCFG();
ParentMap &getParentMap();
+ PsuedoConstantAnalysis *getPsuedoConstantAnalysis();
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=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/AnalysisContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisContext.cpp Wed Aug 18 16:17:24 2010
@@ -18,6 +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/AnalysisContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/Support/BumpVector.h"
@@ -83,6 +84,12 @@
return *PM;
}
+PsuedoConstantAnalysis *AnalysisContext::getPsuedoConstantAnalysis() {
+ if (!PCA)
+ PCA = new PsuedoConstantAnalysis(getBody());
+ return PCA;
+}
+
LiveVariables *AnalysisContext::getLiveVariables() {
if (!liveness) {
CFG *c = getCFG();
@@ -314,6 +321,7 @@
delete completeCFG;
delete liveness;
delete PM;
+ delete PCA;
delete ReferencedBlockVars;
}
Modified: cfe/trunk/lib/Analysis/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CMakeLists.txt?rev=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CMakeLists.txt (original)
+++ cfe/trunk/lib/Analysis/CMakeLists.txt Wed Aug 18 16:17:24 2010
@@ -7,6 +7,7 @@
FormatString.cpp
LiveVariables.cpp
PrintfFormatString.cpp
+ PsuedoConstantAnalysis.cpp
ReachableCode.cpp
ScanfFormatString.cpp
UninitializedValues.cpp
Added: cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp?rev=111426&view=auto
==============================================================================
--- cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp (added)
+++ cfe/trunk/lib/Analysis/PsuedoConstantAnalysis.cpp Wed Aug 18 16:17:24 2010
@@ -0,0 +1,119 @@
+//== 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=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Wed Aug 18 16:17:24 2010
@@ -44,6 +44,7 @@
#include "GRExprEngineExperimentalChecks.h"
#include "clang/Analysis/CFGStmtMap.h"
+#include "clang/Analysis/Analyses/PsuedoConstantAnalysis.h"
#include "clang/Checker/BugReporter/BugType.h"
#include "clang/Checker/PathSensitive/CheckerHelpers.h"
#include "clang/Checker/PathSensitive/CheckerVisitor.h"
@@ -72,16 +73,16 @@
void UpdateAssumption(Assumption &A, const Assumption &New);
- /// contains* - Useful recursive methods to see if a statement contains an
- /// element somewhere. Used in static analysis to reduce false positives.
+ // False positive reduction methods
static bool isParameterSelfAssign(const Expr *LHS, const Expr *RHS);
static bool isTruncationExtensionAssignment(const Expr *LHS,
const Expr *RHS);
static bool PathWasCompletelyAnalyzed(const CFG *C,
const CFGBlock *CB,
const GRCoreEngine &CE);
- static bool CanVary(const Expr *Ex, ASTContext &Ctx);
- static bool isPseudoConstant(const DeclRefExpr *D);
+ static bool CanVary(const Expr *Ex, AnalysisContext *AC);
+ static bool isConstantOrPseudoConstant(const DeclRefExpr *DR,
+ AnalysisContext *AC);
// Hash table
typedef llvm::DenseMap<const BinaryOperator *,
@@ -108,7 +109,8 @@
// 'Possible'.
std::pair<Assumption, AnalysisContext *> &Data = hash[B];
Assumption &A = Data.first;
- Data.second = C.getCurrentAnalysisContext();
+ AnalysisContext *AC = C.getCurrentAnalysisContext();
+ Data.second = AC;
// If we already have visited this node on a path that does not contain an
// idempotent operation, return immediately.
@@ -119,8 +121,14 @@
// may mean this is a false positive.
const Expr *LHS = B->getLHS();
const Expr *RHS = B->getRHS();
- bool LHSCanVary = CanVary(LHS, C.getASTContext());
- bool RHSCanVary = CanVary(RHS, C.getASTContext());
+
+ // Check if either side can vary. We only need to calculate this when we have
+ // no assumption.
+ bool LHSCanVary = true, RHSCanVary = true;
+ if (A == Possible) {
+ LHSCanVary = CanVary(LHS, AC);
+ RHSCanVary = CanVary(RHS, AC);
+ }
const GRState *state = C.getState();
@@ -486,7 +494,8 @@
// that varies. This could be due to a compile-time constant like sizeof. An
// expression may also involve a variable that behaves like a constant. The
// function returns true if the expression varies, and false otherwise.
-bool IdempotentOperationChecker::CanVary(const Expr *Ex, ASTContext &Ctx) {
+bool IdempotentOperationChecker::CanVary(const Expr *Ex,
+ AnalysisContext *AC) {
// Parentheses and casts are irrelevant here
Ex = Ex->IgnoreParenCasts();
@@ -531,12 +540,13 @@
return SE->getTypeOfArgument()->isVariableArrayType();
}
case Stmt::DeclRefExprClass:
- return !isPseudoConstant(cast<DeclRefExpr>(Ex));
+ return !isConstantOrPseudoConstant(cast<DeclRefExpr>(Ex), AC);
// The next cases require recursion for subexpressions
case Stmt::BinaryOperatorClass: {
const BinaryOperator *B = cast<const BinaryOperator>(Ex);
- return CanVary(B->getRHS(), Ctx) || CanVary(B->getLHS(), Ctx);
+ return CanVary(B->getRHS(), AC)
+ || CanVary(B->getLHS(), AC);
}
case Stmt::UnaryOperatorClass: {
const UnaryOperator *U = cast<const UnaryOperator>(Ex);
@@ -545,18 +555,25 @@
case UnaryOperator::Extension:
return false;
default:
- return CanVary(U->getSubExpr(), Ctx);
+ return CanVary(U->getSubExpr(), AC);
}
}
case Stmt::ChooseExprClass:
- return CanVary(cast<const ChooseExpr>(Ex)->getChosenSubExpr(Ctx), Ctx);
+ return CanVary(cast<const ChooseExpr>(Ex)->getChosenSubExpr(
+ AC->getASTContext()), AC);
case Stmt::ConditionalOperatorClass:
- return CanVary(cast<const ConditionalOperator>(Ex)->getCond(), Ctx);
+ return CanVary(cast<const ConditionalOperator>(Ex)->getCond(), AC);
}
}
-// Returns true if a DeclRefExpr behaves like a constant.
-bool IdempotentOperationChecker::isPseudoConstant(const DeclRefExpr *DR) {
+// Returns true if a DeclRefExpr is or behaves like a constant.
+bool IdempotentOperationChecker::isConstantOrPseudoConstant(
+ const DeclRefExpr *DR,
+ AnalysisContext *AC) {
+ // Check if the type of the Decl is const-qualified
+ if (DR->getType().isConstQualified())
+ return true;
+
// Check for an enum
if (isa<EnumConstantDecl>(DR->getDecl()))
return true;
@@ -567,5 +584,10 @@
if (VD->isStaticLocal())
return true;
+ // Check if the Decl behaves like a constant
+ PsuedoConstantAnalysis *PCA = AC->getPsuedoConstantAnalysis();
+ if (PCA->isPsuedoConstant(DR->getDecl()))
+ return true;
+
return false;
}
Modified: cfe/trunk/test/Analysis/constant-folding.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/constant-folding.c?rev=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/constant-folding.c (original)
+++ cfe/trunk/test/Analysis/constant-folding.c Wed Aug 18 16:17:24 2010
@@ -18,10 +18,10 @@
}
void testSelfOperations (int a) {
- if ((a|a) != a) WARN; // expected-warning{{Both operands to '|' always have the same value}} expected-warning{{never executed}}
- if ((a&a) != a) WARN; // expected-warning{{Both operands to '&' always have the same value}} expected-warning{{never executed}}
- if ((a^a) != 0) WARN; // expected-warning{{Both operands to '^' always have the same value}} expected-warning{{never executed}}
- if ((a-a) != 0) WARN; // expected-warning{{Both operands to '-' always have the same value}} expected-warning{{never executed}}
+ if ((a|a) != a) WARN; // expected-warning{{never executed}}
+ if ((a&a) != a) WARN; // expected-warning{{never executed}}
+ if ((a^a) != 0) WARN; // expected-warning{{never executed}}
+ if ((a-a) != 0) WARN; // expected-warning{{never executed}}
}
void testIdempotent (int a) {
@@ -68,5 +68,5 @@
if (b!=a) WARN; // expected-warning{{never executed}}
if (b>a) WARN; // expected-warning{{never executed}}
if (b<a) WARN; // expected-warning{{never executed}}
- if (b-a) WARN; // expected-warning{{Both operands to '-' always have the same value}} expected-warning{{never executed}}
+ if (b-a) WARN; // expected-warning{{never executed}}
}
Modified: cfe/trunk/test/Analysis/idempotent-operations.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.c (original)
+++ cfe/trunk/test/Analysis/idempotent-operations.c Wed Aug 18 16:17:24 2010
@@ -5,7 +5,7 @@
extern void test(int i);
extern void test_f(float f);
-void basic() {
+unsigned basic() {
int x = 10, zero = 0, one = 1;
// x op x
@@ -50,6 +50,13 @@
test(zero ^ x); // expected-warning {{The left operand to '^' is always 0}}
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
+ x = 1;
+ zero = 2;
+ one = 3;
+
+ return x + zero + one;
}
void floats(float x) {
@@ -84,4 +91,23 @@
return enum1 + a; // no-warning
}
-extern unsigned foo();
+// Self assignments of parameters are common false positives
+unsigned false3(int param) {
+ param = param; // no-warning
+
+ unsigned nonparam = 5;
+
+ nonparam = nonparam; // expected-warning{{Assigned value is always the same as the existing value}}
+
+ return nonparam;
+}
+
+// Psuedo-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
+
+ return width * 10; // no-warning
+}
Modified: cfe/trunk/test/Analysis/null-deref-ps.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-ps.c?rev=111426&r1=111425&r2=111426&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-ps.c (original)
+++ cfe/trunk/test/Analysis/null-deref-ps.c Wed Aug 18 16:17:24 2010
@@ -260,7 +260,7 @@
// Test handling of translating between integer "pointers" and back.
void f13() {
int *x = 0;
- if (((((int) x) << 2) + 1) >> 1) *x = 1; // expected-warning{{The left operand to '<<' is always 0}} expected-warning{{The left operand to '+' is always 0}}
+ if (((((int) x) << 2) + 1) >> 1) *x = 1;
}
// PR 4759 - Attribute non-null checking by the analyzer was not correctly
More information about the cfe-commits
mailing list