From 130d1abbce2079137fd5dc9e5c59b5ee5987d32c Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Fri, 15 Oct 2021 03:57:40 -0500
Subject: [PATCH] Fix 10210: FN: nullPointerRedundantCheck regression in member
 function (#3512)

---
 lib/astutils.cpp         |  3 ++-
 lib/valueflow.cpp        |  3 ++-
 test/testnullpointer.cpp | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index 0e39c77b3..503bff984 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -2291,7 +2291,8 @@ bool isVariablesChanged(const Token* start,
 
 bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp)
 {
-    if (Token::Match(tok->previous(), "%name% (")) {
+    if ((Token::Match(tok->previous(), "%name% (") && !Token::simpleMatch(tok->astOperand1(), ".")) ||
+        Token::Match(tok->tokAt(-3), "this . %name% (")) {
         if (tok->previous()->function()) {
             return (!tok->previous()->function()->isConst());
         } else if (!tok->previous()->isKeyword()) {
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index afb04ab52..a86e8de5b 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2333,7 +2333,8 @@ struct ValueFlowAnalyzer : Analyzer {
             return isThisModified(tok);
 
         // bailout: global non-const variables
-        if (isGlobal() && Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) {
+        if (isGlobal() && !dependsOnThis() && Token::Match(tok, "%name% (") &&
+            !Token::simpleMatch(tok->linkAt(1), ") {")) {
             if (tok->function()) {
                 if (!tok->function()->isConstexpr() && !isConstFunctionCall(tok, getSettings()->library))
                     return Action::Invalid;
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index ce26a2046..8716801c6 100644
--- a/test/testnullpointer.cpp
+++ b/test/testnullpointer.cpp
@@ -124,6 +124,7 @@ private:
         TEST_CASE(nullpointer82); // #10331
         TEST_CASE(nullpointer83); // #9870
         TEST_CASE(nullpointer84); // #9873
+        TEST_CASE(nullpointer85); // #10210
         TEST_CASE(nullpointer_addressOf); // address of
         TEST_CASE(nullpointerSwitch); // #2626
         TEST_CASE(nullpointer_cast); // #4692
@@ -2525,6 +2526,27 @@ private:
             errout.str());
     }
 
+    void nullpointer85() // #10210
+    {
+        check("struct MyStruct {\n"
+              "  int GetId() const {\n"
+              "      int id = 0;\n"
+              "      int page = m_notebook->GetSelection();\n"
+              "      if (m_notebook && (m_notebook->GetPageCount() > 0))\n"
+              "        id = page;\n"
+              "      return id;\n"
+              "  }\n"
+              "  wxNoteBook *m_notebook = nullptr;\n"
+              "};\n"
+              "int f() {\n"
+              "  const MyStruct &s = Get();\n"
+              "  return s.GetId();\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:5] -> [test.cpp:4]: (warning) Either the condition 'm_notebook' is redundant or there is possible null pointer dereference: m_notebook.\n",
+            errout.str());
+    }
+
     void nullpointer_addressOf() { // address of
         check("void f() {\n"
               "  struct X *x = 0;\n"