From d18f5d8709674fd7c5d48706a735259faac63ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 29 Dec 2018 09:26:57 +0100 Subject: [PATCH] CTU: Reuse CheckNullPointer::isPointerDeRef in the nullpointer isUnsafeUsage --- lib/checkleakautovar.cpp | 2 +- lib/checknullpointer.cpp | 52 ++++++++++++++++++++++------------------ lib/checknullpointer.h | 4 +++- lib/checkstl.cpp | 2 +- lib/checkuninitvar.cpp | 2 +- test/testnullpointer.cpp | 4 ++-- 6 files changed, 37 insertions(+), 29 deletions(-) diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index 08b9550f1..572840690 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -672,7 +672,7 @@ const Token * CheckLeakAutoVar::checkTokenInsideExpression(const Token * const t const std::map::const_iterator var = varInfo->alloctype.find(tok->varId()); if (var != varInfo->alloctype.end()) { bool unknown = false; - if (var->second.status == VarInfo::DEALLOC && CheckNullPointer::isPointerDeRef(tok, unknown) && !unknown) { + if (var->second.status == VarInfo::DEALLOC && CheckNullPointer::isPointerDeRef(tok, unknown, mSettings) && !unknown) { deallocUseError(tok, tok->str()); } else if (Token::simpleMatch(tok->tokAt(-2), "= &")) { varInfo->erase(tok->varId()); diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 1f687acf2..19d5850d8 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -161,15 +161,37 @@ namespace { * @param unknown it is not known if there is a pointer dereference (could be reported as a debug message) * @return true => there is a dereference */ -bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown) +bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown) const +{ + return isPointerDeRef(tok, unknown, mSettings); +} + +bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Settings *settings) { unknown = false; + // Is pointer used as function parameter? + if (Token::Match(tok->previous(), "[(,] %name% [,)]") && settings) { + const Token *ftok = tok->previous(); + while (ftok && ftok->str() != "(") { + if (ftok->str() == ")") + ftok = ftok->link(); + ftok = ftok->previous(); + } + if (ftok && ftok->previous()) { + std::list varlist; + parseFunctionCall(*ftok->previous(), varlist, &settings->library); + if (std::find(varlist.begin(), varlist.end(), tok) != varlist.end()) { + return true; + } + } + } + const Token* parent = tok->astParent(); if (!parent) return false; if (parent->str() == "." && parent->astOperand2() == tok) - return isPointerDeRef(parent, unknown); + return isPointerDeRef(parent, unknown, settings); const bool firstOperand = parent->astOperand1() == tok; while (parent->str() == "(" && (parent->astOperand2() == nullptr && parent->strAt(1) != ")")) { // Skip over casts parent = parent->astParent(); @@ -339,24 +361,6 @@ void CheckNullPointer::nullPointerByDeRefAndChec() if (!printInconclusive && value->isInconclusive()) continue; - // Is pointer used as function parameter? - if (Token::Match(tok->previous(), "[(,] %name% [,)]")) { - const Token *ftok = tok->previous(); - while (ftok && ftok->str() != "(") { - if (ftok->str() == ")") - ftok = ftok->link(); - ftok = ftok->previous(); - } - if (!ftok || !ftok->previous()) - continue; - std::list varlist; - parseFunctionCall(*ftok->previous(), varlist, &mSettings->library); - if (std::find(varlist.begin(), varlist.end(), tok) != varlist.end()) { - nullPointerError(tok, tok->str(), value, value->isInconclusive()); - } - continue; - } - // Pointer dereference. bool unknown = false; if (!isPointerDeRef(tok,unknown)) { @@ -598,13 +602,15 @@ std::string CheckNullPointer::MyFileInfo::toString() const static bool isUnsafeUsage(const Check *check, const Token *vartok) { - (void)check; - return Token::Match(vartok->astParent(), "*|["); + const CheckNullPointer *checkNullPointer = dynamic_cast(check); + bool unknown = false; + return checkNullPointer && checkNullPointer->isPointerDeRef(vartok, unknown); } Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, nullptr, ::isUnsafeUsage); + CheckNullPointer check(tokenizer, settings, nullptr); + const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, &check, ::isUnsafeUsage); if (unsafeUsage.empty()) return nullptr; diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index 66f1dd478..6d4b59c83 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -86,7 +86,9 @@ public: * @param unknown it is not known if there is a pointer dereference (could be reported as a debug message) * @return true => there is a dereference */ - static bool isPointerDeRef(const Token *tok, bool &unknown); + bool isPointerDeRef(const Token *tok, bool &unknown) const; + + static bool isPointerDeRef(const Token *tok, bool &unknown, const Settings *settings); /** @brief possible null pointer dereference */ void nullPointer(); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index d9a111677..b19c299ff 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -962,7 +962,7 @@ void CheckStl::pushback() // Using invalid pointer.. if (invalidPointer && tok2->varId() == pointerId) { bool unknown = false; - if (CheckNullPointer::isPointerDeRef(tok2, unknown)) + if (CheckNullPointer::isPointerDeRef(tok2, unknown, mSettings)) invalidPointerError(tok2, function->str(), tok2->str()); break; } diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index c022a9312..81b17fb0c 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -985,7 +985,7 @@ bool CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc al } bool unknown = false; - if (pointer && alloc == NO_ALLOC && CheckNullPointer::isPointerDeRef(vartok, unknown)) { + if (pointer && alloc == NO_ALLOC && CheckNullPointer::isPointerDeRef(vartok, unknown, mSettings)) { // function parameter? bool functionParameter = false; if (Token::Match(vartok->tokAt(-2), "%name% (") || vartok->previous()->str() == ",") diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f0fffcda4..300aca001 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2073,12 +2073,12 @@ private: "}", true); ASSERT_EQUALS("[test.cpp:9]: (error) Null pointer dereference: p\n" "[test.cpp:10]: (error) Null pointer dereference: p\n" + "[test.cpp:11]: (error) Null pointer dereference: p\n" + "[test.cpp:12]: (warning, inconclusive) Possible null pointer dereference: p\n" "[test.cpp:3]: (error) Null pointer dereference\n" "[test.cpp:5]: (error) Null pointer dereference\n" "[test.cpp:7]: (error) Null pointer dereference\n" "[test.cpp:8]: (error) Null pointer dereference\n" - /*"[test.cpp:11]: (error) Possible null pointer dereference: p\n" - "[test.cpp:12]: (error) Possible null pointer dereference: p\n"*/ , errout.str()); check("void f(std::string s1) {\n"