From 7acbb656f3cc26d5894c896699f6d48a61658c75 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 9 Dec 2022 00:15:47 -0600 Subject: [PATCH] Fix 11412: False positive: uninitvar (#4624) * Dont remove modified variables from dead code * Add test for 11412 * Format --- lib/forwardanalyzer.cpp | 14 +++++++------- lib/programmemory.cpp | 8 ++++++-- lib/valueflow.cpp | 10 ++++++---- test/testuninitvar.cpp | 11 +++++++++++ 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index abae3dd70..15baa44b1 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -394,24 +394,24 @@ struct ForwardTraversal { return true; if (Token::simpleMatch(condTok, ":")) return true; - bool changed = false; + bool stepChangesCond = false; if (stepTok) { std::pair exprToks = stepTok->findExpressionStartEndTokens(); if (exprToks.first != nullptr && exprToks.second != nullptr) - changed |= isExpressionChanged(condTok, exprToks.first, exprToks.second->next(), settings, true); + stepChangesCond |= isExpressionChanged(condTok, exprToks.first, exprToks.second->next(), settings, true); } - changed |= isExpressionChanged(condTok, endBlock->link(), endBlock, settings, true); + const bool bodyChangesCond = isExpressionChanged(condTok, endBlock->link(), endBlock, settings, true); // Check for mutation in the condition - changed |= nullptr != - findAstNode(condTok, [&](const Token* tok) { + const bool condChanged = + nullptr != findAstNode(condTok, [&](const Token* tok) { return isVariableChanged(tok, 0, settings, true); }); + const bool changed = stepChangesCond || bodyChangesCond || condChanged; if (!changed) return true; ForwardTraversal ft = fork(true); - ft.analyzer->assume(condTok, false, Analyzer::Assume::Absolute); ft.updateScope(endBlock); - return ft.isConditionTrue(condTok); + return ft.isConditionTrue(condTok) && bodyChangesCond; } Progress updateInnerLoop(Token* endBlock, Token* stepTok, Token* condTok) { diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 14b6882cb..527511ce5 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -461,8 +461,12 @@ void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty) programMemoryParseCondition(pm, tok, nullptr, settings, b); const Token* origin = tok; const Token* top = tok->astTop(); - if (top && Token::Match(top->previous(), "for|while (")) - origin = top->link(); + if (top && Token::Match(top->previous(), "for|while|if (") && !Token::simpleMatch(tok->astParent(), "?")) { + origin = top->link()->next(); + if (!b && origin->link()) { + origin = origin->link(); + } + } replace(pm, origin); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2838265eb..72e9d0af4 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2790,11 +2790,13 @@ struct ValueFlowAnalyzer : Analyzer { if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while (")) startBlock = parent->linkAt(-2); const Token* endBlock = startBlock->link(); - pms.removeModifiedVars(endBlock); - if (state) + if (state) { + pms.removeModifiedVars(endBlock); pms.addState(endBlock->previous(), getProgramState()); - else if (Token::simpleMatch(endBlock, "} else {")) - pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); + } else { + if (Token::simpleMatch(endBlock, "} else {")) + pms.addState(endBlock->linkAt(2)->previous(), getProgramState()); + } } if (!(flags & Assume::Quiet)) { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 1cc77a657..497a23ebe 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -5396,6 +5396,17 @@ private: " return n;\n" "}\n"); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Uninitialized variable: i\n", errout.str()); + + // #11412 + valueFlowUninit("void f(int n) {\n" + " short* p;\n" + " if (n) {\n" + " p = g(n);\n" + " }\n" + " for (int i = 0; i < n; i++)\n" + " (void)p[i];\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value