More fixing for #9914. New warning id and warning message when variable expression is explicitly hidden.

This commit is contained in:
Daniel Marjamäki 2020-09-26 13:49:47 +02:00
parent d615dba638
commit 5578b09452
3 changed files with 43 additions and 22 deletions

View File

@ -3155,8 +3155,8 @@ void CheckOther::checkKnownArgument()
continue; continue;
// ensure that there is a integer variable in expression with unknown value // ensure that there is a integer variable in expression with unknown value
std::string varexpr; std::string varexpr;
bool inconclusive = false; bool isVariableExprHidden = false; // Is variable expression explicitly hidden
auto visitAstNode = [&varexpr, &inconclusive](const Token *child) { auto setVarExpr = [&varexpr, &isVariableExprHidden](const Token *child) {
if (Token::Match(child, "%var%|.|[")) { if (Token::Match(child, "%var%|.|[")) {
if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) { if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) {
varexpr = child->expressionString(); varexpr = child->expressionString();
@ -3166,7 +3166,9 @@ void CheckOther::checkKnownArgument()
} }
if (Token::simpleMatch(child->previous(), "sizeof (")) if (Token::simpleMatch(child->previous(), "sizeof ("))
return ChildrenToVisit::none; return ChildrenToVisit::none;
if (!inconclusive) {
// hide variable explicitly with 'x * 0' etc
if (!isVariableExprHidden) {
if (Token::simpleMatch(child, "*") && (Token::simpleMatch(child->astOperand1(), "0") || Token::simpleMatch(child->astOperand2(), "0"))) if (Token::simpleMatch(child, "*") && (Token::simpleMatch(child->astOperand1(), "0") || Token::simpleMatch(child->astOperand2(), "0")))
return ChildrenToVisit::none; return ChildrenToVisit::none;
if (Token::simpleMatch(child, "&&") && (Token::simpleMatch(child->astOperand1(), "false") || Token::simpleMatch(child->astOperand2(), "false"))) if (Token::simpleMatch(child, "&&") && (Token::simpleMatch(child->astOperand1(), "false") || Token::simpleMatch(child->astOperand2(), "false")))
@ -3174,12 +3176,13 @@ void CheckOther::checkKnownArgument()
if (Token::simpleMatch(child, "||") && (Token::simpleMatch(child->astOperand1(), "true") || Token::simpleMatch(child->astOperand2(), "true"))) if (Token::simpleMatch(child, "||") && (Token::simpleMatch(child->astOperand1(), "true") || Token::simpleMatch(child->astOperand2(), "true")))
return ChildrenToVisit::none; return ChildrenToVisit::none;
} }
return ChildrenToVisit::op1_and_op2; return ChildrenToVisit::op1_and_op2;
}; };
visitAstNodes(tok, visitAstNode); visitAstNodes(tok, setVarExpr);
if (varexpr.empty() && mSettings->inconclusive) { if (varexpr.empty()) {
inconclusive = true; isVariableExprHidden = true;
visitAstNodes(tok, visitAstNode); visitAstNodes(tok, setVarExpr);
} }
if (varexpr.empty()) if (varexpr.empty())
continue; continue;
@ -3190,20 +3193,35 @@ void CheckOther::checkKnownArgument()
}); });
if (funcname.find("assert") != std::string::npos) if (funcname.find("assert") != std::string::npos)
continue; continue;
knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr, inconclusive); knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr, isVariableExprHidden);
} }
} }
} }
void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool inconclusive) void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden)
{ {
MathLib::bigint intvalue = value ? value->intvalue : 0; if (!tok) {
const std::string expr = tok ? tok->expressionString() : varexpr; reportError(tok, Severity::style, "knownArgument", "Argument 'x-x' to function 'func' is always 0. It does not matter what value 'x' has.");
const std::string fun = ftok ? ftok->str() : std::string("f"); reportError(tok, Severity::style, "knownArgumentHiddenVariableExpression", "Argument 'x*0' to function 'func' is always 0. Constant literal calculation disable/hide variable expression 'x'.");
return;
}
const MathLib::bigint intvalue = value->intvalue;
const std::string expr = tok->expressionString();
const std::string fun = ftok->str();
const char *id;;
std::string errmsg = "Argument '" + expr + "' to function " + fun + " is always " + std::to_string(intvalue) + ". ";
if (!isVariableExpressionHidden) {
id = "knownArgument";
errmsg += "It does not matter what value '" + varexpr + "' has.";
} else {
id = "knownArgumentHiddenVariableExpression";
errmsg += "Constant literal calculation disable/hide variable expression '" + varexpr + "'.";
}
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); const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
reportError(errorPath, Severity::style, "knownArgument", errmsg, CWE570, inconclusive); reportError(errorPath, Severity::style, id, errmsg, CWE570, false);
} }
void CheckOther::checkComparePointers() 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 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 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 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, bool inconclusive); void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden);
void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2); void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
void checkModuloOfOneError(const Token *tok); void checkModuloOfOneError(const Token *tok);

View File

@ -240,7 +240,8 @@ private:
TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {" TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {"
TEST_CASE(shadowVariables); TEST_CASE(shadowVariables);
TEST_CASE(constArgument); TEST_CASE(knownArgument);
TEST_CASE(knownArgumentHiddenVariableExpression);
TEST_CASE(checkComparePointers); TEST_CASE(checkComparePointers);
TEST_CASE(unusedVariableValueTemplate); // #8994 TEST_CASE(unusedVariableValueTemplate); // #8994
@ -8811,7 +8812,7 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void constArgument() { void knownArgument() {
check("void g(int);\n" check("void g(int);\n"
"void f(int x) {\n" "void f(int x) {\n"
" g((x & 0x01) >> 7);\n" " g((x & 0x01) >> 7);\n"
@ -8936,8 +8937,10 @@ private:
" dostuff(self->maxsize * sizeof(intptr_t));\n" " dostuff(self->maxsize * sizeof(intptr_t));\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
}
// #9914 - zeroed expression void knownArgumentHiddenVariableExpression() {
// #9914 - variable expression is explicitly hidden
check("void f(int x) {\n" check("void f(int x) {\n"
" dostuff(x && false);\n" " dostuff(x && false);\n"
" dostuff(false && x);\n" " dostuff(false && x);\n"
@ -8946,10 +8949,10 @@ private:
" dostuff(x * 0);\n" " dostuff(x * 0);\n"
" dostuff(0 * x);\n" " dostuff(0 * x);\n"
"}\n"); "}\n");
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" ASSERT_EQUALS("[test.cpp:3]: (style) Argument 'false&&x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\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:5]: (style) Argument 'true||x' to function dostuff is always 1. Constant literal calculation disable/hide variable expression 'x'.\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:6]: (style) Argument 'x*0' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\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()); "[test.cpp:7]: (style) Argument '0*x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\n", errout.str());
} }
void checkComparePointers() { void checkComparePointers() {