r212731 - [analyzer] Check for code testing a variable for 0 after using it as a denominator.
Jordan Rose
jordan_rose at apple.com
Thu Jul 10 09:10:52 PDT 2014
Author: jrose
Date: Thu Jul 10 11:10:52 2014
New Revision: 212731
URL: http://llvm.org/viewvc/llvm-project?rev=212731&view=rev
Log:
[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)
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
cfe/trunk/test/Analysis/test-after-div-zero.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=212731&r1=212730&r2=212731&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Thu Jul 10 11:10:52 2014
@@ -174,8 +174,13 @@ public:
return Pred->getLocationContext()->getAnalysisDeclContext();
}
- /// \brief If the given node corresponds to a PostStore program point, retrieve
- /// the location region as it was uttered in the code.
+ /// \brief Get the blockID.
+ unsigned getBlockID() const {
+ return NB.getContext().getBlock()->getBlockID();
+ }
+
+ /// \brief If the given node corresponds to a PostStore program point,
+ /// retrieve the location region as it was uttered in the code.
///
/// This utility can be useful for generating extensive diagnostics, for
/// example, for finding variables that the given symbol was assigned to.
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=212731&r1=212730&r2=212731&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Thu Jul 10 11:10:52 2014
@@ -64,6 +64,7 @@ add_clang_library(clangStaticAnalyzerChe
StackAddrEscapeChecker.cpp
StreamChecker.cpp
TaintTesterChecker.cpp
+ TestAfterDivZeroChecker.cpp
TraversalChecker.cpp
UndefBranchChecker.cpp
UndefCapturedBlockVarChecker.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=212731&r1=212730&r2=212731&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Thu Jul 10 11:10:52 2014
@@ -124,6 +124,10 @@ def CallAndMessageUnInitRefArg : Checker
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)">,
DescFile<"CallAndMessageChecker.cpp">;
+def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
+ HelpText<"Check for division by variable that is later compared against 0. Either the comparison is useless or there is division by zero.">,
+ DescFile<"TestAfterDivZeroChecker.cpp">;
+
} // end "alpha.core"
//===----------------------------------------------------------------------===//
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp?rev=212731&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp Thu Jul 10 11:10:52 2014
@@ -0,0 +1,264 @@
+//== TestAfterDivZeroChecker.cpp - Test after division by zero checker --*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines TestAfterDivZeroChecker, a builtin check that performs checks
+// for division by zero where the division occurs before comparison with zero.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/FoldingSet.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class ZeroState {
+private:
+ SymbolRef ZeroSymbol;
+ unsigned BlockID;
+ const StackFrameContext *SFC;
+
+public:
+ ZeroState(SymbolRef S, unsigned B, const StackFrameContext *SFC)
+ : ZeroSymbol(S), BlockID(B), SFC(SFC) {}
+
+ const StackFrameContext *getStackFrameContext() const { return SFC; }
+
+ bool operator==(const ZeroState &X) const {
+ return BlockID == X.BlockID && SFC == X.SFC && ZeroSymbol == X.ZeroSymbol;
+ }
+
+ bool operator<(const ZeroState &X) const {
+ if (BlockID != X.BlockID)
+ return BlockID < X.BlockID;
+ if (SFC != X.SFC)
+ return SFC < X.SFC;
+ return ZeroSymbol < X.ZeroSymbol;
+ }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger(BlockID);
+ ID.AddPointer(SFC);
+ ID.AddPointer(ZeroSymbol);
+ }
+};
+
+class DivisionBRVisitor : public BugReporterVisitorImpl<DivisionBRVisitor> {
+private:
+ SymbolRef ZeroSymbol;
+ const StackFrameContext *SFC;
+ bool Satisfied = false;
+
+public:
+ DivisionBRVisitor(SymbolRef ZeroSymbol, const StackFrameContext *SFC)
+ : ZeroSymbol(ZeroSymbol), SFC(SFC) {}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.Add(ZeroSymbol);
+ ID.Add(SFC);
+ }
+
+ PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+};
+
+class TestAfterDivZeroChecker
+ : public Checker<check::PreStmt<BinaryOperator>, check::BranchCondition,
+ check::EndFunction> {
+ mutable std::unique_ptr<BuiltinBug> DivZeroBug;
+ void reportBug(SVal Val, CheckerContext &C) const;
+
+public:
+ void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
+ void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const;
+ void checkEndFunction(CheckerContext &C) const;
+ void setDivZeroMap(SVal Var, CheckerContext &C) const;
+ bool hasDivZeroMap(SVal Var, const CheckerContext &C) const;
+ bool isZero(SVal S, CheckerContext &C) const;
+};
+} // end anonymous namespace
+
+REGISTER_SET_WITH_PROGRAMSTATE(DivZeroMap, ZeroState)
+
+PathDiagnosticPiece *DivisionBRVisitor::VisitNode(const ExplodedNode *Succ,
+ const ExplodedNode *Pred,
+ BugReporterContext &BRC,
+ BugReport &BR) {
+ if (Satisfied)
+ return nullptr;
+
+ const Expr *E = nullptr;
+
+ if (Optional<PostStmt> P = Succ->getLocationAs<PostStmt>())
+ if (const BinaryOperator *BO = P->getStmtAs<BinaryOperator>()) {
+ BinaryOperator::Opcode Op = BO->getOpcode();
+ if (Op == BO_Div || Op == BO_Rem || Op == BO_DivAssign ||
+ Op == BO_RemAssign) {
+ E = BO->getRHS();
+ }
+ }
+
+ if (!E)
+ return nullptr;
+
+ ProgramStateRef State = Succ->getState();
+ SVal S = State->getSVal(E, Succ->getLocationContext());
+ if (ZeroSymbol == S.getAsSymbol() && SFC == Succ->getStackFrame()) {
+ Satisfied = true;
+
+ // Construct a new PathDiagnosticPiece.
+ ProgramPoint P = Succ->getLocation();
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::create(P, BRC.getSourceManager());
+
+ if (!L.isValid() || !L.asLocation().isValid())
+ return nullptr;
+
+ return new PathDiagnosticEventPiece(
+ L, "Division with compared value made here");
+ }
+
+ return nullptr;
+}
+
+bool TestAfterDivZeroChecker::isZero(SVal S, CheckerContext &C) const {
+ Optional<DefinedSVal> DSV = S.getAs<DefinedSVal>();
+
+ if (!DSV)
+ return false;
+
+ ConstraintManager &CM = C.getConstraintManager();
+ return !CM.assume(C.getState(), *DSV, true);
+}
+
+void TestAfterDivZeroChecker::setDivZeroMap(SVal Var, CheckerContext &C) const {
+ SymbolRef SR = Var.getAsSymbol();
+ if (!SR)
+ return;
+
+ ProgramStateRef State = C.getState();
+ State =
+ State->add<DivZeroMap>(ZeroState(SR, C.getBlockID(), C.getStackFrame()));
+ C.addTransition(State);
+}
+
+bool TestAfterDivZeroChecker::hasDivZeroMap(SVal Var,
+ const CheckerContext &C) const {
+ SymbolRef SR = Var.getAsSymbol();
+ if (!SR)
+ return false;
+
+ ZeroState ZS(SR, C.getBlockID(), C.getStackFrame());
+ return C.getState()->contains<DivZeroMap>(ZS);
+}
+
+void TestAfterDivZeroChecker::reportBug(SVal Val, CheckerContext &C) const {
+ if (ExplodedNode *N = C.generateSink(C.getState())) {
+ if (!DivZeroBug)
+ DivZeroBug.reset(new BuiltinBug(this, "Division by zero"));
+
+ BugReport *R =
+ new BugReport(*DivZeroBug, "Value being compared against zero has "
+ "already been used for division",
+ N);
+
+ R->addVisitor(new DivisionBRVisitor(Val.getAsSymbol(), C.getStackFrame()));
+ C.emitReport(R);
+ }
+}
+
+void TestAfterDivZeroChecker::checkEndFunction(CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+
+ DivZeroMapTy DivZeroes = State->get<DivZeroMap>();
+ if (DivZeroes.isEmpty())
+ return;
+
+ DivZeroMapTy::Factory &F = State->get_context<DivZeroMap>();
+ for (llvm::ImmutableSet<ZeroState>::iterator I = DivZeroes.begin(),
+ E = DivZeroes.end();
+ I != E; ++I) {
+ ZeroState ZS = *I;
+ if (ZS.getStackFrameContext() == C.getStackFrame())
+ DivZeroes = F.remove(DivZeroes, ZS);
+ }
+ C.addTransition(State->set<DivZeroMap>(DivZeroes));
+}
+
+void TestAfterDivZeroChecker::checkPreStmt(const BinaryOperator *B,
+ CheckerContext &C) const {
+ BinaryOperator::Opcode Op = B->getOpcode();
+ if (Op == BO_Div || Op == BO_Rem || Op == BO_DivAssign ||
+ Op == BO_RemAssign) {
+ SVal S = C.getSVal(B->getRHS());
+
+ if (!isZero(S, C))
+ setDivZeroMap(S, C);
+ }
+}
+
+void TestAfterDivZeroChecker::checkBranchCondition(const Stmt *Condition,
+ CheckerContext &C) const {
+ if (const BinaryOperator *B = dyn_cast<BinaryOperator>(Condition)) {
+ if (B->isComparisonOp()) {
+ const IntegerLiteral *IntLiteral = dyn_cast<IntegerLiteral>(B->getRHS());
+ bool LRHS = true;
+ if (!IntLiteral) {
+ IntLiteral = dyn_cast<IntegerLiteral>(B->getLHS());
+ LRHS = false;
+ }
+
+ if (!IntLiteral || IntLiteral->getValue() != 0)
+ return;
+
+ SVal Val = C.getSVal(LRHS ? B->getLHS() : B->getRHS());
+ if (hasDivZeroMap(Val, C))
+ reportBug(Val, C);
+ }
+ } else if (const UnaryOperator *U = dyn_cast<UnaryOperator>(Condition)) {
+ if (U->getOpcode() == UO_LNot) {
+ SVal Val;
+ if (const ImplicitCastExpr *I =
+ dyn_cast<ImplicitCastExpr>(U->getSubExpr()))
+ Val = C.getSVal(I->getSubExpr());
+
+ if (hasDivZeroMap(Val, C))
+ reportBug(Val, C);
+ else {
+ Val = C.getSVal(U->getSubExpr());
+ if (hasDivZeroMap(Val, C))
+ reportBug(Val, C);
+ }
+ }
+ } else if (const ImplicitCastExpr *IE =
+ dyn_cast<ImplicitCastExpr>(Condition)) {
+ SVal Val = C.getSVal(IE->getSubExpr());
+
+ if (hasDivZeroMap(Val, C))
+ reportBug(Val, C);
+ else {
+ SVal Val = C.getSVal(Condition);
+
+ if (hasDivZeroMap(Val, C))
+ reportBug(Val, C);
+ }
+ }
+}
+
+void ento::registerTestAfterDivZeroChecker(CheckerManager &mgr) {
+ mgr.registerChecker<TestAfterDivZeroChecker>();
+}
Added: cfe/trunk/test/Analysis/test-after-div-zero.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/test-after-div-zero.c?rev=212731&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/test-after-div-zero.c (added)
+++ cfe/trunk/test/Analysis/test-after-div-zero.c Thu Jul 10 11:10:52 2014
@@ -0,0 +1,204 @@
+// RUN: %clang_cc1 -std=c99 -Dbool=_Bool -analyze -analyzer-checker=core,alpha.core.TestAfterDivZero -analyzer-output=text -verify %s
+// RUN: %clang_cc1 -x c++ -analyze -analyzer-checker=core,alpha.core.TestAfterDivZero -analyzer-output=text -verify %s
+
+int var;
+
+void err_eq(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x == 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_eq2(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (0 == x) { } // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_ne(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x != 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_ge(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x >= 0) { } // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_le(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x <= 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_yes(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+void err_not(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_pnot(int x) {
+ int *y = &x;
+ var = 77 / *y; // expected-note {{Division with compared value made here}}
+ if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_pnot2(int x) {
+ int *y = &x;
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (!*y) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_ppnot(int x) {
+ int *y = &x;
+ int **z = &y;
+ var = 77 / **z; // expected-note {{Division with compared value made here}}
+ if (!x) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_orig_checker(int x) {
+ if (x != 0) // expected-note {{Assuming 'x' is equal to 0}} expected-note {{Taking false branch}}
+ return;
+ var = 77 / x; // expected-warning {{Division by zero}} expected-note {{Division by zero}}
+ if (!x) {} // no-warning
+}
+
+void ok_other(int x, int y) {
+ var = 77 / y;
+ if (x == 0) {
+ }
+}
+
+void ok_assign(int x) {
+ var = 77 / x;
+ x = var / 77; // <- assignment => don't warn
+ if (x == 0) {
+ }
+}
+
+void ok_assign2(int x) {
+ var = 77 / x;
+ x = var / 77; // <- assignment => don't warn
+ if (0 == x) {
+ }
+}
+
+void ok_dec(int x) {
+ var = 77 / x;
+ x--; // <- assignment => don't warn
+ if (x == 0) {
+ }
+}
+
+void ok_inc(int x) {
+ var = 77 / x;
+ x++; // <- assignment => don't warn
+ if (x == 0) {
+ }
+}
+
+void do_something_ptr(int *x);
+void ok_callfunc_ptr(int x) {
+ var = 77 / x;
+ do_something_ptr(&x); // <- pass address of x to function => don't warn
+ if (x == 0) {
+ }
+}
+
+void do_something(int x);
+void nok_callfunc(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ do_something(x);
+ if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void ok_if(int x) {
+ if (x > 3)
+ var = 77 / x;
+ if (x == 0) {
+ }
+}
+
+void ok_if2(int x) {
+ if (x < 3)
+ var = 77 / x;
+ if (x == 0) {
+ } // TODO warn here
+}
+
+void ok_pif(int x) {
+ int *y = &x;
+ if (x < 3)
+ var = 77 / *y;
+ if (x == 0) {
+ } // TODO warn here
+}
+
+int getValue(bool *isPositive);
+void use(int a);
+void foo() {
+ bool isPositive;
+ int x = getValue(&isPositive);
+ if (isPositive) {
+ use(5 / x);
+ }
+
+ if (x == 0) {
+ }
+}
+
+int getValue2();
+void foo2() {
+ int x = getValue2();
+ int y = x;
+
+ use(5 / x); // expected-note {{Division with compared value made here}}
+ if (y == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void ok_while(int x) {
+ int n = 100 / x;
+ while (x != 0) { // <- do not warn
+ x--;
+ }
+}
+
+void err_not2(int x, int y) {
+ int v;
+ var = 77 / x;
+
+ if (y)
+ v = 0;
+
+ if (!x) {
+ } // TODO warn here
+}
+
+inline void inline_func(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+void err_inline(int x) {
+ var = 77 / x;
+ inline_func(x); // expected-note {{Calling 'inline_func'}}
+ if (x == 0) {
+ }
+}
+
+inline void inline_func2(int x) {}
+
+void err_inline2(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ inline_func2(x);
+ if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
+
+inline void inline_func3(int x) {
+ var = 77 / x;
+}
+void ok_inline(int x) {
+ var = 77 / x; // expected-note {{Division with compared value made here}}
+ inline_func3(x);
+ if (x == 0) {} // expected-warning {{Value being compared against zero has already been used for division}}
+} // expected-note at -1 {{Value being compared against zero has already been used for division}}
More information about the cfe-commits
mailing list