Skip to content

Commit dc352bb

Browse files
jrose-applejrose-apple
committedJul 10, 2014
[analyzer] Check for code testing a variable for 0 after using it as a denominator.
This new checker, alpha.core.TestAfterDivZero, catches issues like this: int sum = ... int avg = sum / count; // potential division by zero... if (count == 0) { ... } // ...caught here Because the analyzer does not necessarily explore /all/ paths through a program, this check is restricted to only work on zero checks that immediately follow a division operation (/ % /= %=). This could later be expanded to handle checks dominated by a division operation but not necessarily in the same CFG block. Patch by Anders Rönnholm! (with very minor modifications by me) llvm-svn: 212731
1 parent 511fea7 commit dc352bb

File tree

5 files changed

+480
-2
lines changed

5 files changed

+480
-2
lines changed
 

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

+7-2
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,13 @@ class CheckerContext {
174174
return Pred->getLocationContext()->getAnalysisDeclContext();
175175
}
176176

177-
/// \brief If the given node corresponds to a PostStore program point, retrieve
178-
/// the location region as it was uttered in the code.
177+
/// \brief Get the blockID.
178+
unsigned getBlockID() const {
179+
return NB.getContext().getBlock()->getBlockID();
180+
}
181+
182+
/// \brief If the given node corresponds to a PostStore program point,
183+
/// retrieve the location region as it was uttered in the code.
179184
///
180185
/// This utility can be useful for generating extensive diagnostics, for
181186
/// example, for finding variables that the given symbol was assigned to.

‎clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ add_clang_library(clangStaticAnalyzerCheckers
6464
StackAddrEscapeChecker.cpp
6565
StreamChecker.cpp
6666
TaintTesterChecker.cpp
67+
TestAfterDivZeroChecker.cpp
6768
TraversalChecker.cpp
6869
UndefBranchChecker.cpp
6970
UndefCapturedBlockVarChecker.cpp

‎clang/lib/StaticAnalyzer/Checkers/Checkers.td

+4
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ def CallAndMessageUnInitRefArg : Checker<"CallAndMessageUnInitRefArg">,
124124
HelpText<"Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers, and pointer to undefined variables)">,
125125
DescFile<"CallAndMessageChecker.cpp">;
126126

127+
def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
128+
HelpText<"Check for division by variable that is later compared against 0. Either the comparison is useless or there is division by zero.">,
129+
DescFile<"TestAfterDivZeroChecker.cpp">;
130+
127131
} // end "alpha.core"
128132

129133
//===----------------------------------------------------------------------===//

‎clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp

+264
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
//== TestAfterDivZeroChecker.cpp - Test after division by zero checker --*--==//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
//
10+
// This defines TestAfterDivZeroChecker, a builtin check that performs checks
11+
// for division by zero where the division occurs before comparison with zero.
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
#include "ClangSACheckers.h"
16+
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
17+
#include "clang/StaticAnalyzer/Core/Checker.h"
18+
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
19+
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
20+
#include "llvm/ADT/FoldingSet.h"
21+
22+
using namespace clang;
23+
using namespace ento;
24+
25+
namespace {
26+
27+
class ZeroState {
28+
private:
29+
SymbolRef ZeroSymbol;
30+
unsigned BlockID;
31+
const StackFrameContext *SFC;
32+
33+
public:
34+
ZeroState(SymbolRef S, unsigned B, const StackFrameContext *SFC)
35+
: ZeroSymbol(S), BlockID(B), SFC(SFC) {}
36+
37+
const StackFrameContext *getStackFrameContext() const { return SFC; }
38+
39+
bool operator==(const ZeroState &X) const {
40+
return BlockID == X.BlockID && SFC == X.SFC && ZeroSymbol == X.ZeroSymbol;
41+
}
42+
43+
bool operator<(const ZeroState &X) const {
44+
if (BlockID != X.BlockID)
45+
return BlockID < X.BlockID;
46+
if (SFC != X.SFC)
47+
return SFC < X.SFC;
48+
return ZeroSymbol < X.ZeroSymbol;
49+
}
50+
51+
void Profile(llvm::FoldingSetNodeID &ID) const {
52+
ID.AddInteger(BlockID);
53+
ID.AddPointer(SFC);
54+
ID.AddPointer(ZeroSymbol);
55+
}
56+
};
57+
58+
class DivisionBRVisitor : public BugReporterVisitorImpl<DivisionBRVisitor> {
59+
private:
60+
SymbolRef ZeroSymbol;
61+
const StackFrameContext *SFC;
62+
bool Satisfied = false;
63+
64+
public:
65+
DivisionBRVisitor(SymbolRef ZeroSymbol, const StackFrameContext *SFC)
66+
: ZeroSymbol(ZeroSymbol), SFC(SFC) {}
67+
68+
void Profile(llvm::FoldingSetNodeID &ID) const override {
69+
ID.Add(ZeroSymbol);
70+
ID.Add(SFC);
71+
}
72+
73+
PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
74+
const ExplodedNode *Pred,
75+
BugReporterContext &BRC,
76+
BugReport &BR) override;
77+
};
78+
79+
class TestAfterDivZeroChecker
80+
: public Checker<check::PreStmt<BinaryOperator>, check::BranchCondition,
81+
check::EndFunction> {
82+
mutable std::unique_ptr<BuiltinBug> DivZeroBug;
83+
void reportBug(SVal Val, CheckerContext &C) const;
84+
85+
public:
86+
void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
87+
void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
88+
void checkEndFunction(CheckerContext &C) const;
89+
void setDivZeroMap(SVal Var, CheckerContext &C) const;
90+
bool hasDivZeroMap(SVal Var, const CheckerContext &C) const;
91+
bool isZero(SVal S, CheckerContext &C) const;
92+
};
93+
} // end anonymous namespace
94+
95+
REGISTER_SET_WITH_PROGRAMSTATE(DivZeroMap, ZeroState)
96+
97+
PathDiagnosticPiece *DivisionBRVisitor::VisitNode(const ExplodedNode *Succ,
98+
const ExplodedNode *Pred,
99+
BugReporterContext &BRC,
100+
BugReport &BR) {
101+
if (Satisfied)
102+
return nullptr;
103+
104+
const Expr *E = nullptr;
105+
106+
if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>())
107+
if (const BinaryOperator *BO = P->getStmtAs<BinaryOperator>()) {
108+
BinaryOperator::Opcode Op = BO->getOpcode();
109+
if (Op == BO_Div || Op == BO_Rem || Op == BO_DivAssign ||
110+
Op == BO_RemAssign) {
111+
E = BO->getRHS();
112+
}
113+
}
114+
115+
if (!E)
116+
return nullptr;
117+
118+
ProgramStateRef State = Succ->getState();
119+
SVal S = State->getSVal(E, Succ->getLocationContext());
120+
if (ZeroSymbol == S.getAsSymbol() && SFC == Succ->getStackFrame()) {
121+
Satisfied = true;
122+
123+
// Construct a new PathDiagnosticPiece.
124+
ProgramPoint P = Succ->getLocation();
125+
PathDiagnosticLocation L =
126+
PathDiagnosticLocation::create(P, BRC.getSourceManager());
127+
128+
if (!L.isValid() || !L.asLocation().isValid())
129+
return nullptr;
130+
131+
return new PathDiagnosticEventPiece(
132+
L, "Division with compared value made here");
133+
}
134+
135+
return nullptr;
136+
}
137+
138+
bool TestAfterDivZeroChecker::isZero(SVal S, CheckerContext &C) const {
139+
Optional<DefinedSVal> DSV = S.getAs<DefinedSVal>();
140+
141+
if (!DSV)
142+
return false;
143+
144+
ConstraintManager &CM = C.getConstraintManager();
145+
return !CM.assume(C.getState(), *DSV, true);
146+
}
147+
148+
void TestAfterDivZeroChecker::setDivZeroMap(SVal Var, CheckerContext &C) const {
149+
SymbolRef SR = Var.getAsSymbol();
150+
if (!SR)
151+
return;
152+
153+
ProgramStateRef State = C.getState();
154+
State =
155+
State->add<DivZeroMap>(ZeroState(SR, C.getBlockID(), C.getStackFrame()));
156+
C.addTransition(State);
157+
}
158+
159+
bool TestAfterDivZeroChecker::hasDivZeroMap(SVal Var,
160+
const CheckerContext &C) const {
161+
SymbolRef SR = Var.getAsSymbol();
162+
if (!SR)
163+
return false;
164+
165+
ZeroState ZS(SR, C.getBlockID(), C.getStackFrame());
166+
return C.getState()->contains<DivZeroMap>(ZS);
167+
}
168+
169+
void TestAfterDivZeroChecker::reportBug(SVal Val, CheckerContext &C) const {
170+
if (ExplodedNode *N = C.generateSink(C.getState())) {
171+
if (!DivZeroBug)
172+
DivZeroBug.reset(new BuiltinBug(this, "Division by zero"));
173+
174+
BugReport *R =
175+
new BugReport(*DivZeroBug, "Value being compared against zero has "
176+
"already been used for division",
177+
N);
178+
179+
R->addVisitor(new DivisionBRVisitor(Val.getAsSymbol(), C.getStackFrame()));
180+
C.emitReport(R);
181+
}
182+
}
183+
184+
void TestAfterDivZeroChecker::checkEndFunction(CheckerContext &C) const {
185+
ProgramStateRef State = C.getState();
186+
187+
DivZeroMapTy DivZeroes = State->get<DivZeroMap>();
188+
if (DivZeroes.isEmpty())
189+
return;
190+
191+
DivZeroMapTy::Factory &F = State->get_context<DivZeroMap>();
192+
for (llvm::ImmutableSet<ZeroState>::iterator I = DivZeroes.begin(),
193+
E = DivZeroes.end();
194+
I != E; ++I) {
195+
ZeroState ZS = *I;
196+
if (ZS.getStackFrameContext() == C.getStackFrame())
197+
DivZeroes = F.remove(DivZeroes, ZS);
198+
}
199+
C.addTransition(State->set<DivZeroMap>(DivZeroes));
200+
}
201+
202+
void TestAfterDivZeroChecker::checkPreStmt(const BinaryOperator *B,
203+
CheckerContext &C) const {
204+
BinaryOperator::Opcode Op = B->getOpcode();
205+
if (Op == BO_Div || Op == BO_Rem || Op == BO_DivAssign ||
206+
Op == BO_RemAssign) {
207+
SVal S = C.getSVal(B->getRHS());
208+
209+
if (!isZero(S, C))
210+
setDivZeroMap(S, C);
211+
}
212+
}
213+
214+
void TestAfterDivZeroChecker::checkBranchCondition(const Stmt *Condition,
215+
CheckerContext &C) const {
216+
if (const BinaryOperator *B = dyn_cast<BinaryOperator>(Condition)) {
217+
if (B->isComparisonOp()) {
218+
const IntegerLiteral *IntLiteral = dyn_cast<IntegerLiteral>(B->getRHS());
219+
bool LRHS = true;
220+
if (!IntLiteral) {
221+
IntLiteral = dyn_cast<IntegerLiteral>(B->getLHS());
222+
LRHS = false;
223+
}
224+
225+
if (!IntLiteral || IntLiteral->getValue() != 0)
226+
return;
227+
228+
SVal Val = C.getSVal(LRHS ? B->getLHS() : B->getRHS());
229+
if (hasDivZeroMap(Val, C))
230+
reportBug(Val, C);
231+
}
232+
} else if (const UnaryOperator *U = dyn_cast<UnaryOperator>(Condition)) {
233+
if (U->getOpcode() == UO_LNot) {
234+
SVal Val;
235+
if (const ImplicitCastExpr *I =
236+
dyn_cast<ImplicitCastExpr>(U->getSubExpr()))
237+
Val = C.getSVal(I->getSubExpr());
238+
239+
if (hasDivZeroMap(Val, C))
240+
reportBug(Val, C);
241+
else {
242+
Val = C.getSVal(U->getSubExpr());
243+
if (hasDivZeroMap(Val, C))
244+
reportBug(Val, C);
245+
}
246+
}
247+
} else if (const ImplicitCastExpr *IE =
248+
dyn_cast<ImplicitCastExpr>(Condition)) {
249+
SVal Val = C.getSVal(IE->getSubExpr());
250+
251+
if (hasDivZeroMap(Val, C))
252+
reportBug(Val, C);
253+
else {
254+
SVal Val = C.getSVal(Condition);
255+
256+
if (hasDivZeroMap(Val, C))
257+
reportBug(Val, C);
258+
}
259+
}
260+
}
261+
262+
void ento::registerTestAfterDivZeroChecker(CheckerManager &mgr) {
263+
mgr.registerChecker<TestAfterDivZeroChecker>();
264+
}

0 commit comments

Comments
 (0)