From 63811b2993543dd6940ab349857c03c3c2395c75 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 20 Aug 2023 11:08:17 +0200 Subject: [PATCH 1/3] Fix #11872 FN unusedVariable with multidimensional array (#5334) --- lib/checkunusedvar.cpp | 16 +++++++++------- test/cfg/std.c | 2 -- test/testunusedvar.cpp | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 68366756fe8..ebf57b053e6 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -704,7 +704,7 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const else if (i->isArray() && i->nameToken()->previous()->str() == "&") type = Variables::referenceArray; else if (i->isArray()) - type = (i->dimensions().size() == 1U) ? Variables::array : Variables::pointerArray; + type = Variables::array; else if (i->isReference() && !(i->valueType() && i->valueType()->type == ValueType::UNKNOWN_TYPE && Token::simpleMatch(i->typeStartToken(), "auto"))) type = Variables::reference; else if (i->nameToken()->previous()->str() == "*" && i->nameToken()->strAt(-2) == "*") @@ -1069,13 +1069,15 @@ void CheckUnusedVar::checkFunctionVariableUsage_iterateScopes(const Scope* const } else if (Token::Match(tok, "[(,] & %var% [,)]")) { variables.eraseAll(tok->tokAt(2)->varId()); } else if (Token::Match(tok, "[(,] (") && - Token::Match(tok->next()->link(), ") %var% [,)]")) { + Token::Match(tok->next()->link(), ") %var% [,)[]")) { variables.use(tok->next()->link()->next()->varId(), tok); // use = read + write - } else if (Token::Match(tok, "[(,] *| %var% =")) { - tok = tok->next(); - if (tok->str() == "*") - tok = tok->next(); - variables.use(tok->varId(), tok); + } else if (Token::Match(tok, "[(,] *| *| %var%")) { + const Token* vartok = tok->next(); + while (vartok->str() == "*") + vartok = vartok->next(); + if (!(vartok->variable() && vartok == vartok->variable()->nameToken()) && + !(tok->str() == "(" && !Token::Match(tok->previous(), "%name%"))) + variables.use(vartok->varId(), vartok); } // function diff --git a/test/cfg/std.c b/test/cfg/std.c index 5f5a2c5096c..4393a17398c 100644 --- a/test/cfg/std.c +++ b/test/cfg/std.c @@ -4181,7 +4181,6 @@ void uninitvar_tolower(int character) // cppcheck-suppress uninitvar (void)tolower(c1); - // cppcheck-suppress unassignedVariable int c2; // cppcheck-suppress constVariablePointer int *pc=&c2; @@ -4203,7 +4202,6 @@ void uninitvar_toupper(int character) // cppcheck-suppress uninitvar (void)toupper(c1); - // cppcheck-suppress unassignedVariable int c2; // cppcheck-suppress constVariablePointer int *pc=&c2; diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 94904a6d684..839ea34e6fa 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -6041,6 +6041,23 @@ class TestUnusedVar : public TestFixture { " dostuff(*p);\n" "}"); ASSERT_EQUALS("", errout.str()); + + functionVariableUsage("int foo() {\n" + " int p[5];\n" + " dostuff(*p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage("int foo() {\n" + " int p[5][5][5];\n" + " dostuff(**p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + functionVariableUsage("void f() {\n" // #11872 + " char v[1][2];\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Unused variable: v\n", errout.str()); } void localvarstring1() { // ticket #1597 From a5cfa85e0d29e4e668ea3bd4ff3ddd7cdfdc70ec Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 20 Aug 2023 15:01:04 -0500 Subject: [PATCH 2/3] Fix 11884: Hang in valueFlowGetStrLength (#5352) --- lib/valueflow.cpp | 10 ++++++---- test/testvalueflow.cpp | 12 ++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index ffd9c209517..c650e12b65b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -8707,10 +8707,12 @@ static MathLib::bigint valueFlowGetStrLength(const Token* tok) return Token::getStrLength(tok); if (astIsGenericChar(tok) || tok->tokType() == Token::eChar) return 1; - if (const ValueFlow::Value* v2 = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE)) - return v2->intvalue; - if (const ValueFlow::Value* v1 = tok->getKnownValue(ValueFlow::Value::ValueType::TOK)) - return valueFlowGetStrLength(v1->tokvalue); + if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE)) + return v->intvalue; + if (const ValueFlow::Value* v = tok->getKnownValue(ValueFlow::Value::ValueType::TOK)) { + if (v->tokvalue != tok) + return valueFlowGetStrLength(v->tokvalue); + } return 0; } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f24942164fa..8a075721a84 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -7397,6 +7397,18 @@ class TestValueFlow : public TestFixture { " }\n" "}"; valueOfTok(code, "path"); + + code = "struct S {\n" + " std::string to_string() const {\n" + " return { this->p , (size_t)this->n };\n" + " }\n" + " const char* p;\n" + " int n;\n" + "};\n" + "void f(S s, std::string& str) {\n" + " str += s.to_string();\n" + "}\n"; + valueOfTok(code, "s"); } void valueFlowUnknownMixedOperators() { From 03b952d5ebab180be9976f6007b8b367f5d50109 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sun, 20 Aug 2023 15:32:41 -0500 Subject: [PATCH 3/3] Fix 11579: false negative: knownConditionTrueFalse with non-bool as bool parameter (#5349) This adds a new checker to check for pointer to bool conversions that are always known. I removed the previous knownConditionTrueFalse checks since this was too noisy. --- lib/astutils.cpp | 19 ++++++-- lib/astutils.h | 2 +- lib/checkcondition.cpp | 15 +++--- lib/checkcondition.h | 2 +- lib/checkother.cpp | 100 +++++++++++++++++++++++++++------------- lib/checkother.h | 5 ++ lib/forwardanalyzer.cpp | 2 +- lib/token.cpp | 9 ---- lib/token.h | 1 - lib/valueflow.cpp | 3 +- releasenotes.txt | 1 + test/testastutils.cpp | 2 +- test/testcondition.cpp | 16 +++---- test/testother.cpp | 69 +++++++++++++++++++++++++++ 14 files changed, 181 insertions(+), 65 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 78252680e11..868c48f9ab7 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -696,9 +696,10 @@ std::vector getParentValueTypes(const Token* tok, const Settings* set return {*tok->astParent()->astOperand1()->valueType()}; return {}; } + const Token* ftok = nullptr; if (Token::Match(tok->astParent(), "(|{|,")) { int argn = -1; - const Token* ftok = getTokenArgumentFunction(tok, argn); + ftok = getTokenArgumentFunction(tok, argn); const Token* typeTok = nullptr; if (ftok && argn >= 0) { if (ftok->function()) { @@ -741,6 +742,9 @@ std::vector getParentValueTypes(const Token* tok, const Settings* set ValueType vtParent = ValueType::parseDecl(vtCont->containerTypeToken, *settings); return {std::move(vtParent)}; } + // The return type of a function is not the parent valuetype + if (Token::simpleMatch(tok->astParent(), "(") && ftok && !tok->isCast() && ftok->tokType() != Token::eType) + return {}; if (Token::Match(tok->astParent(), "return|(|{|%assign%") && parent) { *parent = tok->astParent(); } @@ -1422,7 +1426,7 @@ static bool isForLoopIncrement(const Token* const tok) parent->astParent()->astParent()->astOperand1()->str() == "for"; } -bool isUsedAsBool(const Token* const tok) +bool isUsedAsBool(const Token* const tok, const Settings* settings) { if (!tok) return false; @@ -1435,6 +1439,15 @@ bool isUsedAsBool(const Token* const tok) const Token* parent = tok->astParent(); if (!parent) return false; + if (Token::simpleMatch(parent, "[")) + return false; + if (parent->isUnaryOp("*")) + return false; + if (Token::simpleMatch(parent, ".")) { + if (astIsRHS(tok)) + return isUsedAsBool(parent, settings); + return false; + } if (Token::Match(parent, "&&|!|%oror%")) return true; if (parent->isCast()) @@ -1451,7 +1464,7 @@ bool isUsedAsBool(const Token* const tok) if (isForLoopCondition(tok)) return true; if (!Token::Match(parent, "%cop%")) { - std::vector vtParents = getParentValueTypes(tok); + std::vector vtParents = getParentValueTypes(tok, settings); return std::any_of(vtParents.cbegin(), vtParents.cend(), [&](const ValueType& vt) { return vt.pointer == 0 && vt.type == ValueType::BOOL; }); diff --git a/lib/astutils.h b/lib/astutils.h index 0ef08ece023..af5f6aff66d 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -254,7 +254,7 @@ const Token* isInLoopCondition(const Token* tok); /** * Is token used as boolean, that is to say cast to a bool, or used as a condition in a if/while/for */ -CPPCHECKLIB bool isUsedAsBool(const Token * const tok); +CPPCHECKLIB bool isUsedAsBool(const Token* const tok, const Settings* settings = nullptr); /** * Is token passed to a function taking a bool argument diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 8219d04b1ec..715ec55afea 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -60,7 +60,7 @@ bool CheckCondition::diag(const Token* tok, bool insert) return false; const Token* parent = tok->astParent(); bool hasParent = false; - while (Token::Match(parent, "&&|%oror%")) { + while (Token::Match(parent, "!|&&|%oror%")) { if (mCondDiags.count(parent) != 0) { hasParent = true; break; @@ -410,6 +410,8 @@ void CheckCondition::comparison() void CheckCondition::comparisonError(const Token *tok, const std::string &bitop, MathLib::bigint value1, const std::string &op, MathLib::bigint value2, bool result) { + if (tok && (diag(tok) | diag(tok->astParent()))) + return; std::ostringstream expression; expression << std::hex << "(X " << bitop << " 0x" << value1 << ") " << op << " 0x" << value2; @@ -1370,6 +1372,8 @@ void CheckCondition::checkModuloAlwaysTrueFalse() void CheckCondition::moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal) { + if (diag(tok)) + return; reportError(tok, Severity::warning, "moduloAlwaysTrueFalse", "Comparison of modulo result is predetermined, because it is always less than " + maxVal + ".", CWE398, Certainty::normal); } @@ -1466,7 +1470,7 @@ void CheckCondition::alwaysTrueFalse() for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used continue; - if (!tok->hasKnownBoolValue()) + if (!tok->hasKnownIntValue()) continue; if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) { const Function* f = tok->previous()->function(); @@ -1490,7 +1494,7 @@ void CheckCondition::alwaysTrueFalse() else if (parent->str() == ";" && parent->astParent() && parent->astParent()->astParent() && Token::simpleMatch(parent->astParent()->astParent()->previous(), "for (")) condition = parent->astParent()->astParent()->previous(); - else if (isBooleanFuncArg(tok)) + else if (Token::Match(tok, "%comp%")) condition = tok; else continue; @@ -1582,10 +1586,7 @@ void CheckCondition::alwaysTrueFalse() if (hasSizeof) continue; - auto it = std::find_if(tok->values().begin(), tok->values().end(), [](const ValueFlow::Value& v) { - return v.isIntValue(); - }); - alwaysTrueFalseError(tok, condition, &*it); + alwaysTrueFalseError(tok, condition, &tok->values().front()); } } } diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 948dd7be217..4c75a8a9352 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -67,12 +67,12 @@ class CPPCHECKLIB CheckCondition : public Check { checkCondition.checkPointerAdditionResultNotNull(); checkCondition.checkDuplicateConditionalAssign(); checkCondition.assignIf(); - checkCondition.alwaysTrueFalse(); checkCondition.checkBadBitmaskCheck(); checkCondition.comparison(); checkCondition.checkModuloAlwaysTrueFalse(); checkCondition.checkAssignmentInCondition(); checkCondition.checkCompareValueOutOfTypeRange(); + checkCondition.alwaysTrueFalse(); } /** mismatching assignment / comparison */ diff --git a/lib/checkother.cpp b/lib/checkother.cpp index deaadbf7ca0..05801b4ad15 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3640,6 +3640,21 @@ static bool isVariableExpression(const Token* tok) return false; } +static bool isVariableExprHidden(const Token* tok) +{ + if (!tok) + return false; + if (!tok->astParent()) + return false; + if (Token::simpleMatch(tok->astParent(), "*") && Token::simpleMatch(tok->astSibling(), "0")) + return true; + if (Token::simpleMatch(tok->astParent(), "&&") && Token::simpleMatch(tok->astSibling(), "false")) + return true; + if (Token::simpleMatch(tok->astParent(), "||") && Token::simpleMatch(tok->astSibling(), "true")) + return true; + return false; +} + void CheckOther::checkKnownArgument() { if (!mSettings->severity.isEnabled(Severity::style)) @@ -3679,44 +3694,25 @@ void CheckOther::checkKnownArgument() mTokenizer->isCPP(), true, tok->astOperand1(), tok->astOperand2(), mSettings->library, true, true)) continue; // ensure that there is a integer variable in expression with unknown value - std::string varexpr; - bool isVariableExprHidden = false; // Is variable expression explicitly hidden - auto setVarExpr = [&varexpr, &isVariableExprHidden](const Token *child) { + const Token* vartok = findAstNode(tok, [](const Token* child) { if (Token::Match(child, "%var%|.|[")) { - if (child->valueType() && child->valueType()->pointer == 0 && child->valueType()->isIntegral() && child->values().empty()) { - varexpr = child->expressionString(); - return ChildrenToVisit::done; - } - return ChildrenToVisit::none; + return astIsIntegral(child, false) && !astIsPointer(child) && child->values().empty(); } - if (Token::simpleMatch(child->previous(), "sizeof (")) - return ChildrenToVisit::none; - - // 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"))) - 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, setVarExpr); - if (varexpr.empty()) { - isVariableExprHidden = true; - visitAstNodes(tok, setVarExpr); - } - if (varexpr.empty()) + return false; + }); + if (!vartok) + continue; + if (vartok->astSibling() && + findAstNode(vartok->astSibling(), [](const Token* child) { + return Token::simpleMatch(child, "sizeof"); + })) continue; // ensure that function name does not contain "assert" - std::string funcname = tok->astParent()->previous()->str(); + std::string funcname = ftok->str(); strTolower(funcname); if (funcname.find("assert") != std::string::npos) continue; - knownArgumentError(tok, ftok, &tok->values().front(), varexpr, isVariableExprHidden); + knownArgumentError(tok, ftok, &tok->values().front(), vartok->expressionString(), isVariableExprHidden(vartok)); } } } @@ -3747,6 +3743,48 @@ void CheckOther::knownArgumentError(const Token *tok, const Token *ftok, const V reportError(errorPath, Severity::style, id, errmsg, CWE570, Certainty::normal); } +void CheckOther::checkKnownPointerToBool() +{ + if (!mSettings->severity.isEnabled(Severity::style)) + return; + const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope* functionScope : symbolDatabase->functionScopes) { + for (const Token* tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) { + if (!tok->hasKnownIntValue()) + continue; + if (!astIsPointer(tok)) + continue; + if (Token::Match(tok->astParent(), "?|!|&&|%oror%|%comp%")) + continue; + if (tok->astParent() && Token::Match(tok->astParent()->previous(), "if|while|switch|sizeof (")) + continue; + if (tok->isExpandedMacro()) + continue; + if (findParent(tok, [](const Token* parent) { + return parent->isExpandedMacro(); + })) + continue; + if (!isUsedAsBool(tok, mSettings)) + continue; + const ValueFlow::Value& value = tok->values().front(); + knownPointerToBoolError(tok, &value); + } + } +} + +void CheckOther::knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value) +{ + if (!tok) { + reportError(tok, Severity::style, "knownPointerToBool", "Pointer expression 'p' converted to bool is always true."); + return; + } + std::string cond = value->intvalue ? "true" : "false"; + const std::string& expr = tok->expressionString(); + std::string errmsg = "Pointer expression '" + expr + "' converted to bool is always " + cond + "."; + const ErrorPath errorPath = getErrorPath(tok, value, errmsg); + reportError(errorPath, Severity::style, "knownPointerToBool", errmsg, CWE570, Certainty::normal); +} + void CheckOther::checkComparePointers() { const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); diff --git a/lib/checkother.h b/lib/checkother.h index b38c82e2cb8..69a62b9b111 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -85,6 +85,7 @@ class CPPCHECKLIB CheckOther : public Check { checkOther.checkFuncArgNamesDifferent(); checkOther.checkShadowVariables(); checkOther.checkKnownArgument(); + checkOther.checkKnownPointerToBool(); checkOther.checkComparePointers(); checkOther.checkIncompleteStatement(); checkOther.checkRedundantCopy(); @@ -218,6 +219,8 @@ class CPPCHECKLIB CheckOther : public Check { void checkKnownArgument(); + void checkKnownPointerToBool(); + void checkComparePointers(); void checkModuloOfOne(); @@ -279,6 +282,7 @@ class CPPCHECKLIB CheckOther : public Check { 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 isVariableExpressionHidden); + void knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value); void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2); void checkModuloOfOneError(const Token *tok); @@ -348,6 +352,7 @@ class CPPCHECKLIB CheckOther : public Check { c.shadowError(nullptr, nullptr, "function"); c.shadowError(nullptr, nullptr, "argument"); c.knownArgumentError(nullptr, nullptr, nullptr, "x", false); + c.knownPointerToBoolError(nullptr, nullptr); c.comparePointersError(nullptr, nullptr, nullptr); c.redundantAssignmentError(nullptr, nullptr, "var", false); c.redundantInitializationError(nullptr, nullptr, "var", false); diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 09abde664c1..fea0526eb5d 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -488,7 +488,7 @@ struct ForwardTraversal { if (allAnalysis.isIncremental()) return Break(Analyzer::Terminate::Bail); } else if (allAnalysis.isModified()) { - std::vector ftv = tryForkScope(endBlock, /*isModified*/ true); + std::vector ftv = tryForkScope(endBlock, allAnalysis.isModified()); bool forkContinue = true; for (ForwardTraversal& ft : ftv) { if (condTok) diff --git a/lib/token.cpp b/lib/token.cpp index 6c832f3a741..a13f98763d4 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2388,15 +2388,6 @@ bool Token::hasKnownIntValue() const }); } -bool Token::hasKnownBoolValue() const -{ - if (!mImpl->mValues) - return false; - return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) { - return value.isIntValue() && (value.isKnown() || (value.intvalue == 0 && value.isImpossible())); - }); -} - bool Token::hasKnownValue() const { return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown)); diff --git a/lib/token.h b/lib/token.h index b8232638152..dfd601a7677 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1216,7 +1216,6 @@ class CPPCHECKLIB Token { } bool hasKnownIntValue() const; - bool hasKnownBoolValue() const; bool hasKnownValue() const; bool hasKnownValue(ValueFlow::Value::ValueType t) const; bool hasKnownSymbolicValue(const Token* tok) const; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c650e12b65b..3572dddd3b9 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -6843,7 +6843,8 @@ static void valueFlowInferCondition(TokenList& tokenlist, } } } else if (Token::Match(tok->astParent(), "?|&&|!|%oror%") || - Token::Match(tok->astParent()->previous(), "if|while (")) { + Token::Match(tok->astParent()->previous(), "if|while (") || + (astIsPointer(tok) && isUsedAsBool(tok, settings))) { std::vector result = infer(IntegralInferModel{}, "!=", tok->values(), 0); if (result.size() != 1) continue; diff --git a/releasenotes.txt b/releasenotes.txt index 57e6d0210fa..c54c20094a9 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -2,6 +2,7 @@ Release Notes for Cppcheck 2.12 New checks: - uselessOverride finds overriding functions that either duplicate code from or delegate back to the base class implementation +- knownPointerToBool finds pointer to bool conversions that are always true or false Improved checking: - truncLongCastAssignment and truncLongCastReturn check additional types, including float/double/long double diff --git a/test/testastutils.cpp b/test/testastutils.cpp index d14d3564079..fe31e478335 100644 --- a/test/testastutils.cpp +++ b/test/testastutils.cpp @@ -484,7 +484,7 @@ class TestAstUtils : public TestFixture { ASSERT(Result::False == isUsedAsBool("void f() { int i; if (i + 2) {} }", "i +")); ASSERT(Result::True == isUsedAsBool("void f() { int i; bool b = i; }", "i ; }")); ASSERT(Result::True == isUsedAsBool("void f(bool b); void f() { int i; f(i); }","i )")); - ASSERT(Result::True == isUsedAsBool("void f() { int *i; if (*i) {} }", "i )")); + ASSERT(Result::False == isUsedAsBool("void f() { int *i; if (*i) {} }", "i )")); ASSERT(Result::True == isUsedAsBool("void f() { int *i; if (*i) {} }", "* i )")); ASSERT(Result::True == isUsedAsBool("int g(); void h(bool); void f() { h(g()); }", "( ) )")); ASSERT(Result::True == isUsedAsBool("int g(int); void h(bool); void f() { h(g(0)); }", "( 0 ) )")); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index b60be8b831b..34abf929b79 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1879,7 +1879,6 @@ class TestCondition : public TestFixture { " return a % 5 > 5;\n" "}"); ASSERT_EQUALS( - "[test.cpp:7]: (style) Return value 'a%5>5' is always false\n" "[test.cpp:2]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" @@ -1894,7 +1893,6 @@ class TestCondition : public TestFixture { " b2 = x.a % 5 == 5;\n" "}"); ASSERT_EQUALS( - "[test.cpp:3]: (style) Condition 'x[593]%5<=5' is always true\n" "[test.cpp:2]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n", @@ -3223,7 +3221,9 @@ class TestCondition : public TestFixture { " A(x++ == 1);\n" " A(x++ == 2);\n" "}"); - TODO_ASSERT_EQUALS("function argument is always true? however is code really weird/suspicious?", "", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'x++==1' is always false\n" + "[test.cpp:4]: (style) Condition 'x++==2' is always false\n", + errout.str()); check("bool foo(int bar) {\n" " bool ret = false;\n" @@ -4229,7 +4229,9 @@ class TestCondition : public TestFixture { " if (w) {}\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'w' is always true\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'v<2' is always true\n" + "[test.cpp:5]: (style) Condition 'w' is always true\n", + errout.str()); check("void f(double d) {\n" // #10792 " if (d != 0) {\n" @@ -4521,13 +4523,9 @@ class TestCondition : public TestFixture { " int* p = &i;\n" " g(i == 7);\n" " g(p == nullptr);\n" - " g(p);\n" - " g(&i);\n" "}\n"); ASSERT_EQUALS("[test.cpp:5]: (style) Condition 'i==7' is always false\n" - "[test.cpp:6]: (style) Condition 'p==nullptr' is always false\n" - "[test.cpp:7]: (style) Condition 'p' is always true\n" - "[test.cpp:8]: (style) Condition '&i' is always true\n", + "[test.cpp:6]: (style) Condition 'p==nullptr' is always false\n", errout.str()); } diff --git a/test/testother.cpp b/test/testother.cpp index d789c3bb18c..b8c48f797a9 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -290,6 +290,8 @@ class TestOther : public TestFixture { TEST_CASE(checkOverlappingWrite); TEST_CASE(constVariableArrayMember); // #10371 + + TEST_CASE(knownPointerToBool); } #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__) @@ -11340,6 +11342,73 @@ class TestOther : public TestFixture { "};\n"); ASSERT_EQUALS("", errout.str()); } + + void knownPointerToBool() + { + check("void g(bool);\n" + "void f() {\n" + " int i = 5;\n" + " int* p = &i;\n" + " g(p);\n" + " g(&i);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Pointer expression 'p' converted to bool is always true.\n" + "[test.cpp:6]: (style) Pointer expression '&i' converted to bool is always true.\n", + errout.str()); + + check("void f() {\n" + " const int* x = nullptr;\n" + " std::empty(x);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { bool x; };\n" + "bool f(A* a) {\n" + " if (a) {\n" + " return a->x;\n" + " }\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct A { int* x; };\n" + "bool f(A a) {\n" + " if (a.x) {\n" + " return a.x;\n" + " }\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Pointer expression 'a.x' converted to bool is always true.\n", errout.str()); + + check("void f(bool* b) { if (b) *b = true; }"); + ASSERT_EQUALS("", errout.str()); + + check("bool f() {\n" + " int* x = nullptr;\n" + " return bool(x);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Pointer expression 'x' converted to bool is always false.\n", errout.str()); + + check("bool f() {\n" + " int* x = nullptr;\n" + " return bool{x};\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Pointer expression 'x' converted to bool is always false.\n", errout.str()); + + check("struct A { A(bool); };\n" + "A f() {\n" + " int* x = nullptr;\n" + " return A(x);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Pointer expression 'x' converted to bool is always false.\n", errout.str()); + + check("struct A { A(bool); };\n" + "A f() {\n" + " int* x = nullptr;\n" + " return A{x};\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Pointer expression 'x' converted to bool is always false.\n", errout.str()); + } }; REGISTER_TEST(TestOther)