From 3aef0c9bd3456b2e2e2cf8b2e8e3dee0f5ed8ba0 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 16 Aug 2019 00:48:54 -0500 Subject: [PATCH] Fix issue 8715: regression uninitvar not detected (#2092) --- lib/astutils.cpp | 22 ++++++++++++++++------ lib/astutils.h | 3 +++ lib/checkuninitvar.cpp | 4 ++-- lib/valueflow.cpp | 11 ++++++++++- lib/valueflow.h | 4 ++++ test/testuninitvar.cpp | 13 +++++++++++++ test/testvalueflow.cpp | 2 +- 7 files changed, 49 insertions(+), 10 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 866879c6f..3544c2e19 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1093,22 +1093,32 @@ bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int } bool isVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth) +{ + return findVariableChanged(start, end, varid, globalvar, settings, cpp, depth) != nullptr; +} + +Token* findVariableChanged(Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth) { if (!precedes(start, end)) - return false; + return nullptr; if (depth < 0) - return true; - for (const Token *tok = start; tok != end; tok = tok->next()) { + return start; + for (Token *tok = start; tok != end; tok = tok->next()) { if (tok->varId() != varid) { if (globalvar && Token::Match(tok, "%name% (")) // TODO: Is global variable really changed by function call? - return true; + return tok; continue; } if (isVariableChanged(tok, settings, cpp, depth)) - return true; + return tok; } - return false; + return nullptr; +} + +const Token* findVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth) +{ + return findVariableChanged(const_cast(start), end, varid, globalvar, settings, cpp, depth); } bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth) diff --git a/lib/astutils.h b/lib/astutils.h index 92f0e4d54..491677bc8 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -142,6 +142,9 @@ bool isVariableChanged(const Token *tok, const Settings *settings, bool cpp, int bool isVariableChanged(const Variable * var, const Settings *settings, bool cpp, int depth = 20); +const Token* findVariableChanged(const Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); +Token* findVariableChanged(Token *start, const Token *end, const nonneg int varid, bool globalvar, const Settings *settings, bool cpp, int depth = 20); + bool isAliased(const Variable *var); /** Determines the number of arguments - if token is a function call or macro diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index cacec03e8..3b889e0e2 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1299,7 +1299,7 @@ void CheckUninitVar::valueFlowUninit() } if (!tok->variable()) continue; - if (Token::simpleMatch(tok->astParent(), ".") && tok->astParent()->astOperand1() == tok) + if (Token::Match(tok->astParent(), ". %var%") && tok->astParent()->astOperand1() == tok) continue; auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); if (v == tok->values().end()) @@ -1308,7 +1308,7 @@ void CheckUninitVar::valueFlowUninit() continue; if (!isVariableUsage(tok, tok->variable()->isPointer(), tok->variable()->isArray() ? ARRAY : NO_ALLOC)) continue; - if (isVariableChanged(tok, mSettings, mTokenizer->isCPP())) + if (!Token::Match(tok->astParent(), ". %name% (") && isVariableChanged(tok, mSettings, mTokenizer->isCPP())) continue; uninitvarError(tok, tok->str(), v->errorPath); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 4940da3c3..6ee2cdc3b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2145,7 +2145,16 @@ static bool valueFlowForward(Token * const startToken, // conditional block of code that assigns variable.. else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) { // is variable changed in condition? - if (isVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) { + Token* tokChanged = findVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP()); + if (tokChanged != nullptr) { + // Set the value before bailing + if (tokChanged->varId() == varid) { + for (const ValueFlow::Value &v : values) { + if (!v.isNonValue()) + continue; + setTokenValue(tokChanged, v, settings); + } + } if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); return false; diff --git a/lib/valueflow.h b/lib/valueflow.h index 62a7c7c37..87a93f3f0 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -139,6 +139,10 @@ namespace ValueFlow { return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Argument; } + bool isNonValue() const { + return isMovedValue() || isUninitValue() || isLifetimeValue(); + } + /** int value */ long long intvalue; diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index f958926e1..175b1df06 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -82,6 +82,7 @@ private: TEST_CASE(trac_5970); TEST_CASE(valueFlowUninit); TEST_CASE(uninitvar_ipa); + TEST_CASE(uninitvar_memberfunction); TEST_CASE(isVariableUsageDeref); // *p @@ -4030,6 +4031,18 @@ private: ASSERT_EQUALS("", errout.str()); } + void uninitvar_memberfunction() { + // # 8715 + valueFlowUninit("struct C {\n" + " int x();\n" + "};\n" + "void f() {\n" + " C *c;\n" + " if (c->x() == 4) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: c\n", errout.str()); + } + void isVariableUsageDeref() { // *p checkUninitVar("void f() {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 53ef49150..55f5c9d3a 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -3474,7 +3474,7 @@ private: " if (c->x() == 4) {}\n" "}"; values = tokenValues(code, "c ."); - TODO_ASSERT_EQUALS(true, false, values.size()==1U && values.front().isUninitValue()); + ASSERT_EQUALS(true, values.size()==1U && values.front().isUninitValue()); code = "void f() {\n" " int **x;\n"