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

Lines changed: 7 additions & 2 deletions
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 4 additions & 0 deletions
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

Lines changed: 264 additions & 0 deletions
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+
}

‎clang/test/Analysis/test-after-div-zero.c

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
// RUN: %clang_cc1 -std=c99 -Dbool=_Bool -analyze -analyzer-checker=core,alpha.core.TestAfterDivZero -analyzer-output=text -verify %s
2+
// RUN: %clang_cc1 -x c++ -analyze -analyzer-checker=core,alpha.core.TestAfterDivZero -analyzer-output=text -verify %s
3+
4+
int var;
5+
6+
void err_eq(int x) {
7+
var = 77 / x; // expected-note {{Division with compared value made here}}
8+
if (x == 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
9+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
10+
11+
void err_eq2(int x) {
12+
var = 77 / x; // expected-note {{Division with compared value made here}}
13+
if (0 == x) { } // expected-warning {{Value being compared against zero has already been used for division}}
14+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
15+
16+
void err_ne(int x) {
17+
var = 77 / x; // expected-note {{Division with compared value made here}}
18+
if (x != 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
19+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
20+
21+
void err_ge(int x) {
22+
var = 77 / x; // expected-note {{Division with compared value made here}}
23+
if (x >= 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
24+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
25+
26+
void err_le(int x) {
27+
var = 77 / x; // expected-note {{Division with compared value made here}}
28+
if (x <= 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
29+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
30+
31+
void err_yes(int x) {
32+
var = 77 / x; // expected-note {{Division with compared value made here}}
33+
if (x) {} // expected-warning {{Value being compared against zero has already been used for division}}
34+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
35+
void err_not(int x) {
36+
var = 77 / x; // expected-note {{Division with compared value made here}}
37+
if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
38+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
39+
40+
void err_pnot(int x) {
41+
int *y = &x;
42+
var = 77 / *y; // expected-note {{Division with compared value made here}}
43+
if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
44+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
45+
46+
void err_pnot2(int x) {
47+
int *y = &x;
48+
var = 77 / x; // expected-note {{Division with compared value made here}}
49+
if (!*y) {} // expected-warning {{Value being compared against zero has already been used for division}}
50+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
51+
52+
void err_ppnot(int x) {
53+
int *y = &x;
54+
int **z = &y;
55+
var = 77 / **z; // expected-note {{Division with compared value made here}}
56+
if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
57+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
58+
59+
void err_orig_checker(int x) {
60+
if (x != 0) // expected-note {{Assuming 'x' is equal to 0}} expected-note {{Taking false branch}}
61+
return;
62+
var = 77 / x; // expected-warning {{Division by zero}} expected-note {{Division by zero}}
63+
if (!x) {} // no-warning
64+
}
65+
66+
void ok_other(int x, int y) {
67+
var = 77 / y;
68+
if (x == 0) {
69+
}
70+
}
71+
72+
void ok_assign(int x) {
73+
var = 77 / x;
74+
x = var / 77; // <- assignment => don't warn
75+
if (x == 0) {
76+
}
77+
}
78+
79+
void ok_assign2(int x) {
80+
var = 77 / x;
81+
x = var / 77; // <- assignment => don't warn
82+
if (0 == x) {
83+
}
84+
}
85+
86+
void ok_dec(int x) {
87+
var = 77 / x;
88+
x--; // <- assignment => don't warn
89+
if (x == 0) {
90+
}
91+
}
92+
93+
void ok_inc(int x) {
94+
var = 77 / x;
95+
x++; // <- assignment => don't warn
96+
if (x == 0) {
97+
}
98+
}
99+
100+
void do_something_ptr(int *x);
101+
void ok_callfunc_ptr(int x) {
102+
var = 77 / x;
103+
do_something_ptr(&x); // <- pass address of x to function => don't warn
104+
if (x == 0) {
105+
}
106+
}
107+
108+
void do_something(int x);
109+
void nok_callfunc(int x) {
110+
var = 77 / x; // expected-note {{Division with compared value made here}}
111+
do_something(x);
112+
if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
113+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
114+
115+
void ok_if(int x) {
116+
if (x > 3)
117+
var = 77 / x;
118+
if (x == 0) {
119+
}
120+
}
121+
122+
void ok_if2(int x) {
123+
if (x < 3)
124+
var = 77 / x;
125+
if (x == 0) {
126+
} // TODO warn here
127+
}
128+
129+
void ok_pif(int x) {
130+
int *y = &x;
131+
if (x < 3)
132+
var = 77 / *y;
133+
if (x == 0) {
134+
} // TODO warn here
135+
}
136+
137+
int getValue(bool *isPositive);
138+
void use(int a);
139+
void foo() {
140+
bool isPositive;
141+
int x = getValue(&isPositive);
142+
if (isPositive) {
143+
use(5 / x);
144+
}
145+
146+
if (x == 0) {
147+
}
148+
}
149+
150+
int getValue2();
151+
void foo2() {
152+
int x = getValue2();
153+
int y = x;
154+
155+
use(5 / x); // expected-note {{Division with compared value made here}}
156+
if (y == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
157+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
158+
159+
void ok_while(int x) {
160+
int n = 100 / x;
161+
while (x != 0) { // <- do not warn
162+
x--;
163+
}
164+
}
165+
166+
void err_not2(int x, int y) {
167+
int v;
168+
var = 77 / x;
169+
170+
if (y)
171+
v = 0;
172+
173+
if (!x) {
174+
} // TODO warn here
175+
}
176+
177+
inline void inline_func(int x) {
178+
var = 77 / x; // expected-note {{Division with compared value made here}}
179+
if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
180+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
181+
182+
void err_inline(int x) {
183+
var = 77 / x;
184+
inline_func(x); // expected-note {{Calling 'inline_func'}}
185+
if (x == 0) {
186+
}
187+
}
188+
189+
inline void inline_func2(int x) {}
190+
191+
void err_inline2(int x) {
192+
var = 77 / x; // expected-note {{Division with compared value made here}}
193+
inline_func2(x);
194+
if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
195+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}
196+
197+
inline void inline_func3(int x) {
198+
var = 77 / x;
199+
}
200+
void ok_inline(int x) {
201+
var = 77 / x; // expected-note {{Division with compared value made here}}
202+
inline_func3(x);
203+
if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
204+
} // expected-note@-1 {{Value being compared against zero has already been used for division}}

0 commit comments

Comments
 (0)
Please sign in to comment.