Fixed #3302 (new check: nullpointer dereference)

lib/checknullpointer.cpp

@@ -1091,6 +1091,7 @@ void CheckNullPointer::nullPointer()
     nullPointerStructByDeRefAndChec();
     nullPointerByDeRefAndChec();
     nullPointerByCheckAndDeRef();
+    nullPointerDefaultArgument();
 }
 
 /** Dereferencing null constant (simplified token list) */
@@ -1171,6 +1172,64 @@ void CheckNullPointer::nullConstantDereference()
 }
 
 
+/**
+* @brief Does one part of the check for nullPointer().
+* -# default argument that sets a pointer to 0
+* -# dereference pointer
+*/
+void CheckNullPointer::nullPointerDefaultArgument()
+{
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
+        if (i->type != Scope::eFunction || !i->classStart || !i->function)
+            continue;
+
+        // Scan the argument list for default arguments that are pointers and
+        // which default to a NULL pointer if no argument is specified.
+        std::set<unsigned int> pointerArgs;
+        for (const Token *tok = i->function->arg; tok != i->function->arg->link(); tok = tok->next()) {
+
+            if (Token::Match(tok, "%var% = 0 ,|)") && tok->varId() != 0) {
+                const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId());
+                if (var && var->isPointer())
+                    pointerArgs.insert(tok->varId());
+            }
+        }
+
+        // Report an error if any of the default-NULL arguments are dereferenced
+        if (!pointerArgs.empty()) {
+            bool unknown = _settings->inconclusive;
+            for (const Token *tok = i->classStart; tok != i->classEnd; tok = tok->next()) {
+                // If we encounter a possible NULL-pointer check, skip over its body
+                if (Token::Match(tok, "if ( "))  {
+                    bool dependsOnPointer = false;
+                    const Token *endOfCondition = tok->next()->link();
+                    for (const Token *tok2 = tok->next(); tok2 != endOfCondition; tok2 = tok2->next()) {
+                        if (tok2->isName() && tok2->varId() > 0 && pointerArgs.count(tok2->varId()) > 0) {
+                            dependsOnPointer = true;
+                        }
+                    }
+                    if (dependsOnPointer && Token::Match(endOfCondition, ") {")) {
+                        tok = endOfCondition->next()->link();
+                        continue;
+                    }
+                }
+
+                if (tok->varId() == 0 || pointerArgs.count(tok->varId()) == 0)
+                    continue;
+
+                // If a pointer is assigned a new value, stop considering it.
+                if (Token::Match(tok, "%var% = %any%"))
+                    pointerArgs.erase(tok->varId());
+
+                if (isPointerDeRef(tok, unknown, symbolDatabase))
+                    nullPointerDefaultArgError(tok, tok->str());
+            }
+        }
+    }
+}
+
 /// @addtogroup Checks
 /// @{
 
@@ -1397,3 +1456,8 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
     const std::string errmsg("Possible null pointer dereference: " + varname + " - otherwise it is redundant to check it against null.");
     reportError(callstack, Severity::error, "nullPointer", errmsg, inconclusive);
 }
+
+void CheckNullPointer::nullPointerDefaultArgError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::warning, "nullPointer", "Possible null pointer dereference if the default parameter value is used: " + varname);
+}

lib/checknullpointer.h

@@ -100,7 +100,7 @@ public:
     void nullPointerError(const Token *tok);  // variable name unknown / doesn't exist
     void nullPointerError(const Token *tok, const std::string &varname);
     void nullPointerError(const Token *tok, const std::string &varname, const Token* nullcheck, bool inconclusive = false);
-
+    void nullPointerDefaultArgError(const Token *tok, const std::string &varname);
 private:
 
     /** Get error messages. Used by --errorlist */
@@ -146,6 +146,13 @@ private:
      */
     void nullPointerConditionalAssignment();
 
+    /**
+     * @brief Does one part of the check for nullPointer().
+     * -# default argument that sets a pointer to 0
+     * -# dereference pointer
+     */
+    void nullPointerDefaultArgument();
+
     /**
      * @brief Investigate if function call can make pointer null. If
      * the pointer is passed by value it can't be made a null pointer.

test/testnullpointer.cpp

@@ -72,8 +72,8 @@ private:
         TEST_CASE(nullpointerStdString);
         TEST_CASE(nullpointerStdStream);
         TEST_CASE(functioncall);
-
         TEST_CASE(crash1);
+        TEST_CASE(functioncallDefaultArguments);
     }
 
     void check(const char code[], bool inconclusive = false, const char filename[] = "test.cpp") {
@@ -2141,6 +2141,67 @@ private:
         }
     }
 
+
+    void functioncallDefaultArguments() {
+
+        check("void f(int *p = 0) {\n"
+              "    *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+
+        check("void f(char a, int *p = 0) {\n"
+              "    *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    printf(\"p = %d\", *p);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    printf(\"p[1] = %d\", p[1]);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    if (p != 0 && bar())\n"
+              "      *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int *p) {\n"
+              "    *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    if (p != 0)\n"
+              "      *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    if (a != 0)\n"
+              "      *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    p = a;\n"
+              "    *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f(int *p = 0) {\n"
+              "    p += a;\n"
+              "    *p = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+    }
+
+
     void crash1() {
         check("int f() {\n"
               "    return if\n"