From bfd9470600be96206f2e98e90394d0f2a06d2042 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 13 Aug 2022 01:27:20 -0500 Subject: [PATCH] Fix 11158: FP zerodiv in loop (#4356) * Fix 11158: FP zerodiv in loop * Format * Add another test * Format * Fix FP * Format --- lib/valueflow.cpp | 82 +++++++++++++++++++++++++++++++++------------- test/testother.cpp | 26 +++++++++++++++ 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 68465f673..38729b4e6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -510,6 +510,10 @@ void combineValueProperties(const ValueFlow::Value &value1, const ValueFlow::Val result->setInconclusive(); else result->setPossible(); + if (value1.tokvalue) + result->tokvalue = value1.tokvalue; + else if (value2.tokvalue) + result->tokvalue = value2.tokvalue; if (value1.isSymbolicValue()) { result->valueType = value1.valueType; result->tokvalue = value1.tokvalue; @@ -2465,10 +2469,7 @@ struct ValueFlowAnalyzer : Analyzer { // Check if its assigned to the same value if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) && astIsIntegral(tok->astParent()->astOperand2(), false)) { - std::vector result = - evaluateInt(tok->astParent()->astOperand2(), [&] { - return ProgramMemory{getProgramState()}; - }); + std::vector result = evaluateInt(tok->astParent()->astOperand2()); if (!result.empty() && value->equalTo(result.front())) return Action::Idempotent; } @@ -2541,23 +2542,29 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; return Action::Read | Action::Write; } - if (parent && parent->isAssignmentOp() && astIsLHS(tok) && - parent->astOperand2()->hasKnownValue()) { + if (parent && parent->isAssignmentOp() && astIsLHS(tok)) { const Token* rhs = parent->astOperand2(); - const ValueFlow::Value* rhsValue = rhs->getKnownValue(ValueFlow::Value::ValueType::INT); - Action a; - if (!rhsValue || !evalAssignment(*value, getAssign(parent, d), *rhsValue)) - a = Action::Invalid; - else - a = Action::Write; - if (parent->str() != "=") { - a |= Action::Read; - } else { - if (rhsValue && !value->isImpossible() && value->equalValue(*rhsValue)) - a = Action::Idempotent; - a |= Action::Incremental; + std::vector result = evaluateInt(rhs); + if (!result.empty()) { + ValueFlow::Value rhsValue{result.front()}; + Action a; + if (!evalAssignment(*value, getAssign(parent, d), rhsValue)) + a = Action::Invalid; + else + a = Action::Write; + if (parent->str() != "=") { + a |= Action::Read | Action::Incremental; + } else { + if (!value->isImpossible() && value->equalValue(rhsValue)) + a = Action::Idempotent; + if (tok->exprId() != 0 && + findAstNode(rhs, [&](const Token* child) { + return tok->exprId() == child->exprId(); + })) + a |= Action::Incremental; + } + return a; } - return a; } // increment/decrement @@ -2576,10 +2583,11 @@ struct ValueFlowAnalyzer : Analyzer { if (value->isLifetimeValue()) return; if (tok->astParent()->isAssignmentOp()) { - const ValueFlow::Value* rhsValue = - tok->astParent()->astOperand2()->getKnownValue(ValueFlow::Value::ValueType::INT); - assert(rhsValue); - if (evalAssignment(*value, getAssign(tok->astParent(), d), *rhsValue)) { + const Token* rhs = tok->astParent()->astOperand2(); + std::vector result = evaluateInt(rhs); + assert(!result.empty()); + ValueFlow::Value rhsValue{result.front()}; + if (evalAssignment(*value, getAssign(tok->astParent(), d), rhsValue)) { std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " + value->infoString()); if (tok->astParent()->str() == "=") @@ -2777,6 +2785,12 @@ struct ValueFlowAnalyzer : Analyzer { } return result; } + std::vector evaluateInt(const Token* tok) const + { + return evaluateInt(tok, [&] { + return ProgramMemory{getProgramState()}; + }); + } std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const override { @@ -5536,6 +5550,28 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat return value.tokvalue->exprId() == tok->astOperand1()->exprId(); return false; }); + // Find references to LHS in RHS + auto isIncremental = [&](const Token* tok2) -> bool { + return findAstNode(tok2, + [&](const Token* child) { + return child->exprId() == tok->astOperand1()->exprId(); + }); + }; + // Check symbolic values as well + const bool incremental = isIncremental(tok->astOperand2()) || + std::any_of(values.begin(), values.end(), [&](const ValueFlow::Value& value) { + if (!value.isSymbolicValue()) + return false; + return isIncremental(value.tokvalue); + }); + // Remove values from the same assignment if it is incremental + if (incremental) { + values.remove_if([&](const ValueFlow::Value& value) { + if (value.tokvalue) + return value.tokvalue == tok->astOperand2(); + return false; + }); + } // If assignment copy by value, remove Uninit values.. if ((tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer == 0) || (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference() && tok->astOperand1()->variable()->nameToken() == tok->astOperand1())) diff --git a/test/testother.cpp b/test/testother.cpp index 7ba66b00f..0f57208ab 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -66,6 +66,7 @@ private: TEST_CASE(zeroDiv13); TEST_CASE(zeroDiv14); // #1169 TEST_CASE(zeroDiv15); // #8319 + TEST_CASE(zeroDiv16); // #11158 TEST_CASE(zeroDivCond); // division by zero / useless condition @@ -611,6 +612,31 @@ private: ASSERT_EQUALS("[test.cpp:4]: (error) Division by zero.\n", errout.str()); } + // #11158 + void zeroDiv16() + { + check("int f(int i) {\n" + " int number = 10, a = 0;\n" + " for (int count = 0; count < 2; count++) {\n" + " a += (i / number) % 10;\n" + " number = number / 10;\n" + " }\n" + " return a;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("int f(int i) {\n" + " int number = 10, a = 0;\n" + " for (int count = 0; count < 2; count++) {\n" + " int x = number / 10;\n" + " a += (i / number) % 10;\n" + " number = x;\n" + " }\n" + " return a;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void zeroDivCond() { check("void f(unsigned int x) {\n" " int y = 17 / x;\n"