diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index c2f29b92d..a2d62ea55 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -1,4 +1,5 @@ #include "forwardanalyzer.h" +#include "analyzer.h" #include "astutils.h" #include "settings.h" #include "symboldatabase.h" @@ -18,6 +19,29 @@ struct ForwardTraversal { Analyzer::Action actions; bool analyzeOnly; + struct Branch { + Analyzer::Action action = Analyzer::Action::None; + bool check = false; + bool escape = false; + bool escapeUnknown = false; + const Token* endBlock = nullptr; + bool isEscape() const { + return escape || escapeUnknown; + } + bool isConclusiveEscape() const { + return escape && !escapeUnknown; + } + bool isModified() const { + return action.isModified() && !isConclusiveEscape(); + } + bool isInconclusive() const { + return action.isInconclusive() && !isConclusiveEscape(); + } + bool isDead() const { + return action.isModified() || action.isInconclusive() || isEscape(); + } + }; + bool stopUpdates() { analyzeOnly = true; return actions.isModified(); @@ -186,6 +210,11 @@ struct ForwardTraversal { return result; } + void forkRange(Token* start, const Token* end) { + ForwardTraversal ft = *this; + ft.updateRange(start, end); + } + void forkScope(Token* endBlock, bool isModified = false) { if (analyzer->updateScope(endBlock, isModified)) { ForwardTraversal ft = *this; @@ -197,11 +226,11 @@ struct ForwardTraversal { return Token::findsimplematch(endBlock->link(), "goto", endBlock); } - bool isEscapeScope(const Token* endBlock, bool unknown = false) { + bool isEscapeScope(const Token* endBlock, bool& unknown) { const Token* ftok = nullptr; bool r = isReturnScope(endBlock, &settings->library, &ftok); if (!r && ftok) - return unknown; + unknown = true; return r; } @@ -379,34 +408,32 @@ struct ForwardTraversal { // Traverse condition if (updateRecursive(condTok) == Progress::Break) return Progress::Break; + Branch thenBranch{}; + Branch elseBranch{}; // Check if condition is true or false - bool checkThen, checkElse; - std::tie(checkThen, checkElse) = evalCond(condTok); - Analyzer::Action thenAction = Analyzer::Action::None; - Analyzer::Action elseAction = Analyzer::Action::None; + std::tie(thenBranch.check, elseBranch.check) = evalCond(condTok); bool hasElse = Token::simpleMatch(endBlock, "} else {"); bool bail = false; // Traverse then block - bool returnThen = isEscapeScope(endBlock, true); - bool returnElse = false; - if (checkThen) { + thenBranch.escape = isEscapeScope(endBlock, thenBranch.escapeUnknown); + if (thenBranch.check) { if (updateRange(endCond->next(), endBlock) == Progress::Break) return Progress::Break; - } else if (!checkElse) { - thenAction = checkScope(endBlock); + } else if (!elseBranch.check) { + thenBranch.action = checkScope(endBlock); if (hasGoto(endBlock)) bail = true; } // Traverse else block if (hasElse) { - returnElse = isEscapeScope(endBlock->linkAt(2), true); - if (checkElse) { + elseBranch.escape = isEscapeScope(endBlock->linkAt(2), elseBranch.escapeUnknown); + if (elseBranch.check) { Progress result = updateRange(endBlock->tokAt(2), endBlock->linkAt(2)); if (result == Progress::Break) return Progress::Break; - } else if (!checkThen) { - elseAction = checkScope(endBlock->linkAt(2)); + } else if (!thenBranch.check) { + elseBranch.action = checkScope(endBlock->linkAt(2)); if (hasGoto(endBlock)) bail = true; } @@ -414,18 +441,18 @@ struct ForwardTraversal { } else { tok = endBlock; } - actions |= (thenAction | elseAction); + actions |= (thenBranch.action | elseBranch.action); if (bail) return Progress::Break; - if (returnThen && returnElse) - return Progress::Break; - else if (thenAction.isModified() && elseAction.isModified()) - return Progress::Break; - else if ((returnThen || returnElse) && (thenAction.isModified() || elseAction.isModified())) + if (thenBranch.isDead() && elseBranch.isDead()) return Progress::Break; // Conditional return - if (returnThen && !hasElse) { - if (checkThen) { + if (thenBranch.isEscape() && !hasElse) { + if (!thenBranch.isConclusiveEscape()) { + if (!analyzer->lowerToInconclusive()) + return Progress::Break; + } + else if (thenBranch.check) { return Progress::Break; } else { if (analyzer->isConditional() && stopUpdates()) @@ -433,15 +460,15 @@ struct ForwardTraversal { analyzer->assume(condTok, false); } } - if (thenAction.isInconclusive() || elseAction.isInconclusive()) { + if (thenBranch.isInconclusive() || elseBranch.isInconclusive()) { if (!analyzer->lowerToInconclusive()) return Progress::Break; - } else if (thenAction.isModified() || elseAction.isModified()) { + } else if (thenBranch.isModified() || elseBranch.isModified()) { if (!hasElse && analyzer->isConditional() && stopUpdates()) return Progress::Break; if (!analyzer->lowerToPossible()) return Progress::Break; - analyzer->assume(condTok, elseAction.isModified()); + analyzer->assume(condTok, elseBranch.isModified()); } } } else if (Token::simpleMatch(tok, "try {")) { diff --git a/lib/templatesimplifier.cpp b/lib/templatesimplifier.cpp index 294475d52..6ba042c55 100644 --- a/lib/templatesimplifier.cpp +++ b/lib/templatesimplifier.cpp @@ -445,7 +445,7 @@ unsigned int TemplateSimplifier::templateParameters(const Token *tok) } // skip std:: - if (tok && tok->str() == "::") + if (tok->str() == "::") tok = tok->next(); while (Token::Match(tok, "%name% ::")) { tok = tok->tokAt(2); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2a1ce2a57..049ed2104 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2314,7 +2314,7 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer { if (value.conditional) return true; if (value.condition) - return !value.isImpossible(); + return !value.isKnown() && !value.isImpossible(); return false; } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 7db46fb64..02f844c29 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3494,6 +3494,23 @@ private: " if (p != (uint32_t) -1) {}\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #9890 + check("int g(int);\n" + "bool h(int*);\n" + "int f(int *x) {\n" + " int y = g(0);\n" + " if (!y) {\n" + " if (h(x)) {\n" + " y = g(1);\n" + " if (y) {}\n" + " return 0;\n" + " }\n" + " if (!y) {}\n" + " }\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:11]: (style) Condition '!y' is always true\n", errout.str()); } void alwaysTrueInfer() {