From 8205b4a4b3c9198ed488883cd08cd0ed9ec9e589 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Wed, 13 Dec 2023 19:22:54 +0100 Subject: [PATCH] Fix #10572 FP nullPointerRedundantCheck with try/catch / #10701 FP knownConditionTrueFalse with nested try/catch (#5761) --- lib/forwardanalyzer.cpp | 5 +++- test/testcondition.cpp | 13 ++++++++++ test/testnullpointer.cpp | 51 ++++++++++++++++++++++++++++++++++++++++ test/testunusedvar.cpp | 17 ++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 6b95905c1..badd544fa 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -748,8 +748,11 @@ namespace { ForwardTraversal tryTraversal = fork(); tryTraversal.updateRange(tok->next(), endBlock, depth - 1); bool bail = tryTraversal.actions.isModified(); - if (bail) + if (bail) { + actions = tryTraversal.actions; + terminate = tryTraversal.terminate; return Break(); + } while (Token::simpleMatch(endBlock, "} catch (")) { Token* endCatch = endBlock->linkAt(2); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 17a28fbe8..2c3a77e3a 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -5232,6 +5232,19 @@ private: " }\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" // #10701 + " std::string s;\n" + " try {\n" + " try {\n" + " s = g();\n" + " }\n" + " catch (const Err& err) {}\n" + " }\n" + " catch (const std::exception& e) {}\n" + " if (s != \"abc\") {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void multiConditionAlwaysTrue() { diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index aa498142a..f984b5aac 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -141,6 +141,7 @@ private: TEST_CASE(nullpointer100); // #11636 TEST_CASE(nullpointer101); // #11382 TEST_CASE(nullpointer102); + TEST_CASE(nullpointer103); TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -2658,6 +2659,26 @@ private: ASSERT_EQUALS( "[test.cpp:9] -> [test.cpp:8]: (warning) Either the condition 'ptr->y!=nullptr' is redundant or there is possible null pointer dereference: ptr->y.\n", errout.str()); + + check("bool argsMatch(const Token *first, const Token *second) {\n" // #6145 + " if (first->str() == \")\")\n" + " return true;\n" + " else if (first->next()->str() == \"=\")\n" + " first = first->nextArgument();\n" + " else if (second->next()->str() == \"=\") {\n" + " second = second->nextArgument();\n" + " if (second)\n" + " second = second->tokAt(-2);\n" + " if (!first || !second) {\n" + " return !first && !second;\n" + " }\n" + " }\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS( + "[test.cpp:10] -> [test.cpp:2]: (warning) Either the condition '!first' is redundant or there is possible null pointer dereference: first.\n" + "[test.cpp:10] -> [test.cpp:4]: (warning) Either the condition '!first' is redundant or there is possible null pointer dereference: first.\n", + errout.str()); } void nullpointer90() // #6098 @@ -2858,6 +2879,36 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer103() + { + check("struct S {\n" // #10572 + " int f();\n" + " int* m_P{};\n" + "};\n" + "int S::f() {\n" + " if (!m_P) {\n" + " try {\n" + " m_P = new int(1);\n" + " }\n" + " catch (...) {\n" + " return 0;\n" + " }\n" + " }\n" + " return *m_P;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int* p, const int* q) {\n" // #11873 + " if (*q == -1)\n" + " *p = 0;\n" + "}\n" + "void g() {\n" + " int x = -2;\n" + " f(nullptr, &x);\n" + "}\n"); + TODO_ASSERT_EQUALS("", "[test.cpp:3]: (warning) Possible null pointer dereference: p\n", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n" diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index fa46a644c..82fc3a114 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -5407,6 +5407,23 @@ private: " return 0;\n" "}"); ASSERT_EQUALS("", errout.str()); + + functionVariableUsage("bool argsMatch(const Token *first, const Token *second) {\n" // #6145 + " if (first->str() == \")\")\n" + " return true;\n" + " else if (first->next()->str() == \"=\")\n" + " first = first->nextArgument();\n" + " else if (second->next()->str() == \"=\") {\n" + " second = second->nextArgument();\n" + " if (second)\n" + " second = second->tokAt(-2);\n" + " if (!first || !second) {\n" + " return !first && !second;\n" + " }\n" + " }\n" + " return false;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Variable 'first' is assigned a value that is never used.\n", errout.str()); } void localvarIfElse() {