Skip to content

Commit

Permalink
Fix 11985: False positive: uninitvar (valueflow) (#5781)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pfultz2
pfultz2 authored Dec 27, 2023
1 parent b6e1574 commit 4d9e69e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 7 deletions.
2 changes: 2 additions & 0 deletions lib/analyzer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Analyzer> reanalyze(Token* tok, const std::string& msg = emptyString) const = 0;
virtual bool invalid() const {
Expand Down
25 changes: 18 additions & 7 deletions lib/forwardanalyzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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 };
}
Expand Down
7 changes: 7 additions & 0 deletions lib/valueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
33 changes: 33 additions & 0 deletions test/testvalueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,23 @@ class TestValueFlow : public TestFixture {
" 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() {
Expand Down Expand Up @@ -5671,6 +5688,22 @@ class TestValueFlow : public TestFixture {
"}";
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() {
Expand Down

2 comments on commit 4d9e69e

@firewave
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfultz2 It appears this introduced a performance regression. At least the selfchecks are slower now. Will dig into this later.

@chrchr-github
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://trac.cppcheck.net/ticket/12329

Please sign in to comment.