From 9d4e3829c29b5abf1e8db8353efecdad86e4d630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 12 Oct 2023 13:56:47 +0200 Subject: [PATCH] Partial fix #12030 (False positive: uninitialized variable, conditional modification, flag) (#5543) --- lib/checkuninitvar.cpp | 63 ++++++++++++++++++++++++------------------ lib/checkuninitvar.h | 2 +- test/testuninitvar.cpp | 12 ++++++++ 3 files changed, 49 insertions(+), 28 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index b1684a1d72d..a23124acd48 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -82,6 +82,27 @@ static const Token *getAstParentSkipPossibleCastAndAddressOf(const Token *vartok return parent; } +static std::map getVariableValues(const Token* tok) { + std::map ret; + if (!tok || !tok->scope()->isExecutable()) + return ret; + while (tok && tok->str() != "{") { + if (tok->str() == "}") { + if (tok->link()->isBinaryOp()) + tok = tok->link()->previous(); + else + break; + } + if (Token::Match(tok, "%var% =|{") && tok->next()->isBinaryOp() && tok->varId() && ret.count(tok->varId()) == 0) { + const Token* rhs = tok->next()->astOperand2(); + if (rhs && rhs->hasKnownIntValue()) + ret[tok->varId()] = VariableValue(rhs->getKnownIntValue()); + } + tok = tok->previous(); + } + return ret; +} + bool CheckUninitVar::diag(const Token* tok) { if (!tok) @@ -175,14 +196,14 @@ void CheckUninitVar::checkScope(const Scope* scope, const std::set } if (!init) { Alloc alloc = ARRAY; - const std::map variableValue; + std::map variableValue = getVariableValues(var.typeStartToken()); checkScopeForVariable(tok, var, nullptr, nullptr, &alloc, emptyString, variableValue); } continue; } if (stdtype || var.isPointer()) { Alloc alloc = NO_ALLOC; - const std::map variableValue; + std::map variableValue = getVariableValues(var.typeStartToken()); checkScopeForVariable(tok, var, nullptr, nullptr, &alloc, emptyString, variableValue); } if (var.type()) @@ -207,7 +228,7 @@ void CheckUninitVar::checkScope(const Scope* scope, const std::set checkStruct(tok, arg); else if (arg.typeStartToken()->isStandardType() || arg.typeStartToken()->isEnumType()) { Alloc alloc = NO_ALLOC; - const std::map variableValue; + std::map variableValue; checkScopeForVariable(tok->next(), arg, nullptr, nullptr, &alloc, emptyString, variableValue); } } @@ -246,7 +267,7 @@ void CheckUninitVar::checkStruct(const Token *tok, const Variable &structvar) const Token *tok2 = tok; if (tok->str() == "}") tok2 = tok2->next(); - const std::map variableValue; + std::map variableValue = getVariableValues(structvar.typeStartToken()); checkScopeForVariable(tok2, structvar, nullptr, nullptr, &alloc, var.name(), variableValue); } } @@ -377,7 +398,7 @@ static bool isVariableUsed(const Token *tok, const Variable& var) return !parent2 || parent2->isConstOp() || (parent2->str() == "=" && parent2->astOperand2() == parent); } -bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, Alloc* const alloc, const std::string &membervar, std::map variableValue) +bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var, bool * const possibleInit, bool * const noreturn, Alloc* const alloc, const std::string &membervar, std::map& variableValue) { const bool suppressErrors(possibleInit && *possibleInit); // Assume that this is a variable declaration, rather than a fundef const bool printDebug = mSettings->debugwarnings; @@ -421,7 +442,7 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var } // track values of other variables.. - if (Token::Match(tok->previous(), "[;{}] %var% =")) { + if (Token::Match(tok->previous(), "[;{}.] %var% =")) { if (tok->next()->astOperand2() && tok->next()->astOperand2()->hasKnownIntValue()) variableValue[tok->varId()] = VariableValue(tok->next()->astOperand2()->getKnownIntValue()); else if (Token::Match(tok->previous(), "[;{}] %var% = - %name% ;")) @@ -489,7 +510,8 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var if (tok->str() == "{") { bool possibleInitIf((!alwaysTrue && number_of_if > 0) || suppressErrors); bool noreturnIf = false; - const bool initif = !alwaysFalse && checkScopeForVariable(tok->next(), var, &possibleInitIf, &noreturnIf, alloc, membervar, variableValue); + std::map varValueIf(variableValue); + const bool initif = !alwaysFalse && checkScopeForVariable(tok->next(), var, &possibleInitIf, &noreturnIf, alloc, membervar, varValueIf); // bail out for such code: // if (a) x=0; // conditional initialization @@ -511,15 +533,8 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var if (alwaysTrue && (initif || noreturnIf)) return true; - std::map varValueIf; - if (!alwaysFalse && !initif && !noreturnIf) { - for (const Token *tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) { - if (Token::Match(tok2, "[;{}.] %name% = - %name% ;")) - varValueIf[tok2->next()->varId()] = !VariableValue(0); - else if (Token::Match(tok2, "[;{}.] %name% = %num% ;")) - varValueIf[tok2->next()->varId()] = VariableValue(MathLib::toBigNumber(tok2->strAt(3))); - } - } + if (!alwaysFalse && !initif && !noreturnIf) + variableValue = varValueIf; if (initif && condVarId > 0) variableValue[condVarId] = !condVarValue; @@ -539,17 +554,11 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var bool possibleInitElse((!alwaysFalse && number_of_if > 0) || suppressErrors); bool noreturnElse = false; - const bool initelse = !alwaysTrue && checkScopeForVariable(tok->next(), var, &possibleInitElse, &noreturnElse, alloc, membervar, variableValue); - - std::map varValueElse; - if (!alwaysTrue && !initelse && !noreturnElse) { - for (const Token *tok2 = tok; tok2 && tok2 != tok->link(); tok2 = tok2->next()) { - if (Token::Match(tok2, "[;{}.] %var% = - %name% ;")) - varValueElse[tok2->next()->varId()] = !VariableValue(0); - else if (Token::Match(tok2, "[;{}.] %var% = %num% ;")) - varValueElse[tok2->next()->varId()] = VariableValue(MathLib::toBigNumber(tok2->strAt(3))); - } - } + std::map varValueElse(variableValue); + const bool initelse = !alwaysTrue && checkScopeForVariable(tok->next(), var, &possibleInitElse, &noreturnElse, alloc, membervar, varValueElse); + + if (!alwaysTrue && !initelse && !noreturnElse) + variableValue = varValueElse; if (initelse && condVarId > 0 && !noreturnIf && !noreturnElse) variableValue[condVarId] = condVarValue; diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 8ab6fd9b8bd..8ce2cc8ccdb 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -88,7 +88,7 @@ class CPPCHECKLIB CheckUninitVar : public Check { void check(); void checkScope(const Scope* scope, const std::set &arrayTypeDefs); void checkStruct(const Token *tok, const Variable &structvar); - bool checkScopeForVariable(const Token *tok, const Variable& var, bool* const possibleInit, bool* const noreturn, Alloc* const alloc, const std::string &membervar, std::map variableValue); + bool checkScopeForVariable(const Token *tok, const Variable& var, bool* const possibleInit, bool* const noreturn, Alloc* const alloc, const std::string &membervar, std::map& variableValue); const Token* checkExpr(const Token* tok, const Variable& var, const Alloc alloc, bool known, bool* bailout = nullptr) const; bool checkIfForWhileHead(const Token *startparentheses, const Variable& var, bool suppressErrors, bool isuninit, Alloc alloc, const std::string &membervar); bool checkLoopBody(const Token *tok, const Variable& var, const Alloc alloc, const std::string &membervar, const bool suppressErrors); diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 51d5bc0569e..960d2a6b7d7 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -1239,6 +1239,18 @@ class TestUninitVar : public TestFixture { " return v2 + v4 + v6 + v7;\n" "}\n"); ASSERT_EQUALS("[test.cpp:12]: (error) Uninitialized variable: v5\n", errout.str()); + + checkUninitVar("bool set(int *p);\n" + "\n" + "void foo(bool a) {\n" + " bool flag{false};\n" + " int x;\n" + " if (!a) {\n" + " flag = set(&x);\n" + " }\n" + " if (!flag || x == 0) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); }