From 05b0a0f970e3b95f0ab413ea5e0062956065ce67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 26 Sep 2020 10:50:58 +0200 Subject: [PATCH] Make duplicateAssignExpression warnings inconclusive for 'x&&false' etc. (#9914) --- lib/checkother.cpp | 30 +++++++++++++++++++----------- lib/checkother.h | 4 ++-- test/testother.cpp | 5 ++++- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 7d1046ccf..623b30e88 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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() diff --git a/lib/checkother.h b/lib/checkother.h index 0e950d5fe..f658a6c15 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); + 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); diff --git a/test/testother.cpp b/test/testother.cpp index f85cca9d6..a1ce7e91e 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -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() {