Skip to content

Commit 5578b09

Browse files
danmardanmar
committed
More fixing for #9914. New warning id and warning message when variable expression is explicitly hidden.
1 parent d615dba commit 5578b09

3 files changed

Lines changed: 43 additions & 22 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3155,8 +3155,8 @@ void CheckOther::checkKnownArgument()
31553155
continue;
31563156
// ensure that there is a integer variable in expression with unknown value
31573157
std::string varexpr;
3158-
bool inconclusive = false;
3159-
auto visitAstNode = [&varexpr, &inconclusive](const Token *child) {
3158+
bool isVariableExprHidden = false; // Is variable expression explicitly hidden
3159+
auto setVarExpr = [&varexpr, &isVariableExprHidden](const Token *child) {
31603160
if (Token::Match(child, "%var%|.|[")) {
31613161
if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) {
31623162
varexpr = child->expressionString();
@@ -3166,20 +3166,23 @@ void CheckOther::checkKnownArgument()
31663166
}
31673167
if (Token::simpleMatch(child->previous(), "sizeof ("))
31683168
return ChildrenToVisit::none;
3169-
if (!inconclusive) {
3169+
3170+
// hide variable explicitly with 'x * 0' etc
3171+
if (!isVariableExprHidden) {
31703172
if (Token::simpleMatch(child, "*") && (Token::simpleMatch(child->astOperand1(), "0") || Token::simpleMatch(child->astOperand2(), "0")))
31713173
return ChildrenToVisit::none;
31723174
if (Token::simpleMatch(child, "&&") && (Token::simpleMatch(child->astOperand1(), "false") || Token::simpleMatch(child->astOperand2(), "false")))
31733175
return ChildrenToVisit::none;
31743176
if (Token::simpleMatch(child, "||") && (Token::simpleMatch(child->astOperand1(), "true") || Token::simpleMatch(child->astOperand2(), "true")))
31753177
return ChildrenToVisit::none;
31763178
}
3179+
31773180
return ChildrenToVisit::op1_and_op2;
31783181
};
3179-
visitAstNodes(tok, visitAstNode);
3180-
if (varexpr.empty() && mSettings->inconclusive) {
3181-
inconclusive = true;
3182-
visitAstNodes(tok, visitAstNode);
3182+
visitAstNodes(tok, setVarExpr);
3183+
if (varexpr.empty()) {
3184+
isVariableExprHidden = true;
3185+
visitAstNodes(tok, setVarExpr);
31833186
}
31843187
if (varexpr.empty())
31853188
continue;
@@ -3190,20 +3193,35 @@ void CheckOther::checkKnownArgument()
31903193
});
31913194
if (funcname.find("assert") != std::string::npos)
31923195
continue;
3193-
knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr, inconclusive);
3196+
knownArgumentError(tok, tok->astParent()->previous(), &tok->values().front(), varexpr, isVariableExprHidden);
31943197
}
31953198
}
31963199
}
31973200

3198-
void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool inconclusive)
3201+
void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden)
31993202
{
3200-
MathLib::bigint intvalue = value ? value->intvalue : 0;
3201-
const std::string expr = tok ? tok->expressionString() : varexpr;
3202-
const std::string fun = ftok ? ftok->str() : std::string("f");
3203+
if (!tok) {
3204+
reportError(tok, Severity::style, "knownArgument", "Argument 'x-x' to function 'func' is always 0. It does not matter what value 'x' has.");
3205+
reportError(tok, Severity::style, "knownArgumentHiddenVariableExpression", "Argument 'x*0' to function 'func' is always 0. Constant literal calculation disable/hide variable expression 'x'.");
3206+
return;
3207+
}
3208+
3209+
const MathLib::bigint intvalue = value->intvalue;
3210+
const std::string expr = tok->expressionString();
3211+
const std::string fun = ftok->str();
3212+
3213+
const char *id;;
3214+
std::string errmsg = "Argument '" + expr + "' to function " + fun + " is always " + std::to_string(intvalue) + ". ";
3215+
if (!isVariableExpressionHidden) {
3216+
id = "knownArgument";
3217+
errmsg += "It does not matter what value '" + varexpr + "' has.";
3218+
} else {
3219+
id = "knownArgumentHiddenVariableExpression";
3220+
errmsg += "Constant literal calculation disable/hide variable expression '" + varexpr + "'.";
3221+
}
32033222

3204-
const std::string errmsg = "Argument '" + expr + "' to function " + fun + " is always " + std::to_string(intvalue) + ". It does not matter what value '" + varexpr + "' has.";
32053223
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
3206-
reportError(errorPath, Severity::style, "knownArgument", errmsg, CWE570, inconclusive);
3224+
reportError(errorPath, Severity::style, id, errmsg, CWE570, false);
32073225
}
32083226

32093227
void CheckOther::checkComparePointers()

‎lib/checkother.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ class CPPCHECKLIB CheckOther : public Check {
274274
void funcArgNamesDifferent(const std::string & functionName, nonneg int index, const Token* declaration, const Token* definition);
275275
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
276276
void shadowError(const Token *var, const Token *shadowed, std::string type);
277-
void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool inconclusive);
277+
void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden);
278278
void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
279279
void checkModuloOfOneError(const Token *tok);
280280

‎test/testother.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ class TestOther : public TestFixture {
240240
TEST_CASE(cpp11FunctionArgInit); // #7846 - "void foo(int declaration = {}) {"
241241

242242
TEST_CASE(shadowVariables);
243-
TEST_CASE(constArgument);
243+
TEST_CASE(knownArgument);
244+
TEST_CASE(knownArgumentHiddenVariableExpression);
244245
TEST_CASE(checkComparePointers);
245246

246247
TEST_CASE(unusedVariableValueTemplate); // #8994
@@ -8811,7 +8812,7 @@ class TestOther : public TestFixture {
88118812
ASSERT_EQUALS("", errout.str());
88128813
}
88138814

8814-
void constArgument() {
8815+
void knownArgument() {
88158816
check("void g(int);\n"
88168817
"void f(int x) {\n"
88178818
" g((x & 0x01) >> 7);\n"
@@ -8936,8 +8937,10 @@ class TestOther : public TestFixture {
89368937
" dostuff(self->maxsize * sizeof(intptr_t));\n"
89378938
"}");
89388939
ASSERT_EQUALS("", errout.str());
8940+
}
89398941

8940-
// #9914 - zeroed expression
8942+
void knownArgumentHiddenVariableExpression() {
8943+
// #9914 - variable expression is explicitly hidden
89418944
check("void f(int x) {\n"
89428945
" dostuff(x && false);\n"
89438946
" dostuff(false && x);\n"
@@ -8946,10 +8949,10 @@ class TestOther : public TestFixture {
89468949
" dostuff(x * 0);\n"
89478950
" dostuff(0 * x);\n"
89488951
"}\n");
8949-
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"
8950-
"[test.cpp:5]: (style, inconclusive) Argument 'true||x' to function dostuff is always 1. It does not matter what value 'x' has.\n"
8951-
"[test.cpp:6]: (style, inconclusive) Argument 'x*0' to function dostuff is always 0. It does not matter what value 'x' has.\n"
8952-
"[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());
8952+
ASSERT_EQUALS("[test.cpp:3]: (style) Argument 'false&&x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\n"
8953+
"[test.cpp:5]: (style) Argument 'true||x' to function dostuff is always 1. Constant literal calculation disable/hide variable expression 'x'.\n"
8954+
"[test.cpp:6]: (style) Argument 'x*0' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\n"
8955+
"[test.cpp:7]: (style) Argument '0*x' to function dostuff is always 0. Constant literal calculation disable/hide variable expression 'x'.\n", errout.str());
89538956
}
89548957

89558958
void checkComparePointers() {

0 commit comments

Comments
 (0)