From d4d77edeae25c2051ccf311ec9dd83fb0fd594b7 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sat, 12 Aug 2023 23:46:31 +0200 Subject: [PATCH] Fix FP uninitStructMember / cleanup from #5311 (#5315) --- lib/astutils.cpp | 29 ++++++++++++++++------------- lib/astutils.h | 2 +- lib/checkother.cpp | 2 +- lib/checkuninitvar.cpp | 14 +++++++++----- test/testuninitvar.cpp | 28 ++++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index b1dc9900a..db5058b07 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -3129,30 +3129,33 @@ static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings return ExprUsage::Inconclusive; } -ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings) +ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, bool cpp) { - if (indirect > 0 && tok->astParent()) { - if (Token::Match(tok->astParent(), "%assign%") && astIsRHS(tok)) + const Token* const parent = tok->astParent(); + if (indirect > 0 && parent) { + if (Token::Match(parent, "%assign%") && astIsRHS(tok)) return ExprUsage::NotUsed; - if (tok->astParent()->isConstOp()) + if (parent->isConstOp()) return ExprUsage::NotUsed; - if (tok->astParent()->isCast()) + if (parent->isCast()) return ExprUsage::NotUsed; - if (Token::simpleMatch(tok->astParent(), ":") && Token::simpleMatch(tok->astParent()->astParent(), "?")) - return getExprUsage(tok->astParent()->astParent(), indirect, settings); + if (Token::simpleMatch(parent, ":") && Token::simpleMatch(parent->astParent(), "?")) + return getExprUsage(parent->astParent(), indirect, settings, cpp); } if (indirect == 0) { - if (Token::Match(tok->astParent(), "%cop%|%assign%|++|--") && !Token::Match(tok->astParent(), "=|>>") && - !tok->astParent()->isUnaryOp("&")) + if (Token::Match(parent, "%cop%|%assign%|++|--") && parent->str() != "=" && + !parent->isUnaryOp("&") && + !(astIsRHS(tok) && isLikelyStreamRead(cpp, parent))) return ExprUsage::Used; - if (Token::simpleMatch(tok->astParent(), "=") && astIsRHS(tok)) { - if (tok->astParent()->astOperand1() && tok->astParent()->astOperand1()->variable() && tok->astParent()->astOperand1()->variable()->isReference()) + if (Token::simpleMatch(parent, "=") && astIsRHS(tok)) { + const Token* const lhs = parent->astOperand1(); + if (lhs && lhs->variable() && lhs->variable()->isReference() && lhs == lhs->variable()->nameToken()) return ExprUsage::NotUsed; return ExprUsage::Used; } // Function call or index - if (((Token::simpleMatch(tok->astParent(), "(") && !tok->astParent()->isCast()) || (Token::simpleMatch(tok->astParent(), "[") && tok->valueType())) && - (astIsLHS(tok) || Token::simpleMatch(tok->astParent(), "( )"))) + if (((Token::simpleMatch(parent, "(") && !parent->isCast()) || (Token::simpleMatch(parent, "[") && tok->valueType())) && + (astIsLHS(tok) || Token::simpleMatch(parent, "( )"))) return ExprUsage::Used; } return getFunctionUsage(tok, indirect, settings); diff --git a/lib/astutils.h b/lib/astutils.h index 04143bc7f..f486418ef 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -409,7 +409,7 @@ bool isConstVarExpression(const Token* tok, std::function sk enum class ExprUsage { None, NotUsed, PassedByReference, Used, Inconclusive }; -ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings); +ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings, bool cpp); const Variable *getLHSVariable(const Token *tok); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index f11a523a9..8663ed91e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3388,7 +3388,7 @@ void CheckOther::checkAccessOfMovedVariable() else inconclusive = true; } else { - const ExprUsage usage = getExprUsage(tok, 0, mSettings); + const ExprUsage usage = getExprUsage(tok, 0, mSettings, mTokenizer->isCPP()); if (usage == ExprUsage::Used) accessOfMoved = true; if (usage == ExprUsage::PassedByReference) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 887e38b61..ca701d697 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -709,8 +709,10 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var if (!membervar.empty()) { if (!suppressErrors && Token::Match(tok, "%name% . %name%") && tok->strAt(2) == membervar && Token::Match(tok->next()->astParent(), "%cop%|return|throw|?")) uninitStructMemberError(tok, tok->str() + "." + membervar); - else if (mTokenizer->isCPP() && !suppressErrors && Token::Match(tok, "%name%") && Token::Match(tok->astParent(), "return|throw|?")) - uninitStructMemberError(tok, tok->str() + "." + membervar); + else if (mTokenizer->isCPP() && !suppressErrors && Token::Match(tok, "%name%") && Token::Match(tok->astParent(), "return|throw|?")) { + if (std::any_of(tok->values().cbegin(), tok->values().cend(), std::mem_fn(&ValueFlow::Value::isUninitValue))) + uninitStructMemberError(tok, tok->str() + "." + membervar); + } } // Use variable @@ -1488,8 +1490,10 @@ bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, All if (!isPointer && !Token::simpleMatch(tok->astParent(), ".") && Token::Match(tok->previous(), "[(,] %name% [,)]") && isVariableUsage(tok, isPointer, alloc)) return true; - if (!isPointer && Token::Match(tok->previous(), "= %name% ;")) - return true; + if (!isPointer && Token::Match(tok->previous(), "= %name% ;")) { + const Token* lhs = tok->previous()->astOperand1(); + return !(lhs && lhs->variable() && lhs->variable()->isReference() && lhs == lhs->variable()->nameToken()); + } // = *(&var); if (!isPointer && @@ -1639,7 +1643,7 @@ void CheckUninitVar::valueFlowUninit() if (!isleaf && Token::Match(tok->astParent(), ". %name%") && (tok->astParent()->next()->varId() || tok->astParent()->next()->isEnumerator())) continue; } - const ExprUsage usage = getExprUsage(tok, v->indirect, mSettings); + const ExprUsage usage = getExprUsage(tok, v->indirect, mSettings, mTokenizer->isCPP()); if (usage == ExprUsage::NotUsed || usage == ExprUsage::Inconclusive) continue; if (!v->subexpressions.empty() && usage == ExprUsage::PassedByReference) diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index f7432725d..2a54b03c6 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4787,6 +4787,21 @@ private: " }\n" "}"); ASSERT_EQUALS("", errout.str()); + + checkUninitVar("struct S { int x; };\n" + "S h() {\n" + " S s;\n" + " S& r = s;\n" + " r.x = 0;\n" + " return s;\n" + "}\n" + "S i() {\n" + " S s;\n" + " S& r{ s };\n" + " r.x = 0;\n" + " return s;\n" + "}"); + ASSERT_EQUALS("", errout.str()); } void uninitvar2_while() { @@ -6169,6 +6184,19 @@ private: " return s->i;\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f(int i) {\n" + " int x;\n" + " int* p = &x;\n" + " return i >> *p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: *p\n", errout.str()); + + valueFlowUninit("void f(int& x) {\n" + " int i;\n" + " x = i;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: i\n", errout.str()); } void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value