diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 27b580ef4..bd617f7c4 100644 --- a/lib/checknullpointer.cpp +++ b/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); } + + diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index 1a7d2bb58..1bac446d0 100644 --- a/lib/checknullpointer.h +++ b/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; }; /// @} //--------------------------------------------------------------------------- diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index d61c78dfc..0fe8e2911 100644 --- a/test/testnullpointer.cpp +++ b/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)