Fix 10210: FN: nullPointerRedundantCheck regression in member function (danmar#3512)

pfultz2 · web-flow · commit 130d1abbce20 · 2021-10-15T10:57:40.000+02:00
diff --git 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
@@ -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
@@ -124,6 +124,7 @@ class TestNullPointer : public TestFixture {
         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 @@ class TestNullPointer : public TestFixture {
             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"

Original file line number	Diff line number	Diff line change
`@@ -2291,7 +2291,8 @@ bool isVariablesChanged(const Token* start,`
`2291`	`2291`
`2292`	`2292`	`bool isThisChanged(const Token* tok, int indirect, const Settings* settings, bool cpp)`
`2293`	`2293`	`{`
`2294`		`- if (Token::Match(tok->previous(), "%name% (")) {`
	`2294`	`+ if ((Token::Match(tok->previous(), "%name% (") && !Token::simpleMatch(tok->astOperand1(), ".")) \|\|`
	`2295`	`+ Token::Match(tok->tokAt(-3), "this . %name% (")) {`
`2295`	`2296`	`if (tok->previous()->function()) {`
`2296`	`2297`	`return (!tok->previous()->function()->isConst());`
`2297`	`2298`	`} else if (!tok->previous()->isKeyword()) {`