[cfe-commits] r80786 - in /cfe/trunk: include/clang/Analysis/PathSensitive/Checker.h include/clang/Analysis/PathSensitive/CheckerVisitor.def include/clang/Analysis/PathSensitive/CheckerVisitor.h lib/Analysis/BugReporterVisitors.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/GRExprEngineInternalChecks.cpp
Zhongxing Xu
xuzhongxing at gmail.com
Wed Sep 2 06:26:26 PDT 2009
Author: zhongxingxu
Date: Wed Sep 2 08:26:26 2009
New Revision: 80786
URL: http://llvm.org/viewvc/llvm-project?rev=80786&view=rev
Log:
Refactor the check for bad divide into a checker.
Also fix a checker context bug: the Dst set is not always empty initially.
Because in GRExprEngine::CheckerVisit(), *CurrSet is used repeatedly.
So we removed the Dst.empty() condition in ~CheckerContext() when deciding
whether to do autotransision.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h
cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.def
cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.h
cfe/trunk/lib/Analysis/BugReporterVisitors.cpp
cfe/trunk/lib/Analysis/GRExprEngine.cpp
cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h Wed Sep 2 08:26:26 2009
@@ -49,14 +49,15 @@
: Dst(dst), B(builder), Eng(eng), Pred(pred),
OldSink(B.BuildSinks), OldTag(B.Tag),
OldPointKind(B.PointKind), OldHasGen(B.HasGeneratedNode) {
- assert(Dst.empty());
+ //assert(Dst.empty()); // This is a fake assertion.
+ // See GRExprEngine::CheckerVisit(), CurrSet is repeatedly used.
B.Tag = tag;
if (preVisit)
B.PointKind = ProgramPoint::PreStmtKind;
}
~CheckerContext() {
- if (!B.BuildSinks && Dst.empty() && !B.HasGeneratedNode)
+ if (!B.BuildSinks && !B.HasGeneratedNode)
Dst.Add(Pred);
}
@@ -71,8 +72,12 @@
ASTContext &getASTContext() {
return Eng.getContext();
}
+
+ ExplodedNode *GenerateNode(const Stmt *S, bool markAsSink = false) {
+ return GenerateNode(S, getState(), markAsSink);
+ }
- ExplodedNode *generateNode(const Stmt* S, const GRState *state,
+ ExplodedNode *GenerateNode(const Stmt* S, const GRState *state,
bool markAsSink = false) {
ExplodedNode *node = B.generateNode(S, state, Pred);
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.def?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.def (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.def Wed Sep 2 08:26:26 2009
@@ -13,5 +13,6 @@
PREVISIT(CallExpr)
PREVISIT(ObjCMessageExpr)
+PREVISIT(BinaryOperator)
#undef PREVISIT
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.h?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/CheckerVisitor.h Wed Sep 2 08:26:26 2009
@@ -36,6 +36,10 @@
default:
assert(false && "Unsupport statement.");
return;
+ case Stmt::CompoundAssignOperatorClass:
+ static_cast<ImplClass*>(this)->PreVisitBinaryOperator(C,
+ static_cast<const BinaryOperator*>(S));
+ break;
#define PREVISIT(NAME) \
case Stmt::NAME ## Class:\
static_cast<ImplClass*>(this)->PreVisit ## NAME(C,static_cast<const NAME*>(S));\
Modified: cfe/trunk/lib/Analysis/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BugReporterVisitors.cpp?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/Analysis/BugReporterVisitors.cpp Wed Sep 2 08:26:26 2009
@@ -55,7 +55,7 @@
const Stmt*
clang::bugreporter::GetDenomExpr(const ExplodedNode *N) {
- const Stmt *S = N->getLocationAs<PostStmt>()->getStmt();
+ const Stmt *S = N->getLocationAs<PreStmt>()->getStmt();
if (const BinaryOperator *BE = dyn_cast<BinaryOperator>(S))
return BE->getRHS();
return NULL;
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Wed Sep 2 08:26:26 2009
@@ -2700,44 +2700,6 @@
// Transfer functions: Binary operators.
//===----------------------------------------------------------------------===//
-const GRState* GRExprEngine::CheckDivideZero(Expr* Ex, const GRState* state,
- ExplodedNode* Pred, SVal Denom) {
-
- // Divide by undefined? (potentially zero)
-
- if (Denom.isUndef()) {
- ExplodedNode* DivUndef = Builder->generateNode(Ex, state, Pred);
-
- if (DivUndef) {
- DivUndef->markAsSink();
- ExplicitBadDivides.insert(DivUndef);
- }
-
- return 0;
- }
-
- // Check for divide/remainder-by-zero.
- // First, "assume" that the denominator is 0 or undefined.
- const GRState* zeroState = state->assume(Denom, false);
-
- // Second, "assume" that the denominator cannot be 0.
- state = state->assume(Denom, true);
-
- // Create the node for the divide-by-zero (if it occurred).
- if (zeroState)
- if (ExplodedNode* DivZeroNode = Builder->generateNode(Ex, zeroState, Pred)) {
- DivZeroNode->markAsSink();
-
- if (state)
- ImplicitBadDivides.insert(DivZeroNode);
- else
- ExplicitBadDivides.insert(DivZeroNode);
-
- }
-
- return state;
-}
-
void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
ExplodedNode* Pred,
ExplodedNodeSet& Dst) {
@@ -2765,10 +2727,14 @@
ExplodedNodeSet Tmp2;
Visit(RHS, *I1, Tmp2);
+
+ ExplodedNodeSet CheckedSet;
+ CheckerVisit(B, CheckedSet, Tmp2, true);
// With both the LHS and RHS evaluated, process the operation itself.
- for (ExplodedNodeSet::iterator I2=Tmp2.begin(), E2=Tmp2.end(); I2 != E2; ++I2) {
+ for (ExplodedNodeSet::iterator I2=CheckedSet.begin(), E2=CheckedSet.end();
+ I2 != E2; ++I2) {
const GRState* state = GetState(*I2);
const GRState* OldSt = state;
@@ -2799,17 +2765,6 @@
continue;
}
- case BinaryOperator::Div:
- case BinaryOperator::Rem:
-
- // Special checking for integer denominators.
- if (RHS->getType()->isIntegerType() &&
- RHS->getType()->isScalarType()) {
-
- state = CheckDivideZero(B, state, *I2, RightV);
- if (!state) continue;
- }
-
// FALL-THROUGH.
default: {
@@ -2875,24 +2830,12 @@
SVal location = state->getSVal(LHS);
EvalLoad(Tmp3, LHS, *I2, state, location);
- for (ExplodedNodeSet::iterator I3=Tmp3.begin(), E3=Tmp3.end(); I3!=E3; ++I3) {
+ for (ExplodedNodeSet::iterator I3=Tmp3.begin(), E3=Tmp3.end(); I3!=E3;
+ ++I3) {
state = GetState(*I3);
SVal V = state->getSVal(LHS);
- // Check for divide-by-zero.
- if ((Op == BinaryOperator::Div || Op == BinaryOperator::Rem)
- && RHS->getType()->isIntegerType()
- && RHS->getType()->isScalarType()) {
-
- // CheckDivideZero returns a new state where the denominator
- // is assumed to be non-zero.
- state = CheckDivideZero(B, state, *I3, RightV);
-
- if (!state)
- continue;
- }
-
// Propagate undefined values (left-side).
if (V.isUndef()) {
EvalStore(Dst, B, LHS, *I3, state->BindExpr(B, V),
Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=80786&r1=80785&r2=80786&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Wed Sep 2 08:26:26 2009
@@ -193,14 +193,10 @@
class VISIBILITY_HIDDEN DivZero : public BuiltinBug {
public:
- DivZero(GRExprEngine* eng)
+ DivZero(GRExprEngine* eng = 0)
: BuiltinBug(eng,"Division-by-zero",
"Division by zero or undefined value.") {}
- void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
- Emit(BR, Eng.explicit_bad_divides_begin(), Eng.explicit_bad_divides_end());
- }
-
void registerInitialVisitors(BugReporterContext& BRC,
const ExplodedNode* N,
BuiltinBugReport *R) {
@@ -575,7 +571,7 @@
if (stateNull && !stateNotNull) {
// Generate an error node. Check for a null node in case
// we cache out.
- if (ExplodedNode *errorNode = C.generateNode(CE, stateNull, true)) {
+ if (ExplodedNode *errorNode = C.GenerateNode(CE, stateNull, true)) {
// Lazily allocate the BugType object if it hasn't already been
// created. Ownership is transferred to the BugReporter object once
@@ -611,7 +607,7 @@
// If we reach here all of the arguments passed the nonnull check.
// If 'state' has been updated generated a new node.
if (state != originalState)
- C.addTransition(C.generateNode(CE, state));
+ C.addTransition(C.GenerateNode(CE, state));
}
};
} // end anonymous namespace
@@ -639,7 +635,7 @@
for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end();
I != E; ++I) {
if (C.getState()->getSVal(*I).isUndef()) {
- if (ExplodedNode *ErrorNode = C.generateNode(CE, C.getState(), true)) {
+ if (ExplodedNode *ErrorNode = C.GenerateNode(CE, true)) {
if (!BT)
BT = new BadArg();
// Generate a report for this bug.
@@ -672,7 +668,7 @@
SVal L = C.getState()->getSVal(Callee);
if (L.isUndef() || isa<loc::ConcreteInt>(L)) {
- if (ExplodedNode *N = C.generateNode(CE, C.getState(), true)) {
+ if (ExplodedNode *N = C.GenerateNode(CE, true)) {
if (!BT)
BT = new BadCall();
C.EmitReport(new BuiltinBugReport(*BT, BT->getDescription().c_str(), N));
@@ -680,6 +676,68 @@
}
}
+class VISIBILITY_HIDDEN CheckBadDiv : public CheckerVisitor<CheckBadDiv> {
+ DivZero *BT;
+public:
+ CheckBadDiv() : BT(0) {}
+ ~CheckBadDiv() {}
+
+ const void *getTag() {
+ static int x;
+ return &x;
+ }
+
+ void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B);
+};
+
+void CheckBadDiv::PreVisitBinaryOperator(CheckerContext &C,
+ const BinaryOperator *B) {
+ BinaryOperator::Opcode Op = B->getOpcode();
+ if (Op != BinaryOperator::Div &&
+ Op != BinaryOperator::Rem &&
+ Op != BinaryOperator::DivAssign &&
+ Op != BinaryOperator::RemAssign)
+ return;
+
+ if (!B->getRHS()->getType()->isIntegerType() ||
+ !B->getRHS()->getType()->isScalarType())
+ return;
+
+ printf("should not check!\n");
+
+ // Check for divide by undefined.
+ SVal Denom = C.getState()->getSVal(B->getRHS());
+
+ if (Denom.isUndef()) {
+ if (ExplodedNode *N = C.GenerateNode(B, true)) {
+ if (!BT)
+ BT = new DivZero();
+
+ C.EmitReport(new BuiltinBugReport(*BT, BT->getDescription().c_str(), N));
+ }
+ return;
+ }
+
+ // Check for divide by zero.
+ ConstraintManager &CM = C.getConstraintManager();
+ const GRState *stateNotZero, *stateZero;
+ llvm::tie(stateNotZero, stateZero) = CM.AssumeDual(C.getState(),
+ cast<DefinedSVal>(Denom));
+
+ if (stateZero && !stateNotZero) {
+ if (ExplodedNode *N = C.GenerateNode(B, stateZero, true)) {
+ if (!BT)
+ BT = new DivZero();
+
+ C.EmitReport(new BuiltinBugReport(*BT, BT->getDescription().c_str(), N));
+ }
+ return;
+ }
+
+ // If we get here, then the denom should not be zero.
+ if (stateNotZero != C.getState())
+ C.addTransition(C.GenerateNode(B, stateNotZero));
+}
}
//===----------------------------------------------------------------------===//
// Check registration.
@@ -694,7 +752,6 @@
BR.Register(new NullDeref(this));
BR.Register(new UndefinedDeref(this));
BR.Register(new UndefBranch(this));
- BR.Register(new DivZero(this));
BR.Register(new UndefResult(this));
BR.Register(new RetStack(this));
BR.Register(new RetUndef(this));
@@ -713,4 +770,5 @@
registerCheck(new CheckAttrNonNull());
registerCheck(new CheckUndefinedArg());
registerCheck(new CheckBadCall());
+ registerCheck(new CheckBadDiv());
}
More information about the cfe-commits
mailing list