diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index a1c999317..bdf648110 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -12,6 +12,14 @@ #include #include +struct OnExit { + std::function f; + + ~OnExit() { + f(); + } +}; + struct ForwardTraversal { enum class Progress { Continue, Break, Skip }; enum class Terminate { None, Bail, Escape, Modified, Inconclusive, Conditional }; @@ -25,6 +33,7 @@ struct ForwardTraversal { bool analyzeTerminate; Analyzer::Terminate terminate = Analyzer::Terminate::None; bool forked = false; + std::vector loopEnds = {}; Progress Break(Analyzer::Terminate t = Analyzer::Terminate::None) { if ((!analyzeOnly || analyzeTerminate) && t != Analyzer::Terminate::None) @@ -85,9 +94,15 @@ struct ForwardTraversal { template )> Progress traverseTok(T* tok, std::function f, bool traverseUnknown, T** out = nullptr) { - if (Token::Match(tok, "asm|goto|continue|setjmp|longjmp")) - return Break(); - else if (Token::Match(tok, "return|throw") || isEscapeFunction(tok, &settings->library)) { + if (Token::Match(tok, "asm|goto|setjmp|longjmp")) + return Break(Analyzer::Terminate::Bail); + else if (Token::simpleMatch(tok, "continue")) { + if (loopEnds.empty()) + return Break(Analyzer::Terminate::Escape); + // If we are in a loop then jump to the end + if (out) + *out = loopEnds.back(); + } else if (Token::Match(tok, "return|throw") || isEscapeFunction(tok, &settings->library)) { traverseRecursive(tok->astOperand1(), f, traverseUnknown); traverseRecursive(tok->astOperand2(), f, traverseUnknown); return Break(Analyzer::Terminate::Escape); @@ -361,6 +376,10 @@ struct ForwardTraversal { } Progress updateInnerLoop(Token* endBlock, Token* stepTok, Token* condTok) { + loopEnds.push_back(endBlock); + OnExit oe{[&] { + loopEnds.pop_back(); + }}; if (endBlock && updateScope(endBlock) == Progress::Break) return Break(); if (stepTok && updateRecursive(stepTok) == Progress::Break) @@ -632,7 +651,7 @@ struct ForwardTraversal { } actions |= (thenBranch.action | elseBranch.action); if (bail) - return Break(); + return Break(Analyzer::Terminate::Bail); if (thenBranch.isDead() && elseBranch.isDead()) { if (thenBranch.isModified() && elseBranch.isModified()) return Break(Analyzer::Terminate::Modified); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6e801d2a0..585c7a6ea 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5313,6 +5313,7 @@ struct ConditionHandler { startTokens[1] = top->link()->linkAt(1)->tokAt(2); int changeBlock = -1; + int bailBlock = -1; for (int i = 0; i < 2; i++) { const Token* const startToken = startTokens[i]; @@ -5326,6 +5327,9 @@ struct ConditionHandler { deadBranch[i] = r.terminate == Analyzer::Terminate::Escape; if (r.action.isModified() && !deadBranch[i]) changeBlock = i; + if (r.terminate != Analyzer::Terminate::None && r.terminate != Analyzer::Terminate::Escape && + r.terminate != Analyzer::Terminate::Modified) + bailBlock = i; changeKnownToPossible(values); } if (changeBlock >= 0 && !Token::simpleMatch(top->previous(), "while (")) { @@ -5336,6 +5340,13 @@ struct ConditionHandler { "valueFlowAfterCondition: " + cond.vartok->expressionString() + " is changed in conditional block"); return; + } else if (bailBlock >= 0) { + if (settings->debugwarnings) + bailout(tokenlist, + errorLogger, + startTokens[bailBlock]->link(), + "valueFlowAfterCondition: bailing in conditional block"); + return; } // After conditional code.. diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index b4586485c..7cde18eab 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -121,6 +121,7 @@ private: TEST_CASE(nullpointer79); // #10400 TEST_CASE(nullpointer80); // #10410 TEST_CASE(nullpointer81); // #8724 + TEST_CASE(nullpointer82); // #10331 TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -2474,6 +2475,26 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer82() // #10331 + { + check("bool g();\n" + "int* h();\n" + "void f(int* ptr) {\n" + " if (!ptr) {\n" + " if (g())\n" + " goto done;\n" + " ptr = h();\n" + " if (!ptr)\n" + " return;\n" + " }\n" + " if (*ptr == 1)\n" + " return;\n" + "\n" + "done:\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f8d417c94..61c9c71c8 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1703,7 +1703,8 @@ private: " if (x==123){}\n" "}"); ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n", + "[test.cpp:3]: (debug) valueflow.cpp::valueFlowConditionExpressions bailout: Skipping function due to incomplete variable a\n" + "[test.cpp:2]: (debug) valueflow.cpp::(valueFlow) bailout: valueFlowAfterCondition: bailing in conditional block\n", errout.str()); // #5721 - FP @@ -2952,6 +2953,18 @@ private: " return 0;\n" "}\n"; ASSERT_EQUALS(true, testValueOfXKnown(code, 5U, 1)); + + code = "int f(int x) {\n" + " if (x == 1) {\n" + " for(int i=0;i<1;i++) {\n" + " if (x == 1)\n" + " continue;\n" + " }\n" + " }\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 8U, 1)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 8U, 1)); } void valueFlowAfterConditionExpr() {