Fix #10572 FP nullPointerRedundantCheck with try/catch / #10701 FP kn…

…ownConditionTrueFalse with nested try/catch (#5761)
danmar · Dec 13, 2023 · 8205b4a · 8205b4a
1 parent bc50237
commit 8205b4a
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 1 deletion.
diff --git 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
@@ -5232,6 +5232,19 @@ class TestCondition : public TestFixture {
               "    }\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
@@ -141,6 +141,7 @@ class TestNullPointer : public TestFixture {
         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 @@ class TestNullPointer : public TestFixture {
         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 @@ class TestNullPointer : public TestFixture {
         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
@@ -5407,6 +5407,23 @@ class TestUnusedVar : public TestFixture {
                               "    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() {