From 40bc74fde71c42cc0a1df20d03fb10dd4fd251d5 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Fri, 7 Mar 2008 19:04:53 +0000 Subject: [PATCH] Refined divide-by-zero checking to distinguish between must and may divide-by-zero errors. llvm-svn: 48013 --- clang/Analysis/GRExprEngine.cpp | 68 ++++++++++++------- clang/Analysis/GRSimpleVals.cpp | 4 +- .../Analysis/PathSensitive/GRExprEngine.h | 38 +++++++++-- 3 files changed, 75 insertions(+), 35 deletions(-) diff --git a/clang/Analysis/GRExprEngine.cpp b/clang/Analysis/GRExprEngine.cpp index 3b4b787d6a69..082e124765a2 100644 --- a/clang/Analysis/GRExprEngine.cpp +++ b/clang/Analysis/GRExprEngine.cpp @@ -1040,7 +1040,7 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, if (DivUndef) { DivUndef->markAsSink(); - BadDivides.insert(DivUndef); + ExplicitBadDivides.insert(DivUndef); } continue; @@ -1050,21 +1050,28 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, // // First, "assume" that the denominator is 0 or undefined. - bool isFeasible = false; - ValueState* ZeroSt = Assume(St, RightV, false, isFeasible); - - if (isFeasible) - if (NodeTy* DivZeroNode = Builder->generateNode(B, ZeroSt, N2)) { - DivZeroNode->markAsSink(); - BadDivides.insert(DivZeroNode); - } + bool isFeasibleZero = false; + ValueState* ZeroSt = Assume(St, RightV, false, isFeasibleZero); // Second, "assume" that the denominator cannot be 0. - isFeasible = false; - St = Assume(St, RightV, true, isFeasible); + bool isFeasibleNotZero = false; + St = Assume(St, RightV, true, isFeasibleNotZero); - if (!isFeasible) + // Create the node for the divide-by-zero (if it occurred). + + if (isFeasibleZero) + if (NodeTy* DivZeroNode = Builder->generateNode(B, ZeroSt, N2)) { + DivZeroNode->markAsSink(); + + if (isFeasibleNotZero) + ImplicitBadDivides.insert(DivZeroNode); + else + ExplicitBadDivides.insert(DivZeroNode); + + } + + if (!isFeasibleNotZero) continue; } @@ -1208,7 +1215,7 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, if (DivUndef) { DivUndef->markAsSink(); - BadDivides.insert(DivUndef); + ExplicitBadDivides.insert(DivUndef); } continue; @@ -1216,24 +1223,30 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, // First, "assume" that the denominator is 0. - bool isFeasible = false; - ValueState* ZeroSt = Assume(St, RightV, false, isFeasible); + bool isFeasibleZero = false; + ValueState* ZeroSt = Assume(St, RightV, false, isFeasibleZero); - if (isFeasible) { + // Second, "assume" that the denominator cannot be 0. + + bool isFeasibleNotZero = false; + St = Assume(St, RightV, true, isFeasibleNotZero); + + // Create the node for the divide-by-zero error (if it occurred). + + if (isFeasibleZero) { NodeTy* DivZeroNode = Builder->generateNode(B, ZeroSt, N2); if (DivZeroNode) { DivZeroNode->markAsSink(); - BadDivides.insert(DivZeroNode); + + if (isFeasibleNotZero) + ImplicitBadDivides.insert(DivZeroNode); + else + ExplicitBadDivides.insert(DivZeroNode); } } - // Second, "assume" that the denominator cannot be 0. - - isFeasible = false; - St = Assume(St, RightV, true, isFeasible); - - if (!isFeasible) + if (!isFeasibleNotZero) continue; // Fall-through. The logic below processes the divide. @@ -1659,7 +1672,8 @@ struct VISIBILITY_HIDDEN DOTGraphTraits : GraphPrintCheckerState->isUndefDeref(N) || GraphPrintCheckerState->isUndefStore(N) || GraphPrintCheckerState->isUndefControlFlow(N) || - GraphPrintCheckerState->isBadDivide(N) || + GraphPrintCheckerState->isExplicitBadDivide(N) || + GraphPrintCheckerState->isImplicitBadDivide(N) || GraphPrintCheckerState->isUndefResult(N) || GraphPrintCheckerState->isBadCall(N) || GraphPrintCheckerState->isUndefArg(N)) @@ -1702,8 +1716,10 @@ struct VISIBILITY_HIDDEN DOTGraphTraits : Out << "\\|Dereference of undefialied value.\\l"; else if (GraphPrintCheckerState->isUndefStore(N)) Out << "\\|Store to Undefined LVal."; - else if (GraphPrintCheckerState->isBadDivide(N)) - Out << "\\|Divide-by zero or undefined value."; + else if (GraphPrintCheckerState->isExplicitBadDivide(N)) + Out << "\\|Explicit divide-by zero or undefined value."; + else if (GraphPrintCheckerState->isImplicitBadDivide(N)) + Out << "\\|Implicit divide-by zero or undefined value."; else if (GraphPrintCheckerState->isUndefResult(N)) Out << "\\|Result of operation is undefined."; else if (GraphPrintCheckerState->isNoReturnCall(N)) diff --git a/clang/Analysis/GRSimpleVals.cpp b/clang/Analysis/GRSimpleVals.cpp index af8ea2f4bbb0..95cf3bbbe40c 100644 --- a/clang/Analysis/GRSimpleVals.cpp +++ b/clang/Analysis/GRSimpleVals.cpp @@ -121,8 +121,8 @@ unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx, "Dereference of undefined value."); EmitWarning(Diag, SrcMgr, - CheckerState->bad_divides_begin(), - CheckerState->bad_divides_end(), + CheckerState->explicit_bad_divides_begin(), + CheckerState->explicit_bad_divides_end(), "Division by zero/undefined value."); EmitWarning(Diag, SrcMgr, diff --git a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h b/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h index 6a7d243add51..cd8d555886ae 100644 --- a/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/clang/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -98,9 +98,15 @@ protected: /// taking a dereference on an undefined value. BadDerefTy UndefDeref; - /// BadDivides - Nodes in the ExplodedGraph that result from evaluating - /// a divide-by-zero or divide-by-undefined. - BadDividesTy BadDivides; + /// ImplicitBadDivides - Nodes in the ExplodedGraph that result from + /// evaluating a divide or modulo operation where the denominator + /// MAY be zero. + BadDividesTy ImplicitBadDivides; + + /// ExplicitBadDivides - Nodes in the ExplodedGraph that result from + /// evaluating a divide or modulo operation where the denominator + /// MUST be zero or undefined. + BadDividesTy ExplicitBadDivides; /// UndefResults - Nodes in the ExplodedGraph where the operands are defined /// by the result is not. Excludes divide-by-zero errors. @@ -169,8 +175,12 @@ public: return N->isSink() && UndefDeref.count(const_cast(N)) != 0; } - bool isBadDivide(const NodeTy* N) const { - return N->isSink() && BadDivides.count(const_cast(N)) != 0; + bool isImplicitBadDivide(const NodeTy* N) const { + return N->isSink() && ImplicitBadDivides.count(const_cast(N)) != 0; + } + + bool isExplicitBadDivide(const NodeTy* N) const { + return N->isSink() && ExplicitBadDivides.count(const_cast(N)) != 0; } bool isNoReturnCall(const NodeTy* N) const { @@ -199,8 +209,22 @@ public: undef_deref_iterator undef_derefs_end() { return UndefDeref.end(); } typedef BadDividesTy::iterator bad_divide_iterator; - bad_divide_iterator bad_divides_begin() { return BadDivides.begin(); } - bad_divide_iterator bad_divides_end() { return BadDivides.end(); } + + bad_divide_iterator explicit_bad_divides_begin() { + return ExplicitBadDivides.begin(); + } + + bad_divide_iterator explicit_bad_divides_end() { + return ExplicitBadDivides.end(); + } + + bad_divide_iterator implicit_bad_divides_begin() { + return ImplicitBadDivides.begin(); + } + + bad_divide_iterator implicit_bad_divides_end() { + return ImplicitBadDivides.end(); + } typedef UndefResultsTy::iterator undef_result_iterator; undef_result_iterator undef_results_begin() { return UndefResults.begin(); }