Fix #11311 Do not search for null pointer in dead code

danmar · Jun 2, 2024 · d2f546c · d2f546c
1 parent dfa928f
commit d2f546c
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 27 deletions.
diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
@@ -24,6 +24,7 @@
 #include "ctu.h"
 #include "errorlogger.h"
 #include "errortypes.h"
+#include "findtoken.h"
 #include "library.h"
 #include "mathlib.h"
 #include "settings.h"
@@ -279,47 +280,72 @@ static bool isNullablePointer(const Token* tok)
     return false;
 }
 
-void CheckNullPointer::nullPointerByDeRefAndChec()
+static bool isPointerUnevaluated(const Token *tok)
+{
+    if (isUnevaluated(tok))
+        return true;
+
+    while (tok) {
+        if (isUnevaluated(tok->previous()))
+            return true;
+
+        tok = tok->astParent();
+    }
+    return false;
+}
+
+
+void CheckNullPointer::nullPointerByDeRefAndCheck()
 {
     const bool printInconclusive = (mSettings->certainty.isEnabled(Certainty::inconclusive));
 
-    for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
-        if (isUnevaluated(tok)) {
-            tok = tok->linkAt(1);
-            continue;
-        }
+    const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
+    for (const Scope * scope : symbolDatabase->functionScopes) {
+        auto pred = [printInconclusive](const Token* tok) -> bool {
+            if (!tok)
+                return false;
 
-        if (Token::Match(tok, "%num%|%char%|%str%"))
-            continue;
+            if (Token::Match(tok, "%num%|%char%|%str%"))
+                return false;
 
-        if (!isNullablePointer(tok) ||
-            (tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning
-            continue;
+            if (!isNullablePointer(tok) ||
+                (tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning
+                return false;
 
-        // Can pointer be NULL?
-        const ValueFlow::Value *value = tok->getValue(0);
-        if (!value)
-            continue;
+            // Can pointer be NULL?
+            const ValueFlow::Value *value = tok->getValue(0);
+            if (!value)
+                return false;
 
-        if (!printInconclusive && value->isInconclusive())
-            continue;
+            if (!printInconclusive && value->isInconclusive())
+                return false;
 
-        // Pointer dereference.
-        bool unknown = false;
-        if (!isPointerDeRef(tok,unknown)) {
-            if (unknown)
-                nullPointerError(tok, tok->expressionString(), value, true);
-            continue;
-        }
+            return true;
+        };
+        std::vector<const Token *> tokens = findTokensSkipDeadCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred);
+        for (const Token *tok : tokens) {
+            if (isPointerUnevaluated(tok))
+                continue;
 
-        nullPointerError(tok, tok->expressionString(), value, value->isInconclusive());
+            const ValueFlow::Value *value = tok->getValue(0);
+
+            // Pointer dereference.
+            bool unknown = false;
+            if (!isPointerDeRef(tok, unknown)) {
+                if (unknown)
+                    nullPointerError(tok, tok->expressionString(), value, true);
+                continue;
+            }
+
+            nullPointerError(tok, tok->expressionString(), value, value->isInconclusive());
+        }
     }
 }
 
 void CheckNullPointer::nullPointer()
 {
     logChecker("CheckNullPointer::nullPointer");
-    nullPointerByDeRefAndChec();
+    nullPointerByDeRefAndCheck();
 }
 
 namespace {

diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h
@@ -137,7 +137,7 @@ class CPPCHECKLIB CheckNullPointer : public Check {
      * @brief Does one part of the check for nullPointer().
      * Dereferencing a pointer and then checking if it's NULL..
      */
-    void nullPointerByDeRefAndChec();
+    void nullPointerByDeRefAndCheck();
 
     /** undefined null pointer arithmetic */
     void arithmetic();

diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
@@ -158,6 +158,7 @@ class TestNullPointer : public TestFixture {
         TEST_CASE(nullpointer_in_typeid);
         TEST_CASE(nullpointer_in_alignof); // #11401
         TEST_CASE(nullpointer_in_for_loop);
+        TEST_CASE(nullpointerDeadCode); // #11311
         TEST_CASE(nullpointerDelete);
         TEST_CASE(nullpointerSubFunction);
         TEST_CASE(nullpointerExit);
@@ -3657,6 +3658,47 @@ class TestNullPointer : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void nullpointerDeadCode() {
+        // Ticket #11311
+        check ("void f() {\n"
+               "    if (0)\n"
+               "        *(int *)0 = 1;\n"
+               "    else\n"
+               "         ;\n"
+               "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check ("void f() {\n"
+               "    if (0)\n"
+               "        *(int *)0 = 1;\n"
+               "    else {\n"
+               "        if (0)\n"
+               "            *(int *)0 = 2;\n"
+               "        else\n"
+               "            ;\n"
+               "    }\n"
+               "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check ("void f() {\n"
+               "    while(0)\n"
+               "        *(int*)0 = 1;\n"
+               "    do {\n"
+               "    } while(0);\n"
+               "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check ("int f() {\n"
+               "    return 0 ? *(int*)0 = 1 : 1;\n"
+               "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check ("int f() {\n"
+               "    return 1 ? 1 : *(int*)0 = 1;\n"
+               "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void nullpointerDelete() {
         check("void f() {\n"
               "  K *k = getK();\n"