From 9770dd7e0bd05e27dcba0a9d4bab51c844a2b494 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 3 May 2023 23:03:47 -0500 Subject: [PATCH] Fix 11673: FP uninitvar when capturing by reference (#4984) * Fix 11673: FP uninitvar when capturing by reference * Format * Fix tests --- lib/valueflow.cpp | 116 ++++++++++++++++++++++++++++++++++++----- test/testuninitvar.cpp | 15 ++++-- test/testvalueflow.cpp | 2 +- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b9c522b7b..46ccb6997 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7751,23 +7751,115 @@ static void addToErrorPath(ValueFlow::Value& value, const ValueFlow::Value& from }); } -static std::vector findAllUsages(const Variable* var, Token* start) // cppcheck-suppress constParameterPointer // FP +template +bool findTokenSkipDeadCodeImpl(const Library* library, Token* start, const Token* end, Predicate pred, Found found) +{ + for (Token* tok = start; precedes(tok, end); tok = tok->next()) { + if (pred(tok)) { + if (found(tok)) + return true; + } + if (Token::simpleMatch(tok, "if (")) { + const Token* condTok = tok->next()->astOperand2(); + if (!condTok) + continue; + if (!condTok->hasKnownIntValue()) + continue; + if (!Token::simpleMatch(tok->linkAt(1), ") {")) + continue; + if (findTokenSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found)) + return true; + Token* thenStart = tok->linkAt(1)->next(); + Token* elseStart = nullptr; + if (Token::simpleMatch(thenStart->link(), "} else {")) + elseStart = thenStart->link()->tokAt(2); + + int r = condTok->values().front().intvalue; + if (r == 0) { + if (elseStart) { + if (findTokenSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found)) + return true; + if (isReturnScope(elseStart->link(), library)) + return true; + tok = elseStart->link(); + } else { + tok = thenStart->link(); + } + } else { + if (findTokenSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found)) + return true; + if (isReturnScope(thenStart->link(), library)) + return true; + tok = thenStart->link(); + } + } else if (Token::Match(tok->astParent(), "&&|?|%oror%") && astIsLHS(tok) && tok->hasKnownIntValue()) { + int r = tok->values().front().intvalue; + Token* next = nullptr; + if ((r == 0 && Token::simpleMatch(tok->astParent(), "||")) || + (r != 0 && Token::simpleMatch(tok->astParent(), "&&"))) { + next = nextAfterAstRightmostLeaf(tok->astParent()); + } else if (Token::simpleMatch(tok->astParent(), "?")) { + Token* colon = tok->astParent()->astOperand2(); + if (r == 0) { + next = colon; + } else { + if (findTokenSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found)) + return true; + next = nextAfterAstRightmostLeaf(colon); + } + } + if (next) + tok = next; + } else if (Token::simpleMatch(tok, "} else {")) { + const Token* condTok = getCondTokFromEnd(tok); + if (!condTok) + continue; + if (!condTok->hasKnownIntValue()) + continue; + if (isReturnScope(tok->link(), library)) + return true; + int r = condTok->values().front().intvalue; + if (r != 0) { + tok = tok->linkAt(1); + } + } else if (Token::simpleMatch(tok, "[") && Token::Match(tok->link(), "] (|{")) { + Token* afterCapture = tok->link()->next(); + if (Token::simpleMatch(afterCapture, "(") && afterCapture->link()) + tok = afterCapture->link()->next(); + else + tok = afterCapture; + } + } + return false; +} + +template +std::vector findTokensSkipDeadCode(const Library* library, Token* start, const Token* end, Predicate pred) { std::vector result; - const Scope* scope = var->scope(); - if (!scope) - return result; - Token* tok2 = Token::findmatch(start, "%varid%", scope->bodyEnd, var->declarationId()); - while (tok2) { - result.push_back(tok2); - tok2 = Token::findmatch(tok2->next(), "%varid%", scope->bodyEnd, var->declarationId()); - } + findTokenSkipDeadCodeImpl(library, start, end, pred, [&](Token* tok) { + result.push_back(tok); + return false; + }); return result; } -static Token* findStartToken(const Variable* var, Token* start) +static std::vector findAllUsages(const Variable* var, + Token* start, // cppcheck-suppress constParameterPointer // FP + const Library* library) { - std::vector uses = findAllUsages(var, start); + // std::vector result; + const Scope* scope = var->scope(); + if (!scope) + return {}; + return findTokensSkipDeadCode(library, start, scope->bodyEnd, [&](const Token* tok) { + return tok->varId() == var->declarationId(); + }); +} + +static Token* findStartToken(const Variable* var, Token* start, const Library* library) +{ + std::vector uses = findAllUsages(var, start, library); if (uses.empty()) return start; Token* first = uses.front(); @@ -7823,7 +7915,7 @@ static void valueFlowUninit(TokenList* tokenlist, SymbolDatabase* /*symbolDataba bool partial = false; - Token* start = findStartToken(var, tok->next()); + Token* start = findStartToken(var, tok->next(), &settings->library); std::map partialReads; if (const Scope* scope = var->typeScope()) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index b15ba1da5..0b149fe0d 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -3498,7 +3498,7 @@ private: " }\n" "}", "test.cpp"); - TODO_ASSERT_EQUALS("", "[test.cpp:6]: (error) Uninitialized variable: i\n", errout.str()); + ASSERT_EQUALS("", errout.str()); valueFlowUninit("void f() {\n" " int i, y;\n" @@ -3509,7 +3509,7 @@ private: " }\n" "}", "test.cpp"); - TODO_ASSERT_EQUALS("", "[test.cpp:6]: (error) Uninitialized variable: i\n", errout.str()); + ASSERT_EQUALS("", errout.str()); valueFlowUninit("void f() {\n" " int i, y;\n" @@ -5970,6 +5970,15 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + // #11673 + valueFlowUninit("void f() {\n" + " bool b;\n" + " auto g = [&b]() {\n" + " b = true;\n" + " };\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + // #6619 valueFlowUninit("void f() {\n" " int nok, i;\n" @@ -5979,7 +5988,7 @@ private: " }\n" " printf(\"nok = %d\\n\", nok);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:7]: (warning) Uninitialized variable: nok\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (error) Uninitialized variable: nok\n", errout.str()); // #7475 valueFlowUninit("struct S {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 06b463bd7..0829974e3 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -5445,7 +5445,7 @@ private: " }\n" "}\n"; values = tokenValues(code, "i ++", ValueFlow::Value::ValueType::UNINIT); - TODO_ASSERT_EQUALS(0, 1, values.size()); + ASSERT_EQUALS(0, values.size()); } void valueFlowConditionExpressions() {