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

Makefile

@@ -533,7 +533,7 @@ $(libcppdir)/checkleakautovar.o: lib/checkleakautovar.cpp lib/addoninfo.h lib/as
 $(libcppdir)/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkmemoryleak.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp
 
-$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
+$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/findtoken.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
 
 $(libcppdir)/checkother.o: lib/checkother.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkother.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h

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,54 @@ static bool isNullablePointer(const Token* tok)
     return false;
 }
 
-void CheckNullPointer::nullPointerByDeRefAndChec()
+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 = findTokensSkipDeadAndUnevaluatedCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred);
+        for (const Token *tok : tokens) {
+            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());
+            nullPointerError(tok, tok->expressionString(), value, value->isInconclusive());
+        }
     }
 }
 
 void CheckNullPointer::nullPointer()
 {
     logChecker("CheckNullPointer::nullPointer");
-    nullPointerByDeRefAndChec();
+    nullPointerByDeRefAndCheck();
 }
 
 namespace {

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();

lib/findtoken.h

@@ -84,7 +84,8 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
                                 const Token* end,
                                 const Predicate& pred,
                                 Found found,
-                                const Evaluate& evaluate)
+                                const Evaluate& evaluate,
+                                bool skipUnevaluated)
 {
     for (T* tok = start; precedes(tok, end); tok = tok->next()) {
         if (pred(tok)) {
@@ -98,7 +99,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
             auto result = evaluate(condTok);
             if (result.empty())
                 continue;
-            if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate))
+            if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate, skipUnevaluated))
                 return true;
             T* thenStart = tok->linkAt(1)->next();
             T* elseStart = nullptr;
@@ -108,7 +109,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
             int r = result.front();
             if (r == 0) {
                 if (elseStart) {
-                    if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate))
+                    if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate, skipUnevaluated))
                         return true;
                     if (isReturnScope(elseStart->link(), library))
                         return true;
@@ -117,7 +118,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
                     tok = thenStart->link();
                 }
             } else {
-                if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate))
+                if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate, skipUnevaluated))
                     return true;
                 if (isReturnScope(thenStart->link(), library))
                     return true;
@@ -137,7 +138,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
                 if (!cond) {
                     next = colon;
                 } else {
-                    if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate))
+                    if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate, skipUnevaluated))
                         return true;
                     next = nextAfterAstRightmostLeaf(colon);
                 }
@@ -164,6 +165,12 @@ bool findTokensSkipDeadCodeImpl(const Library& library,
             else
                 tok = afterCapture;
         }
+        if (skipUnevaluated && isUnevaluated(tok)) {
+            T *next = tok->linkAt(1);
+            if (!next)
+                continue;
+            tok = next;
+        }
     }
     return false;
 }
@@ -185,7 +192,8 @@ std::vector<T*> findTokensSkipDeadCode(const Library& library,
         result.push_back(tok);
         return false;
     },
-        evaluate);
+        evaluate,
+        false);
     return result;
 }
 
@@ -195,6 +203,35 @@ std::vector<T*> findTokensSkipDeadCode(const Library& library, T* start, const T
     return findTokensSkipDeadCode(library, start, end, pred, &evaluateKnownValues);
 }
 
+template<class T, class Predicate, class Evaluate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
+std::vector<T*> findTokensSkipDeadAndUnevaluatedCode(const Library& library,
+                                                     T* start,
+                                                     const Token* end,
+                                                     const Predicate& pred,
+                                                     const Evaluate& evaluate)
+{
+    std::vector<T*> result;
+    (void)findTokensSkipDeadCodeImpl(
+        library,
+        start,
+        end,
+        pred,
+        [&](T* tok) {
+        result.push_back(tok);
+        return false;
+    },
+        evaluate,
+        true);
+    return result;
+}
+
+template<class T, class Predicate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
+std::vector<T*> findTokensSkipDeadAndUnevaluatedCode(const Library& library, T* start, const Token* end, const Predicate& pred)
+{
+    return findTokensSkipDeadAndUnevaluatedCode(library, start, end, pred, &evaluateKnownValues);
+}
+
+
 template<class T, class Predicate, class Evaluate, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
 T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, const Predicate& pred, const Evaluate& evaluate)
 {
@@ -208,7 +245,8 @@ T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, con
         result = tok;
         return true;
     },
-        evaluate);
+        evaluate,
+        false);
     return result;
 }
 

oss-fuzz/Makefile

@@ -212,7 +212,7 @@ $(libcppdir)/checkleakautovar.o: ../lib/checkleakautovar.cpp ../lib/addoninfo.h
 $(libcppdir)/checkmemoryleak.o: ../lib/checkmemoryleak.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkmemoryleak.h ../lib/color.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h
 	$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp
 
-$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h
+$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/findtoken.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h
 	$(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp
 
 $(libcppdir)/checkother.o: ../lib/checkother.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkother.h ../lib/config.h ../lib/errortypes.h ../lib/fwdanalysis.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h

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"