Make duplicateAssignExpression warnings inconclusive for 'x&&false' etc. (#9914)

This commit is contained in:
Daniel Marjamäki 2020-09-26 10:50:58 +02:00
parent 887b40e08b
commit 05b0a0f970
3 changed files with 25 additions and 14 deletions

View File

@ -3155,7 +3155,8 @@ void CheckOther::checkKnownArgument()
continue;
// ensure that there is a integer variable in expression with unknown value
std::string varexpr;
visitAstNodes(tok, [&varexpr](const Token *child) {
bool inconclusive = false;
auto visitAstNode = [&varexpr, &inconclusive](const Token *child) {
if (Token::Match(child, "%var%|.|[")) {
if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) {
varexpr = child->expressionString();
@ -3165,14 +3166,21 @@ void CheckOther::checkKnownArgument()
}
if (Token::simpleMatch(child->previous(), "sizeof ("))
return ChildrenToVisit::none;
if (Token::simpleMatch(child, "*") && (Token::simpleMatch(child->astOperand1(), "0") || Token::simpleMatch(child->astOperand2(), "0")))
return ChildrenToVisit::none;
if (Token::simpleMatch(child, "&&") && (Token::simpleMatch(child->astOperand1(), "false") || Token::simpleMatch(child->astOperand2(), "false")))
return ChildrenToVisit::none;
if (Token::simpleMatch(child, "||") && (Token::simpleMatch(child->astOperand1(), "true") || Token::simpleMatch(child->astOperand2(), "true")))
return ChildrenToVisit::none;
if (!inconclusive) {
if (Token::simpleMatch(child, "*") && (Token::simpleMatch(child->astOperand1(), "0") || Token::simpleMatch(child->astOperand2(), "0")))
return ChildrenToVisit::none;
if (Token::simpleMatch(child, "&&") && (Token::simpleMatch(child->astOperand1(), "false") || Token::simpleMatch(child->astOperand2(), "false")))
return ChildrenToVisit::none;
if (Token::simpleMatch(child, "||") && (Token::simpleMatch(child->astOperand1(), "true") || Token::simpleMatch(child->astOperand2(), "true")))
return ChildrenToVisit::none;
}
return ChildrenToVisit::op1_and_op2;
});
};
visitAstNodes(tok, visitAstNode);
if (varexpr.empty() && mSettings->inconclusive) {
inconclusive = true;
visitAstNodes(tok, visitAstNode);
}
if (varexpr.empty())
continue;
// ensure that function name does not contain "assert"
@ -3182,12 +3190,12 @@ void CheckOther::checkKnownArgument()
});
if (funcname.find("assert") != std::string::npos)
continue;
knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr);
knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr, inconclusive);
}
}
}
void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr)
void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool inconclusive)
{
MathLib::bigint intvalue = value ? value->intvalue : 0;
const std::string expr = tok ? tok->expressionString() : varexpr;
@ -3195,7 +3203,7 @@ void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const V
const std::string errmsg = "Argument '" + expr + "' to function " + fun + " is always " + std::to_string(intvalue) + ". It does not matter what value '" + varexpr + "' has.";
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
reportError(errorPath, Severity::style, "knownArgument", errmsg, CWE570, false);
reportError(errorPath, Severity::style, "knownArgument", errmsg, CWE570, inconclusive);
}
void CheckOther::checkComparePointers()

View File

@ -274,7 +274,7 @@ private:
void funcArgNamesDifferent(const std::string & functionName, nonneg int index, const Token* declaration, const Token* definition);
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
void shadowError(const Token *var, const Token *shadowed, std::string type);
void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr);
void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool inconclusive);
void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
void checkModuloOfOneError(const Token *tok);
@ -343,7 +343,7 @@ private:
c.shadowError(nullptr, nullptr, "variable");
c.shadowError(nullptr, nullptr, "function");
c.shadowError(nullptr, nullptr, "argument");
c.knownArgumentError(nullptr, nullptr, nullptr, "x");
c.knownArgumentError(nullptr, nullptr, nullptr, "x", false);
c.comparePointersError(nullptr, nullptr, nullptr);
c.redundantAssignmentError(nullptr, nullptr, "var", false);
c.redundantInitializationError(nullptr, nullptr, "var", false);

View File

@ -8946,7 +8946,10 @@ private:
" dostuff(x * 0);\n"
" dostuff(0 * x);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (style, inconclusive) Argument 'false&&x' to function dostuff is always 0. It does not matter what value 'x' has.\n"
"[test.cpp:5]: (style, inconclusive) Argument 'true||x' to function dostuff is always 1. It does not matter what value 'x' has.\n"
"[test.cpp:6]: (style, inconclusive) Argument 'x*0' to function dostuff is always 0. It does not matter what value 'x' has.\n"
"[test.cpp:7]: (style, inconclusive) Argument '0*x' to function dostuff is always 0. It does not matter what value 'x' has.\n", errout.str());
}
void checkComparePointers() {