From 095824373a6ccb16495d2ae63f0a49ffb030475f Mon Sep 17 00:00:00 2001 From: Zachary Blair Date: Tue, 20 Nov 2012 23:56:17 -0800 Subject: [PATCH] Fixed #3302 (new check: nullpointer dereference) --- lib/checknullpointer.cpp | 64 ++++++++++++++++++++++++++++++++++++++++ lib/checknullpointer.h | 9 +++++- test/testnullpointer.cpp | 63 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index e18305d83..b954b23e3 100644 --- a/lib/checknullpointer.cpp +++ b/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::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 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); +} diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index fc43bbb19..10aa077c2 100644 --- a/lib/checknullpointer.h +++ b/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. diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 3ec32a7f3..f73325554 100644 --- a/test/testnullpointer.cpp +++ b/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"