diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index a1e5a9aa2..8d92d9104 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -1194,6 +1194,26 @@ void CheckNullPointer::nullConstantDereference() } } +/** +* @brief If tok is a function call that passes in a pointer such that +* the pointer may be modified, this function will remove that +* pointer from pointerArgs. +*/ +void CheckNullPointer::removeAssignedVarFromSet(const Token* tok, std::set& pointerArgs) +{ + // Common functions that are known NOT to modify their pointer argument + const char safeFunctions[] = "printf|sprintf|fprintf|vprintf"; + + // If a pointer's address is passed into a function, stop considering it + if (Token::Match(tok->previous(), "[;{}] %var% (")) { + const Token* endParen = tok->next()->link(); + for (const Token* tok2 = tok->next(); tok2 != endParen; tok2 = tok2->next()) { + if (tok2->isName() && tok2->varId() > 0 && !Token::Match(tok, safeFunctions)) { + pointerArgs.erase(tok2->varId()); + } + } + } +} /** * @brief Does one part of the check for nullPointer(). @@ -1223,8 +1243,8 @@ void CheckNullPointer::nullPointerDefaultArgument() // Report an error if any of the default-NULL arguments are dereferenced if (!pointerArgs.empty()) { - bool unknown = _settings->inconclusive; for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { + // If we encounter a possible NULL-pointer check, skip over its body if (Token::simpleMatch(tok, "if ( ")) { bool dependsOnPointer = false; @@ -1241,31 +1261,53 @@ void CheckNullPointer::nullPointerDefaultArgument() // pointer check for the pointers referenced in its condition const Token *endOfIf = startOfIfBlock->link(); bool isExitOrReturn = - Token::findmatch(startOfIfBlock, "exit|return", endOfIf) != NULL; + Token::findmatch(startOfIfBlock, "exit|return|throw", endOfIf) != NULL; - for (const Token *tok2 = tok->next(); tok2 != endOfCondition; tok2 = tok2->next()) { - if (tok2->isName() && tok2->varId() > 0 && - pointerArgs.count(tok2->varId()) > 0) { - - // If the if() depends on a pointer and may return, stop - // considering that pointer because it may be a NULL-pointer - // check that returns if the pointer is NULL. + if (Token::Match(tok, "if ( %var% == 0 )")) { + const unsigned int var = tok->tokAt(2)->varId(); + if (var > 0 && pointerArgs.count(var) > 0) { if (isExitOrReturn) - pointerArgs.erase(tok2->varId()); + pointerArgs.erase(var); else dependsOnPointer = true; } + } else { + for (const Token *tok2 = tok->next(); tok2 != endOfCondition; tok2 = tok2->next()) { + if (tok2->isName() && tok2->varId() > 0 && + pointerArgs.count(tok2->varId()) > 0) { + + // If the if() depends on a pointer and may return, stop + // considering that pointer because it may be a NULL-pointer + // check that returns if the pointer is NULL. + if (isExitOrReturn) + pointerArgs.erase(tok2->varId()); + else + dependsOnPointer = true; + } + } } + if (dependsOnPointer && endOfIf) { for (; tok != endOfIf; tok = tok->next()) { // If a pointer is assigned a new value, stop considering it. if (Token::Match(tok, "%var% =")) pointerArgs.erase(tok->varId()); + else + removeAssignedVarFromSet(tok, pointerArgs); } continue; } } + // If there is a noreturn function (e.g. exit()), stop considering the rest of + // this function. + bool unknown = false; + if (Token::Match(tok, "return|throw|exit") || + (_tokenizer->IsScopeNoReturn(tok, &unknown) && !unknown)) + break; + + removeAssignedVarFromSet(tok, pointerArgs); + if (tok->varId() == 0 || pointerArgs.count(tok->varId()) == 0) continue; @@ -1276,7 +1318,7 @@ void CheckNullPointer::nullPointerDefaultArgument() // If a pointer dereference is preceded by an && or ||, // they serve as a sequence point so the dereference // may not be executed. - if (isPointerDeRef(tok, unknown) && + if (isPointerDeRef(tok, unknown) && !unknown && tok->strAt(-1) != "&&" && tok->strAt(-1) != "||" && tok->strAt(-2) != "&&" && tok->strAt(-2) != "||") nullPointerDefaultArgError(tok, tok->str()); diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index 10a877e82..0fbfd0a2b 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -152,6 +152,11 @@ private: */ void nullPointerDefaultArgument(); + /** + * @brief Removes any variable that may be assigned from pointerArgs. + */ + void removeAssignedVarFromSet(const Token* tok, std::set& pointerArgs); + /** * @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 62676693a..ddb68aa75 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2110,6 +2110,13 @@ private: "}"); 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)\n" + " return;\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + check("void f(char a, int *p = 0) {\n" " *p = 0;\n" "}"); @@ -2195,6 +2202,53 @@ private: " return !p || *p;\n" "}"); ASSERT_EQUALS("", errout.str()); + + + // bar may initialize p but be can't know for sure without knowing + // if p is passed in by reference and is modified by bar() + check("void f(int *p = 0) {\n" + " bar(p);\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int *p = 0) {\n" + " printf(\"%d\", p);\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()); + + // The init() function may or may not initialize p, but since the address + // of p is passed in, it's a good bet that p may be modified and + // so we should not report an error. + check("void f(int *p = 0) {\n" + " init(&p);\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void init(int* &g);\n" + "void f(int *p = 0) {\n" + " init(p);\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int *p = 0) {\n" + " if (p == 0) {\n" + " init(&p);\n" + " }\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f(int *p = 0) {\n" + " if (p == 0) {\n" + " throw SomeException;\n" + " }\n" + " *p = 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); }