From 115be7dfc8c793dc19097fd912f72ef734605bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 1 Jan 2019 18:23:47 +0100 Subject: [PATCH] ValueFlow: better FwdAnalysis for complex expressions --- lib/astutils.cpp | 19 ++++++++++++++++--- lib/astutils.h | 13 +++++++++---- lib/valueflow.cpp | 9 +++------ test/testvalueflow.cpp | 10 ++++++++++ 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 1d4b2d6a9..8d15c960b 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -1121,6 +1121,10 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const } if (tok->str() == "}") { + // Known value => possible value + if (tok->scope() == expr->scope()) + mValueFlowKnown = false; + Scope::ScopeType scopeType = tok->scope()->type; if (scopeType == Scope::eWhile || scopeType == Scope::eFor || scopeType == Scope::eDo) { // check condition @@ -1173,8 +1177,12 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const parent = parent->astParent(); if (parent && isSameExpression(mCpp, false, expr, parent, mLibrary, false, false, nullptr)) { same = true; - if (mWhat == What::ValueFlow) - mValueFlow.push_back(parent); + if (mWhat == What::ValueFlow) { + KnownAndToken v; + v.known = mValueFlowKnown; + v.token = parent; + mValueFlow.push_back(v); + } } if (!same && Token::Match(parent, ". %var%") && parent->next()->varId() && exprVarIds.find(parent->next()->varId()) == exprVarIds.end()) { other = true; @@ -1212,9 +1220,13 @@ struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const const Result &result1 = checkRecursive(expr, tok->tokAt(2), tok->linkAt(1), exprVarIds, local); if (result1.type == Result::Type::READ || result1.type == Result::Type::BAILOUT) return result1; + if (mWhat == What::ValueFlow && result1.type == Result::Type::WRITE) + mValueFlowKnown = false; if (Token::simpleMatch(tok->linkAt(1), "} else {")) { const Token *elseStart = tok->linkAt(1)->tokAt(2); const Result &result2 = checkRecursive(expr, elseStart, elseStart->link(), exprVarIds, local); + if (mWhat == What::ValueFlow && result2.type == Result::Type::WRITE) + mValueFlowKnown = false; if (result2.type == Result::Type::READ || result2.type == Result::Type::BAILOUT) return result2; if (result1.type == Result::Type::WRITE && result2.type == Result::Type::WRITE) @@ -1362,9 +1374,10 @@ bool FwdAnalysis::unusedValue(const Token *expr, const Token *startToken, const return (result.type == FwdAnalysis::Result::Type::NONE || result.type == FwdAnalysis::Result::Type::RETURN) && !possiblyAliased(expr, startToken); } -std::vector FwdAnalysis::valueFlow(const Token *expr, const Token *startToken, const Token *endToken) +std::vector FwdAnalysis::valueFlow(const Token *expr, const Token *startToken, const Token *endToken) { mWhat = What::ValueFlow; + mValueFlowKnown = true; check(expr, startToken, endToken); return mValueFlow; } diff --git a/lib/astutils.h b/lib/astutils.h index ace3c5531..1725de938 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -170,7 +170,7 @@ bool isConstVarExpression(const Token *tok); */ class FwdAnalysis { public: - FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mWhat(What::Reassign) {} + FwdAnalysis(bool cpp, const Library &library) : mCpp(cpp), mLibrary(library), mWhat(What::Reassign), mValueFlowKnown(true) {} bool hasOperand(const Token *tok, const Token *lhs) const; @@ -192,13 +192,17 @@ public: */ bool unusedValue(const Token *expr, const Token *startToken, const Token *endToken); - std::vector valueFlow(const Token *expr, const Token *startToken, const Token *endToken); + struct KnownAndToken { + bool known; + const Token *token; + }; + + std::vector valueFlow(const Token *expr, const Token *startToken, const Token *endToken); /** Is there some possible alias for given expression */ bool possiblyAliased(const Token *expr, const Token *startToken) const; static bool isNullOperand(const Token *expr); - private: /** Result of forward analysis */ struct Result { @@ -217,7 +221,8 @@ private: const bool mCpp; const Library &mLibrary; enum class What { Reassign, UnusedValue, ValueFlow } mWhat; - std::vector mValueFlow; + std::vector mValueFlow; + bool mValueFlowKnown; }; #endif // astutilsH diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index d96988ab1..8cab41f00 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4683,12 +4683,9 @@ static void valueFlowFwdAnalysis(const TokenList *tokenlist, const Settings *set while (functionScope->nestedIn && functionScope->nestedIn->isExecutable()) functionScope = functionScope->nestedIn; const Token *endToken = functionScope->bodyEnd; - for (const Token *tok2 : fwdAnalysis.valueFlow(tok->astOperand1(), startToken, endToken)) { - const Scope *s = tok2->scope(); - while (s && s != tok->scope()) - s = s->nestedIn; - v.valueKind = s ? ValueFlow::Value::ValueKind::Known : ValueFlow::Value::ValueKind::Possible; - setTokenValue(const_cast(tok2), v, settings); + for (const FwdAnalysis::KnownAndToken read : fwdAnalysis.valueFlow(tok->astOperand1(), startToken, endToken)) { + v.valueKind = read.known ? ValueFlow::Value::ValueKind::Known : ValueFlow::Value::ValueKind::Possible; + setTokenValue(const_cast(read.token), v, settings); } } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 44b958269..4663c24e9 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -2436,6 +2436,16 @@ private: ASSERT_EQUALS(true, values.front().isKnown()); ASSERT_EQUALS(true, values.front().isIntValue()); ASSERT_EQUALS(1, values.front().intvalue); + + code = "void f() {\n" + " Hints hints;\n" + " hints.x = 1;\n" + " if (foo)\n" + " hints.x = 2;\n" + " x = 0 + foo.x;\n" // <- foo.x is possible 1, possible 2 + "}"; + values = tokenValues(code, "+"); + TODO_ASSERT_EQUALS(2U, 0U, values.size()); // should be 2 } void valueFlowSwitchVariable() {