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