diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 0ff4bcd8931..9ff98974a2d 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -295,6 +295,18 @@ static const Token* getReturnValueFromOutparamAlloc(const Token* alloc, const Se return nullptr; } +static std::vector getComparisonTokens(const Token* tok) +{ + std::vector result{ tok }; + if (tok->hasKnownValue(ValueFlow::Value::ValueType::SYMBOLIC)) + result.push_back(tok->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC)->tokvalue); + for (const Token* op : { tok->astOperand1(), tok->astOperand2() }) { + if (op && op->hasKnownValue(ValueFlow::Value::ValueType::SYMBOLIC)) + result.push_back(op->getKnownValue(ValueFlow::Value::ValueType::SYMBOLIC)->tokvalue); + } + return result; +} + bool CheckLeakAutoVar::checkScope(const Token * const startToken, VarInfo &varInfo, std::set notzero, @@ -444,7 +456,7 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken, if (tok2->str() == ";") { break; } - if (tok2->varId()) { + if (tok2->varId() && !Token::Match(tok2->astParent(), "%comp%|!")) { varInfo.erase(tok2->varId()); } } @@ -578,53 +590,57 @@ bool CheckLeakAutoVar::checkScope(const Token * const startToken, astOperand2AfterCommas = astOperand2AfterCommas->astOperand2(); // Recursively scan variable comparisons in condition - visitAstNodes(astOperand2AfterCommas, [&](const Token *tok3) { - if (!tok3) - return ChildrenToVisit::none; - if (tok3->str() == "&&" || tok3->str() == "||") { - // FIXME: handle && ! || better - return ChildrenToVisit::op1_and_op2; - } - if (tok3->str() == "(" && Token::Match(tok3->astOperand1(), "UNLIKELY|LIKELY")) { - return ChildrenToVisit::op2; - } - if (tok3->str() == "(" && tok3->previous()->isName()) { - const std::vector params = getArguments(tok3->previous()); - for (const Token *par : params) { - if (!par->isComparisonOp()) - continue; - const Token *vartok = nullptr; - if (isVarTokComparison(par, &vartok, alloc_success_conds) || - (isVarTokComparison(par, &vartok, alloc_failed_conds))) { - varInfo1.erase(vartok->varId()); - varInfo2.erase(vartok->varId()); + const std::vector compToks = getComparisonTokens(astOperand2AfterCommas); + for (const Token* compTok : compToks) { + visitAstNodes(compTok, [&](const Token* tok3) { + if (!tok3) + return ChildrenToVisit::none; + if (tok3->str() == "&&" || tok3->str() == "||") { + // FIXME: handle && ! || better + return ChildrenToVisit::op1_and_op2; + } + if (tok3->str() == "(" && Token::Match(tok3->astOperand1(), "UNLIKELY|LIKELY")) { + return ChildrenToVisit::op2; + } + if (tok3->str() == "(" && tok3->previous()->isName()) { + const std::vector params = getArguments(tok3->previous()); + for (const Token* par : params) { + if (!par->isComparisonOp()) + continue; + const Token* vartok = nullptr; + if (isVarTokComparison(par, &vartok, alloc_success_conds) || + (isVarTokComparison(par, &vartok, alloc_failed_conds))) { + varInfo1.erase(vartok->varId()); + varInfo2.erase(vartok->varId()); + } } + return ChildrenToVisit::none; } - return ChildrenToVisit::none; - } - const Token *vartok = nullptr; - if (isVarTokComparison(tok3, &vartok, alloc_success_conds)) { - varInfo2.reallocToAlloc(vartok->varId()); - varInfo2.erase(vartok->varId()); - if (astIsVariableComparison(tok3, "!=", "0", &vartok) && - (notzero.find(vartok->varId()) != notzero.end())) - varInfo2.clear(); - - if (std::any_of(varInfo1.alloctype.begin(), varInfo1.alloctype.end(), [&](const std::pair& info) { - if (info.second.status != VarInfo::ALLOC) - return false; - const Token* ret = getReturnValueFromOutparamAlloc(info.second.allocTok, *mSettings); - return ret && vartok && ret->varId() && ret->varId() == vartok->varId(); - })) { - varInfo1.clear(); + const Token* vartok = nullptr; + if (isVarTokComparison(tok3, &vartok, alloc_success_conds)) { + varInfo2.reallocToAlloc(vartok->varId()); + varInfo2.erase(vartok->varId()); + if (astIsVariableComparison(tok3, "!=", "0", &vartok) && + (notzero.find(vartok->varId()) != notzero.end())) + varInfo2.clear(); + + if (std::any_of(varInfo1.alloctype.begin(), varInfo1.alloctype.end(), [&](const std::pair& info) { + if (info.second.status != VarInfo::ALLOC) + return false; + const Token* ret = getReturnValueFromOutparamAlloc(info.second.allocTok, *mSettings); + return ret && vartok && ret->varId() && ret->varId() == vartok->varId(); + })) { + varInfo1.clear(); + } } - } else if (isVarTokComparison(tok3, &vartok, alloc_failed_conds)) { - varInfo1.reallocToAlloc(vartok->varId()); - varInfo1.erase(vartok->varId()); - } - return ChildrenToVisit::none; - }); + else if (isVarTokComparison(tok3, &vartok, alloc_failed_conds)) { + varInfo1.reallocToAlloc(vartok->varId()); + varInfo1.erase(vartok->varId()); + } + return ChildrenToVisit::none; + }); + } if (!skipIfBlock && !checkScope(closingParenthesis->next(), varInfo1, notzero, recursiveCount)) { varInfo.clear(); diff --git a/test/testleakautovar.cpp b/test/testleakautovar.cpp index 7aa64c1cd3c..d2448c5ebf6 100644 --- a/test/testleakautovar.cpp +++ b/test/testleakautovar.cpp @@ -641,6 +641,14 @@ class TestLeakAutoVar : public TestFixture { " x = p != nullptr ? p : nullptr;\n" "}", dinit(CheckOptions, $.cpp = true)); ASSERT_EQUALS("", errout_str()); + + check("void f(const char* n) {\n" // #12724 + " FILE* fp = fopen(n, \"r\");\n" + "bool b = (fp == NULL);\n" + "if (b)\n" + " return;\n" + "}\n", dinit(CheckOptions, $.cpp = true)); + ASSERT_EQUALS("[test.cpp:6:1]: (error) Resource leak: fp [resourceLeak]\n", errout_str()); } void memcpy1() { // #11542