Fix 11158: FP zerodiv in loop (#4356)

* Fix 11158: FP zerodiv in loop

* Format

* Add another test

* Format

* Fix FP

* Format
This commit is contained in:
Paul Fultz II 2022-08-13 01:27:20 -05:00 committed by GitHub
parent bdbd84ba98
commit bfd9470600
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 85 additions and 23 deletions

View File

@ -510,6 +510,10 @@ void combineValueProperties(const ValueFlow::Value &value1, const ValueFlow::Val
result->setInconclusive(); result->setInconclusive();
else else
result->setPossible(); result->setPossible();
if (value1.tokvalue)
result->tokvalue = value1.tokvalue;
else if (value2.tokvalue)
result->tokvalue = value2.tokvalue;
if (value1.isSymbolicValue()) { if (value1.isSymbolicValue()) {
result->valueType = value1.valueType; result->valueType = value1.valueType;
result->tokvalue = value1.tokvalue; result->tokvalue = value1.tokvalue;
@ -2465,10 +2469,7 @@ struct ValueFlowAnalyzer : Analyzer {
// Check if its assigned to the same value // Check if its assigned to the same value
if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) && if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) &&
astIsIntegral(tok->astParent()->astOperand2(), false)) { astIsIntegral(tok->astParent()->astOperand2(), false)) {
std::vector<MathLib::bigint> result = std::vector<MathLib::bigint> result = evaluateInt(tok->astParent()->astOperand2());
evaluateInt(tok->astParent()->astOperand2(), [&] {
return ProgramMemory{getProgramState()};
});
if (!result.empty() && value->equalTo(result.front())) if (!result.empty() && value->equalTo(result.front()))
return Action::Idempotent; return Action::Idempotent;
} }
@ -2541,24 +2542,30 @@ struct ValueFlowAnalyzer : Analyzer {
return Action::None; return Action::None;
return Action::Read | Action::Write; return Action::Read | Action::Write;
} }
if (parent && parent->isAssignmentOp() && astIsLHS(tok) && if (parent && parent->isAssignmentOp() && astIsLHS(tok)) {
parent->astOperand2()->hasKnownValue()) {
const Token* rhs = parent->astOperand2(); const Token* rhs = parent->astOperand2();
const ValueFlow::Value* rhsValue = rhs->getKnownValue(ValueFlow::Value::ValueType::INT); std::vector<MathLib::bigint> result = evaluateInt(rhs);
if (!result.empty()) {
ValueFlow::Value rhsValue{result.front()};
Action a; Action a;
if (!rhsValue || !evalAssignment(*value, getAssign(parent, d), *rhsValue)) if (!evalAssignment(*value, getAssign(parent, d), rhsValue))
a = Action::Invalid; a = Action::Invalid;
else else
a = Action::Write; a = Action::Write;
if (parent->str() != "=") { if (parent->str() != "=") {
a |= Action::Read; a |= Action::Read | Action::Incremental;
} else { } else {
if (rhsValue && !value->isImpossible() && value->equalValue(*rhsValue)) if (!value->isImpossible() && value->equalValue(rhsValue))
a = Action::Idempotent; a = Action::Idempotent;
if (tok->exprId() != 0 &&
findAstNode(rhs, [&](const Token* child) {
return tok->exprId() == child->exprId();
}))
a |= Action::Incremental; a |= Action::Incremental;
} }
return a; return a;
} }
}
// increment/decrement // increment/decrement
if (Token::Match(tok->astParent(), "++|--")) { if (Token::Match(tok->astParent(), "++|--")) {
@ -2576,10 +2583,11 @@ struct ValueFlowAnalyzer : Analyzer {
if (value->isLifetimeValue()) if (value->isLifetimeValue())
return; return;
if (tok->astParent()->isAssignmentOp()) { if (tok->astParent()->isAssignmentOp()) {
const ValueFlow::Value* rhsValue = const Token* rhs = tok->astParent()->astOperand2();
tok->astParent()->astOperand2()->getKnownValue(ValueFlow::Value::ValueType::INT); std::vector<MathLib::bigint> result = evaluateInt(rhs);
assert(rhsValue); assert(!result.empty());
if (evalAssignment(*value, getAssign(tok->astParent(), d), *rhsValue)) { ValueFlow::Value rhsValue{result.front()};
if (evalAssignment(*value, getAssign(tok->astParent(), d), rhsValue)) {
std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " + std::string info("Compound assignment '" + tok->astParent()->str() + "', assigned value is " +
value->infoString()); value->infoString());
if (tok->astParent()->str() == "=") if (tok->astParent()->str() == "=")
@ -2777,6 +2785,12 @@ struct ValueFlowAnalyzer : Analyzer {
} }
return result; return result;
} }
std::vector<MathLib::bigint> evaluateInt(const Token* tok) const
{
return evaluateInt(tok, [&] {
return ProgramMemory{getProgramState()};
});
}
std::vector<MathLib::bigint> evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const override std::vector<MathLib::bigint> 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 value.tokvalue->exprId() == tok->astOperand1()->exprId();
return false; 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 assignment copy by value, remove Uninit values..
if ((tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer == 0) || if ((tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->pointer == 0) ||
(tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference() && tok->astOperand1()->variable()->nameToken() == tok->astOperand1())) (tok->astOperand1()->variable() && tok->astOperand1()->variable()->isReference() && tok->astOperand1()->variable()->nameToken() == tok->astOperand1()))

View File

@ -66,6 +66,7 @@ private:
TEST_CASE(zeroDiv13); TEST_CASE(zeroDiv13);
TEST_CASE(zeroDiv14); // #1169 TEST_CASE(zeroDiv14); // #1169
TEST_CASE(zeroDiv15); // #8319 TEST_CASE(zeroDiv15); // #8319
TEST_CASE(zeroDiv16); // #11158
TEST_CASE(zeroDivCond); // division by zero / useless condition 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()); 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() { void zeroDivCond() {
check("void f(unsigned int x) {\n" check("void f(unsigned int x) {\n"
" int y = 17 / x;\n" " int y = 17 / x;\n"