From efc7d79f814ef3352bf7581a4abd3e13b57d420f Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 27 Apr 2024 15:54:03 -0500 Subject: [PATCH 1/5] Fix 11996: FP: knownConditionTrueFalse --- lib/astutils.cpp | 8 ++++++++ lib/forwardanalyzer.cpp | 17 ++++++----------- lib/reverseanalyzer.cpp | 18 +++++++++++++----- test/testcondition.cpp | 11 +++++++++++ 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 486eb730e4a..562717627ea 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -815,6 +815,12 @@ 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 +837,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..06cf3010bf4 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) { 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() From 765787a5fd66d6f6b8028fdfc234e5d3255cc69e Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 27 Apr 2024 15:54:48 -0500 Subject: [PATCH 2/5] Format --- lib/astutils.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 562717627ea..48e9a5d2d4c 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -815,8 +815,7 @@ static T* getCondTokImpl(T* tok) return nullptr; if (Token::simpleMatch(tok, "(")) return getCondTok(tok->previous()); - if (Token::simpleMatch(tok, "do {")) - { + if (Token::simpleMatch(tok, "do {")) { T* endTok = tok->linkAt(1); if (Token::simpleMatch(endTok, "} while (")) return endTok->tokAt(2)->astOperand2(); From 7f754a976b04d7a23c9d71ecd27bf18f27f90472 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 3 May 2024 19:46:53 -0500 Subject: [PATCH 3/5] Improve catch scope --- lib/forwardanalyzer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 06cf3010bf4..3f88855aa9f 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -640,7 +640,7 @@ namespace { assert(!inDoWhile || Token::simpleMatch(tok, "} while (")); if (Token::simpleMatch(tok, "} else {") || inDoWhile) tok = tok->linkAt(2); - } else if (scope->type == Scope::eTry) { + } else if (ontains({Scope::eDo, Scope::eTry, Scope::eCatch}, scope->type)) { if (!analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } else if (scope->type == Scope::eLambda) { From 9bfe714a9477122b22f8aea5192bab6d325df875 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 3 May 2024 19:47:15 -0500 Subject: [PATCH 4/5] Remove do --- lib/forwardanalyzer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 3f88855aa9f..12cd6000e76 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -640,7 +640,7 @@ namespace { assert(!inDoWhile || Token::simpleMatch(tok, "} while (")); if (Token::simpleMatch(tok, "} else {") || inDoWhile) tok = tok->linkAt(2); - } else if (ontains({Scope::eDo, Scope::eTry, Scope::eCatch}, scope->type)) { + } else if (ontains({Scope::eTry, Scope::eCatch}, scope->type)) { if (!analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } else if (scope->type == Scope::eLambda) { From 0b543f72f9ce0bdf920bf0400442b8ce605674a0 Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 4 May 2024 11:59:59 -0500 Subject: [PATCH 5/5] Fix typo --- lib/forwardanalyzer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 12cd6000e76..12b0e689bf9 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -640,7 +640,7 @@ namespace { assert(!inDoWhile || Token::simpleMatch(tok, "} while (")); if (Token::simpleMatch(tok, "} else {") || inDoWhile) tok = tok->linkAt(2); - } else if (ontains({Scope::eTry, Scope::eCatch}, scope->type)) { + } else if (contains({Scope::eTry, Scope::eCatch}, scope->type)) { if (!analyzer->lowerToPossible()) return Break(Analyzer::Terminate::Bail); } else if (scope->type == Scope::eLambda) {