Fixed #3443 (false positives: possible null pointer dereference (calling unknown function))

lib/checknullpointer.cpp

@@ -288,6 +288,44 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown)
     return false;
 }
 
+
+// check if function can assign pointer
+bool CheckNullPointer::CanFunctionAssignPointer(const Token *functiontoken, unsigned int varid) const
+{
+    if (Token::Match(functiontoken, "if|while|sizeof"))
+        return false;
+
+    int argumentNumber = 0;
+    for (const Token *arg = functiontoken->tokAt(2); arg; arg = arg->nextArgument()) {
+        ++argumentNumber;
+        if (Token::Match(arg, "%varid% [,)]", varid)) {
+            const Token *ftok = _tokenizer->getFunctionTokenByName(functiontoken->str().c_str());
+            if (!Token::Match(ftok, "%type% (")) {
+                // assume that the function might assign the pointer
+                return true;
+            }
+
+            ftok = ftok->tokAt(2);
+            while (ftok && argumentNumber > 1) {
+                ftok = ftok->nextArgument();
+                --argumentNumber;
+            }
+
+            // check if it's a pointer parameter..
+            while (ftok && ftok->isName())
+                ftok = ftok->next();
+            if (Token::Match(ftok, "* *| const| %var% [,)]"))
+                // parameter is passed by value
+                return false;
+        }
+    }
+
+    // pointer is not passed
+    return false;
+}
+
+
+
 void CheckNullPointer::nullPointerAfterLoop()
 {
     const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
@@ -584,6 +622,10 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec()
             else if (tok2->str() == ":")
                 break;
 
+            // function call..
+            else if (Token::Match(tok2, "[;{}] %var% (") && CanFunctionAssignPointer(tok2->next(), varid1))
+                break;
+
             // Reassignment of the struct
             else if (tok2->varId() == varid1) {
                 if (tok2->next()->str() == "=") {
@@ -690,6 +732,12 @@ void CheckNullPointer::nullPointerByDeRefAndChec()
                         }
                     }
 
+                    // Passing pointer as parameter..
+                    if (Token::Match(tok2, "[;{}] %type% (")) {
+                        if (CanFunctionAssignPointer(tok2->next(), varid))
+                            break;
+                    }
+
                     // calling unknown function => it might initialize the pointer
                     if (!(var->isLocal() || var->isArgument()))
                         break;
@@ -1233,3 +1281,5 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
         reportError(tok, Severity::error, "nullPointer", errmsg);
 }
 
+
+

lib/checknullpointer.h

@@ -151,6 +151,12 @@ private:
      * -# dereference pointer
      */
     void nullPointerConditionalAssignment();
+
+    /**
+     * @brief Investigate if function call can make pointer null. If
+     * the pointer is passed by value it can't be made a null pointer.
+     */
+    bool CanFunctionAssignPointer(const Token *functiontoken, unsigned int varid) const;
 };
 /// @}
 //---------------------------------------------------------------------------

test/testnullpointer.cpp

@@ -60,6 +60,8 @@ private:
         TEST_CASE(nullpointer_in_for_loop);
         TEST_CASE(nullpointerDelete);
         TEST_CASE(nullpointerExit);
+
+        TEST_CASE(functioncall);
     }
 
     void check(const char code[], bool inconclusive = false, bool cpp11 = false) {
@@ -1716,6 +1718,70 @@ private:
               "}\n", true);
         ASSERT_EQUALS("", errout.str());
     }
+
+    void functioncall() {    // #3443 - function calls
+        // dereference pointer and then check if it's null
+        {
+            // function not seen
+            check("void f(int *p) {\n"
+                  "    *p = 0;\n"
+                  "    foo(p);\n"
+                  "    if (p) { }\n"
+                  "}");
+            ASSERT_EQUALS("", errout.str());
+
+            // function seen (taking pointer parameter)
+            check("void foo(int *p) { }\n"
+                  "\n"
+                  "void f(int *p) {\n"
+                  "    *p = 0;\n"
+                  "    foo(p);\n"
+                  "    if (p) { }\n"
+                  "}");
+            ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 6\n", errout.str());
+
+            // function implementation not seen
+            check("void foo(int *p);\n"
+                  "\n"
+                  "void f(int *p) {\n"
+                  "    *p = 0;\n"
+                  "    foo(p);\n"
+                  "    if (p) { }\n"
+                  "}");
+            TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: p - otherwise it is redundant to check if p is null at line 6\n", "", errout.str());
+        }
+
+        // dereference struct pointer and then check if it's null
+        {
+            // function not seen
+            check("void f(struct ABC *abc) {\n"
+                  "    abc->a = 0;\n"
+                  "    foo(abc);\n"
+                  "    if (abc) { }\n"
+                  "}");
+            ASSERT_EQUALS("", errout.str());
+
+            // function seen (taking pointer parameter)
+            check("void foo(struct ABC *abc) { }\n"
+                  "\n"
+                  "void f(struct ABC *abc) {\n"
+                  "    abc->a = 0;\n"
+                  "    foo(abc);\n"
+                  "    if (abc) { }\n"
+                  "}");
+            ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 6\n", errout.str());
+
+            // function implementation not seen
+            check("void foo(struct ABC *abc);\n"
+                  "\n"
+                  "void f(struct ABC *abc) {\n"
+                  "    abc->a = 0;\n"
+                  "    foo(abc);\n"
+                  "    if (abc) { }\n"
+                  "}");
+            TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: abc - otherwise it is redundant to check if abc is null at line 6\n", "", errout.str());
+        }
+    }
 };
 
 REGISTER_TEST(TestNullPointer)