From 52081ef08fcb90b61ae02cc9b8167e78fd7737ee Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Mon, 14 Aug 2023 03:27:00 -0500 Subject: [PATCH] Add special function to match lifetimes (#5320) This also removes the termination checking in `valueFlowUninit` as this causes a lot of FNs. --- lib/checkuninitvar.cpp | 4 +++- lib/valueflow.cpp | 43 ++++++++++++++++++++++++++++-------------- test/testuninitvar.cpp | 9 ++++----- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index ca701d697..239f1ad8d 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -710,7 +710,9 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var 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|?")) { - if (std::any_of(tok->values().cbegin(), tok->values().cend(), std::mem_fn(&ValueFlow::Value::isUninitValue))) + if (std::any_of(tok->values().cbegin(), tok->values().cend(), [](const ValueFlow::Value& v) { + return v.isUninitValue() && !v.isInconclusive(); + })) uninitStructMemberError(tok, tok->str() + "." + membervar); } } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c18e2ce45..713b2dad9 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -641,9 +641,10 @@ static void setTokenValue(Token* tok, } } - if (Token::simpleMatch(parent, "=") && astIsRHS(tok) && !value.isLifetimeValue()) { - setTokenValue(parent, std::move(value), settings); - return; + if (Token::simpleMatch(parent, "=") && astIsRHS(tok)) { + setTokenValue(parent, value, settings); + if (!value.isUninitValue()) + return; } if (value.isContainerSizeValue() && astIsContainer(tok)) { @@ -2433,6 +2434,20 @@ struct ValueFlowAnalyzer : Analyzer { return settings; } + // Returns Action::Match if its an exact match, return Action::Read if it partially matches the lifetime + Action analyzeLifetime(const Token* tok) const + { + if (!tok) + return Action::None; + if (match(tok)) + return Action::Match; + if (Token::simpleMatch(tok, ".") && analyzeLifetime(tok->astOperand1()) != Action::None) + return Action::Read; + if (astIsRHS(tok) && Token::simpleMatch(tok->astParent(), ".")) + return analyzeLifetime(tok->astParent()); + return Action::None; + } + struct ConditionState { bool dependent = true; bool unknown = true; @@ -2806,7 +2821,10 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; lifeTok = v.tokvalue; } - if (lifeTok && match(lifeTok)) { + if (!lifeTok) + return Action::None; + Action la = analyzeLifetime(lifeTok); + if (la.matches()) { Action a = Action::Read; if (isModified(tok).isModified()) a = Action::Invalid; @@ -2816,6 +2834,9 @@ struct ValueFlowAnalyzer : Analyzer { return Action::Inconclusive; return a; } + if (la.isRead()) { + return isAliasModified(tok); + } return Action::None; } else if (isAlias(ref, inconclusive)) { @@ -3299,12 +3320,6 @@ struct MemberExpressionAnalyzer : SubExpressionAnalyzer { : SubExpressionAnalyzer(e, std::move(val), t, s), varname(std::move(varname)) {} - bool match(const Token* tok) const override - { - return SubExpressionAnalyzer::match(tok) || - (Token::simpleMatch(tok->astParent(), ".") && SubExpressionAnalyzer::match(tok->astParent())); - } - bool submatch(const Token* tok, bool exact) const override { if (!Token::Match(tok, ". %var%")) @@ -3334,6 +3349,8 @@ static std::string lifetimeType(const Token *tok, const ValueFlow::Value *val) case ValueFlow::Value::LifetimeKind::Address: if (astIsPointer(tok)) result = "pointer"; + else if (Token::simpleMatch(tok, "=") && astIsPointer(tok->astOperand2())) + result = "pointer"; else result = "object"; break; @@ -7977,7 +7994,6 @@ static void valueFlowUninit(TokenList& tokenlist, const Settings* settings) Token* start = findStartToken(var, tok->next(), &settings->library); std::map partialReads; - Analyzer::Result result; if (const Scope* scope = var->typeScope()) { if (Token::findsimplematch(scope->bodyStart, "union", scope->bodyEnd)) continue; @@ -7992,7 +8008,7 @@ static void valueFlowUninit(TokenList& tokenlist, const Settings* settings) continue; } MemberExpressionAnalyzer analyzer(memVar.nameToken()->str(), tok, uninitValue, tokenlist, settings); - result = valueFlowGenericForward(start, tok->scope()->bodyEnd, analyzer, *settings); + valueFlowGenericForward(start, tok->scope()->bodyEnd, analyzer, *settings); for (auto&& p : *analyzer.partialReads) { Token* tok2 = p.first; @@ -8022,8 +8038,7 @@ static void valueFlowUninit(TokenList& tokenlist, const Settings* settings) if (partial) continue; - if (result.terminate != Analyzer::Terminate::Modified) - valueFlowForward(start, tok->scope()->bodyEnd, var->nameToken(), uninitValue, tokenlist, settings); + valueFlowForward(start, tok->scope()->bodyEnd, var->nameToken(), uninitValue, tokenlist, settings); } } diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 2a54b03c6..9cc8b5030 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -6209,7 +6209,7 @@ private: " memcpy(wcsin, x, sizeof(wcsstruct));\n" // <- warning " x->wcsprm = NULL;\n" // <- no warning "}"); - ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: x.wcsprm\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: x\n", errout.str()); valueFlowUninit("struct wcsstruct {\n" " int *wcsprm;\n" @@ -7037,8 +7037,7 @@ private: " memcpy(in, s, sizeof(S));\n" " s->p = NULL;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: s.p\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: s\n", errout.str()); valueFlowUninit("struct S {\n" // #11321 " int a = 0;\n" @@ -7298,7 +7297,7 @@ private: " A::B b;\n" " x.push_back(b);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:9]: (error) Uninitialized variable: b.i\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (error) Uninitialized variable: b\n", errout.str()); valueFlowUninit("struct A {\n" " struct B {\n" @@ -7322,7 +7321,7 @@ private: " A a;\n" " x.push_back(a);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:9]: (error) Uninitialized variable: a.j\n", errout.str()); + ASSERT_EQUALS("[test.cpp:9]: (error) Uninitialized variable: a\n", errout.str()); valueFlowUninit("struct S { struct T { int* p; } t[2]; };\n" // #11018 "void f() {\n"