From c9b85010f9b1df86894bc27ec743f36f3b73289f Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 10 Jun 2022 13:42:02 -0500 Subject: [PATCH] Fix 11096: FP knownConditionTrueFalse in do while loop (#4192) * Check for loop * Improve handling of exit values * Add more checks to test * Simplify * Remove unnecessary test * Fix typo * Format * Use simpleMatch --- lib/valueflow.cpp | 49 ++++++++++++++++++++++++++++++++++++++++-- test/testvalueflow.cpp | 2 ++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 93877fca3..88f3d68a7 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4768,6 +4768,23 @@ static std::vector getConditions(const Token* tok, const char* op) return conds; } +static bool isBreakOrContinueScope(const Token* endToken) +{ + if (!Token::simpleMatch(endToken, "}")) + return false; + return Token::Match(endToken->tokAt(-2), "break|continue ;"); +} + +static const Scope* getLoopScope(const Token* tok) +{ + if (!tok) + return nullptr; + const Scope* scope = tok->scope(); + while (scope && scope->type != Scope::eWhile && scope->type != Scope::eFor && scope->type != Scope::eDo) + scope = scope->nestedIn; + return scope; +} + // static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) { @@ -4828,13 +4845,20 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* // Check if the block terminates early if (isEscapeScope(blockTok, tokenlist)) { + const Scope* scope2 = scope; + // If escaping a loop then only use the loop scope + if (isBreakOrContinueScope(blockTok->link())) { + scope2 = getLoopScope(blockTok->link()); + if (!scope2) + continue; + } for (const Token* condTok2:conds) { ExpressionAnalyzer a1(condTok2, makeConditionValue(0, condTok2, false), tokenlist); - valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a1, settings); + valueFlowGenericForward(startTok->link()->next(), scope2->bodyEnd, a1, settings); if (is1) { OppositeExpressionAnalyzer a2(true, condTok2, makeConditionValue(1, condTok2, false), tokenlist); - valueFlowGenericForward(startTok->link()->next(), scope->bodyEnd, a2, settings); + valueFlowGenericForward(startTok->link()->next(), scope2->bodyEnd, a2, settings); } } } @@ -6049,6 +6073,27 @@ struct ConditionHandler { } if (values.empty()) return; + bool isKnown = std::any_of(values.begin(), values.end(), [&](const ValueFlow::Value& v) { + return v.isKnown() || v.isImpossible(); + }); + if (isKnown && isBreakOrContinueScope(after)) { + const Scope* loopScope = getLoopScope(cond.vartok); + if (loopScope) { + Analyzer::Result r = forward(after, loopScope->bodyEnd, cond.vartok, values, tokenlist); + if (r.terminate != Analyzer::Terminate::None) + return; + if (r.action.isModified()) + return; + Token* start = const_cast(loopScope->bodyEnd); + if (Token::simpleMatch(start, "} while (")) { + start = start->tokAt(2); + forward(start, start->link(), cond.vartok, values, tokenlist); + start = start->link(); + } + values.remove_if(std::mem_fn(&ValueFlow::Value::isImpossible)); + changeKnownToPossible(values); + } + } forward(after, getEndOfExprScope(cond.vartok, scope), cond.vartok, values, tokenlist); } }); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 40b5a5602..dc09943a0 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1444,6 +1444,8 @@ private: " if (x) {}\n" "}\n"; ASSERT_EQUALS(false, testValueOfX(code, 5U, 0)); + ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, 1)); + TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 6U, 0)); } void valueFlowBeforeConditionAssignIncDec() { // assignment / increment