From 4d9e69e42c9fff96509dbe388fe8f2502d77b065 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 27 Dec 2023 11:11:57 -0600 Subject: [PATCH] Fix 11985: False positive: uninitvar (valueflow) (#5781) --- lib/analyzer.h | 2 ++ lib/forwardanalyzer.cpp | 25 ++++++++++++++++++------- lib/valueflow.cpp | 7 +++++++ test/testvalueflow.cpp | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/lib/analyzer.h b/lib/analyzer.h index 475c5f28c..d6bcec727 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -181,6 +181,8 @@ struct Analyzer { virtual bool stopOnCondition(const Token* condTok) const = 0; /// The condition that will be assumed during analysis virtual void assume(const Token* tok, bool state, unsigned int flags = 0) = 0; + /// Update the state of the program at the token + virtual void updateState(const Token* tok) = 0; /// Return analyzer for expression at token virtual ValuePtr reanalyze(Token* tok, const std::string& msg = emptyString) const = 0; virtual bool invalid() const { diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index badd544fa..61b9ee517 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -443,7 +443,7 @@ namespace { if (checkElse && isDoWhile && (condTok->hasKnownIntValue() || (!bodyAnalysis.isModified() && !condAnalysis.isModified() && condAnalysis.isRead()))) { - if (updateRange(endBlock->link(), endBlock) == Progress::Break) + if (updateScope(endBlock) == Progress::Break) return Break(); return updateRecursive(condTok); } @@ -519,8 +519,17 @@ namespace { return updateLoop(endToken, endBlock, condTok, initTok, stepTok, true); } - Progress updateScope(Token* endBlock) { - return updateRange(endBlock->link(), endBlock); + Progress updateScope(Token* endBlock, int depth = 20) + { + if (!endBlock) + return Break(); + assert(endBlock->link()); + Token* ctx = endBlock->link()->previous(); + if (Token::simpleMatch(ctx, ")")) + ctx = ctx->link()->previous(); + if (ctx) + analyzer->updateState(ctx); + return updateRange(endBlock->link(), endBlock, depth); } Progress updateRange(Token* start, const Token* end, int depth = 20) { @@ -682,7 +691,7 @@ namespace { thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown); if (thenBranch.check) { thenBranch.active = true; - if (updateRange(endCond->next(), endBlock, depth - 1) == Progress::Break) + if (updateScope(endBlock, depth - 1) == Progress::Break) return Break(); } else if (!elseBranch.check) { thenBranch.active = true; @@ -694,7 +703,7 @@ namespace { elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown); if (elseBranch.check) { elseBranch.active = true; - const Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2), depth - 1); + const Progress result = updateScope(endBlock->linkAt(2), depth - 1); if (result == Progress::Break) return Break(); } else if (!thenBranch.check) { @@ -746,7 +755,7 @@ namespace { } else if (Token::simpleMatch(tok, "try {")) { Token* endBlock = tok->next()->link(); ForwardTraversal tryTraversal = fork(); - tryTraversal.updateRange(tok->next(), endBlock, depth - 1); + tryTraversal.updateScope(endBlock, depth - 1); bool bail = tryTraversal.actions.isModified(); if (bail) { actions = tryTraversal.actions; @@ -760,7 +769,7 @@ namespace { return Break(); endBlock = endCatch->linkAt(1); ForwardTraversal ft = fork(); - ft.updateRange(endBlock->link(), endBlock, depth - 1); + ft.updateScope(endBlock, depth - 1); bail |= ft.terminate != Analyzer::Terminate::None || ft.actions.isModified(); } if (bail) @@ -880,6 +889,8 @@ Analyzer::Result valueFlowGenericForward(Token* start, const Token* end, const V if (a->invalid()) return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail}; ForwardTraversal ft{a, settings}; + if (start) + ft.analyzer->updateState(start); ft.updateRange(start, end); return Analyzer::Result{ ft.actions, ft.terminate }; } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 04db01b8f..8f1088fc7 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2969,6 +2969,13 @@ struct ValueFlowAnalyzer : Analyzer { makeConditional(); } + void updateState(const Token* tok) override + { + // Update program state + pms.removeModifiedVars(tok); + pms.addState(tok, getProgramState()); + } + virtual void internalUpdate(Token* /*tok*/, const ValueFlow::Value& /*v*/, Direction /*d*/) { assert(false && "Internal update unimplemented."); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 679a006f7..6c8cea602 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -876,6 +876,23 @@ private: " y=x;\n" "}"; ASSERT_EQUALS(false, testValueOfX(code, 3U, ValueFlow::Value::MoveKind::MovedVariable)); + + code = "void g(std::string);\n" + "void foo(std::string x) {\n" + " g(std::move(x));\n" + " int counter = 0;\n" + "\n" + " for (int i = 0; i < 5; i++) {\n" + " if (i % 2 == 0) {\n" + " x = std::to_string(i);\n" + " counter++;\n" + " }\n" + " }\n" + " for (int i = 0; i < counter; i++) {\n" + " if(x > 5) {}\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 13U, ValueFlow::Value::MoveKind::MovedVariable)); } void valueFlowCalculations() { @@ -5671,6 +5688,22 @@ private: "}"; values = tokenValues(code, "buflen >=", ValueFlow::Value::ValueType::UNINIT); ASSERT_EQUALS(1, values.size()); + + code = "void foo() {\n" + " int counter = 0;\n" + " int x;\n" + " for (int i = 0; i < 5; i++) {\n" + " if (i % 2 == 0) {\n" + " x = i;\n" + " counter++;\n" + " }\n" + " }\n" + " for (int i = 0; i < counter; i++) {\n" + " if(x > 5) {}\n" + " }\n" + "}\n"; + values = tokenValues(code, "x > 5", ValueFlow::Value::ValueType::UNINIT); + ASSERT_EQUALS(0, values.size()); } void valueFlowConditionExpressions() {