From 0be6e27231b851339cb9e0278c7828c1eb225768 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Mon, 6 Dec 2021 13:06:48 -0600
Subject: [PATCH] Fix 10640: FN nullPointerRedundantCheck (#3607)

* Fix 10640: FN nullPointerRedundantCheck

* Format
---
 lib/checknullpointer.cpp | 21 ++++++---------------
 lib/reverseanalyzer.cpp  |  6 ++++++
 test/testnullpointer.cpp | 18 ++++++++++++++++++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
index b236bed2d..622710ffb 100644
--- a/lib/checknullpointer.cpp
+++ b/lib/checknullpointer.cpp
@@ -168,7 +168,8 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
     const Token* parent = tok->astParent();
     if (!parent)
         return false;
-    if (parent->str() == "." && parent->astOperand2() == tok)
+    const bool addressOf = parent->astParent() && parent->astParent()->str() == "&";
+    if (parent->str() == "." && astIsRHS(tok))
         return isPointerDeRef(parent, unknown, settings);
     const bool firstOperand = parent->astOperand1() == tok;
     parent = astParentSkipParens(tok);
@@ -176,11 +177,11 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
         return false;
 
     // Dereferencing pointer..
-    if (parent->isUnaryOp("*") && !Token::Match(parent->tokAt(-2), "sizeof|decltype|typeof"))
+    if (parent->isUnaryOp("*") && !addressOf)
         return true;
 
     // array access
-    if (firstOperand && parent->str() == "[" && (!parent->astParent() || parent->astParent()->str() != "&"))
+    if (firstOperand && parent->str() == "[" && !addressOf)
         return true;
 
     // address of member variable / array element
@@ -191,18 +192,8 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
         return false;
 
     // read/write member variable
-    if (firstOperand && parent->originalName() == "->" && (!parent->astParent() || parent->astParent()->str() != "&")) {
-        if (!parent->astParent())
-            return true;
-        if (!Token::Match(parent->astParent()->previous(), "if|while|for|switch ("))
-            return true;
-        if (!Token::Match(parent->astParent()->previous(), "%name% ("))
-            return true;
-        if (parent->astParent() == tok->previous())
-            return true;
-        unknown = true;
-        return false;
-    }
+    if (firstOperand && parent->originalName() == "->" && !addressOf)
+        return true;
 
     // If its a function pointer then check if its called
     if (tok->variable() && tok->variable()->isPointer() && Token::Match(tok->variable()->nameToken(), "%name% ) (") &&
diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
index f16107617..84df4a535 100644
--- a/lib/reverseanalyzer.cpp
+++ b/lib/reverseanalyzer.cpp
@@ -302,6 +302,12 @@ struct ReverseTraversal {
                     if (action.isModified())
                         break;
                 }
+                Token* condTok = getCondTokFromEnd(tok->link());
+                if (condTok) {
+                    Analyzer::Result r = valueFlowGenericForward(condTok, analyzer, settings);
+                    if (r.action.isModified())
+                        break;
+                }
                 if (Token::simpleMatch(tok->tokAt(-2), "} else {"))
                     tok = tok->linkAt(-2);
                 if (Token::simpleMatch(tok->previous(), ") {"))
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index 4a4776703..a027719c9 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -128,6 +128,7 @@ private:
         TEST_CASE(nullpointer86);
         TEST_CASE(nullpointer87); // #9291
         TEST_CASE(nullpointer88); // #9949
+        TEST_CASE(nullpointer89); // #10640
         TEST_CASE(nullpointer_addressOf); // address of
         TEST_CASE(nullpointerSwitch); // #2626
         TEST_CASE(nullpointer_cast); // #4692
@@ -2618,6 +2619,23 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void nullpointer89() // #10640
+    {
+        check("typedef struct {\n"
+              "    int x;\n"
+              "} foo_t;\n"
+              "typedef struct {\n"
+              "    foo_t *y;\n"
+              "} bar_t;\n"
+              "void f(bar_t *ptr) {\n"
+              "    if(ptr->y->x)\n"
+              "        if(ptr->y != nullptr) {}\n"
+              "}\n");
+        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());
+    }
+
     void nullpointer_addressOf() { // address of
         check("void f() {\n"
               "  struct X *x = 0;\n"