From 665cfce030fae63011cadfcc4ca50fc06df6fef3 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 4 May 2024 13:00:50 -0500 Subject: [PATCH] Fix 11996: FP: knownConditionTrueFalse (#6357) --- lib/astutils.cpp | 7 +++++++ lib/forwardanalyzer.cpp | 19 +++++++------------ lib/reverseanalyzer.cpp | 18 +++++++++++++----- test/testcondition.cpp | 11 +++++++++++ 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index afc5ef81ac1..348f9ec641c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -815,6 +815,11 @@ static T* getCondTokImpl(T* tok) return nullptr; if (Token::simpleMatch(tok, "(")) return getCondTok(tok->previous()); + if (Token::simpleMatch(tok, "do {")) { + T* endTok = tok->linkAt(1); + if (Token::simpleMatch(endTok, "} while (")) + return endTok->tokAt(2)->astOperand2(); + } if (Token::simpleMatch(tok, "for") && Token::simpleMatch(tok->next()->astOperand2(), ";") && tok->next()->astOperand2()->astOperand2()) return tok->next()->astOperand2()->astOperand2()->astOperand1(); @@ -831,6 +836,8 @@ static T* getCondTokFromEndImpl(T* endBlock) T* startBlock = endBlock->link(); if (!Token::simpleMatch(startBlock, "{")) return nullptr; + if (Token::simpleMatch(startBlock->previous(), "do")) + return getCondTok(startBlock->previous()); if (Token::simpleMatch(startBlock->previous(), ")")) return getCondTok(startBlock->previous()->link()); if (Token::simpleMatch(startBlock->tokAt(-2), "} else {")) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 8a60d278971..12b0e689bf9 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -605,10 +605,10 @@ namespace { const Scope* scope = tok->scope(); if (!scope) return Break(); - if (Token::Match(tok->link()->previous(), ")|else {")) { - const Token* tok2 = tok->link()->previous(); - const bool inElse = Token::simpleMatch(tok2, "else {"); - const bool inLoop = inElse ? false : Token::Match(tok2->link()->previous(), "while|for ("); + if (contains({Scope::eDo, Scope::eFor, Scope::eWhile, Scope::eIf, Scope::eElse}, scope->type)) { + const bool inElse = scope->type == Scope::eElse; + const bool inDoWhile = scope->type == Scope::eDo; + const bool inLoop = contains({Scope::eDo, Scope::eFor, Scope::eWhile}, scope->type); Token* condTok = getCondTokFromEnd(tok); if (!condTok) return Break(); @@ -637,19 +637,14 @@ namespace { } } analyzer->assume(condTok, !inElse, Analyzer::Assume::Quiet); - if (Token::simpleMatch(tok, "} else {")) + assert(!inDoWhile || Token::simpleMatch(tok, "} while (")); + if (Token::simpleMatch(tok, "} else {") || inDoWhile) tok = tok->linkAt(2); - } else if (scope->type == Scope::eTry) { + } else if (contains({Scope::eTry, Scope::eCatch}, scope->type)) { if (!analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } else if (scope->type == Scope::eLambda) { return Break(); - } else if (scope->type == Scope::eDo && Token::simpleMatch(tok, "} while (")) { - if (updateLoopExit(end, tok, tok->tokAt(2)->astOperand2()) == Progress::Break) - return Break(); - tok = tok->linkAt(2); - } else if (Token::simpleMatch(tok->next(), "else {")) { - tok = tok->linkAt(2); } } else if (tok->isControlFlowKeyword() && Token::Match(tok, "if|while|for (") && Token::simpleMatch(tok->next()->link(), ") {")) { diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 637b47483cb..22a7fda933a 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -106,6 +106,17 @@ namespace { return top; } + static Token* jumpToStart(Token* tok) + { + if (Token::simpleMatch(tok->tokAt(-2), "} else {")) + tok = tok->linkAt(-2); + if (Token::simpleMatch(tok->previous(), ") {")) + return tok->previous()->link(); + if (Token::simpleMatch(tok->previous(), "do {")) + return tok->previous(); + return tok; + } + bool updateRecursive(Token* start) { bool continueB = true; visitAstNodes(start, [&](Token* tok) { @@ -326,7 +337,7 @@ namespace { // If the condition modifies the variable then bail if (condAction.isModified()) break; - tok = condTok->astTop()->previous(); + tok = jumpToStart(tok->link()); continue; } if (tok->str() == "{") { @@ -343,10 +354,7 @@ namespace { if (r.action.isModified()) break; } - if (Token::simpleMatch(tok->tokAt(-2), "} else {")) - tok = tok->linkAt(-2); - if (Token::simpleMatch(tok->previous(), ") {")) - tok = tok->previous()->link(); + tok = jumpToStart(tok); continue; } if (Token* next = isUnevaluated(tok)) { diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 6f4feed4fc9..9222c4f27f0 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -5263,6 +5263,17 @@ class TestCondition : public TestFixture { " for (int i = 1000; i < 20; ++i) {}\n" "}\n"); ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'i<20' is always false\n", errout_str()); + + check("int foo(int foo, int bar, bool baz, bool flag) {\n" + " if (baz && flag) {\n" + " do {\n" + " if (bar==42)\n" + " return 0;\n" + " } while (flag);\n" + " }\n" + " return foo;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (style) Condition 'flag' is always true\n", errout_str()); } void alwaysTrueTryCatch()