diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 623b30e88..35cd1fd1d 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3155,8 +3155,8 @@ void CheckOther::checkKnownArgument() continue; // ensure that there is a integer variable in expression with unknown value std::string varexpr; - bool inconclusive = false; - auto visitAstNode = [&varexpr, &inconclusive](const Token *child) { + bool isVariableExprHidden = false; // Is variable expression explicitly hidden + auto setVarExpr = [&varexpr, &isVariableExprHidden](const Token *child) { if (Token::Match(child, "%var%|.|[")) { if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) { varexpr = child->expressionString(); @@ -3166,7 +3166,9 @@ void CheckOther::checkKnownArgument() } if (Token::simpleMatch(child->previous(), "sizeof (")) 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"))) return ChildrenToVisit::none; 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"))) return ChildrenToVisit::none; } + return ChildrenToVisit::op1_and_op2; }; - visitAstNodes(tok, visitAstNode); - if (varexpr.empty() && mSettings->inconclusive) { - inconclusive = true; - visitAstNodes(tok, visitAstNode); + visitAstNodes(tok, setVarExpr); + if (varexpr.empty()) { + isVariableExprHidden = true; + visitAstNodes(tok, setVarExpr); } if (varexpr.empty()) continue; @@ -3190,20 +3193,35 @@ void CheckOther::checkKnownArgument() }); if (funcname.find("assert") != std::string::npos) 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; - const std::string expr = tok ? tok->expressionString() : varexpr; - const std::string fun = ftok ? ftok->str() : std::string("f"); + if (!tok) { + reportError(tok, Severity::style, "knownArgument", "Argument 'x-x' to function 'func' is always 0. It does not matter what value 'x' has."); + 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); - reportError(errorPath, Severity::style, "knownArgument", errmsg, CWE570, inconclusive); + reportError(errorPath, Severity::style, id, errmsg, CWE570, false); } void CheckOther::checkComparePointers() diff --git a/lib/checkother.h b/lib/checkother.h index f658a6c15..fbf55c779 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -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 & declarations, const std::vector & 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, 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 checkModuloOfOneError(const Token *tok); diff --git a/test/testother.cpp b/test/testother.cpp index a1ce7e91e..5042aa06a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -240,7 +240,8 @@ private: TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {" TEST_CASE(shadowVariables); - TEST_CASE(constArgument); + TEST_CASE(knownArgument); + TEST_CASE(knownArgumentHiddenVariableExpression); TEST_CASE(checkComparePointers); TEST_CASE(unusedVariableValueTemplate); // #8994 @@ -8811,7 +8812,7 @@ private: ASSERT_EQUALS("", errout.str()); } - void constArgument() { + void knownArgument() { check("void g(int);\n" "void f(int x) {\n" " g((x & 0x01) >> 7);\n" @@ -8936,8 +8937,10 @@ private: " dostuff(self->maxsize * sizeof(intptr_t));\n" "}"); ASSERT_EQUALS("", errout.str()); + } - // #9914 - zeroed expression + void knownArgumentHiddenVariableExpression() { + // #9914 - variable expression is explicitly hidden check("void f(int x) {\n" " dostuff(x && false);\n" " dostuff(false && x);\n" @@ -8946,10 +8949,10 @@ private: " dostuff(x * 0);\n" " dostuff(0 * x);\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" - "[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()); + 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) Argument 'true||x' to function dostuff is always 1. Constant literal calculation disable/hide variable expression 'x'.\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) Argument '0*x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\n", errout.str()); } void checkComparePointers() {