diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 6e034738260..513bc710708 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -24,6 +24,7 @@ #include "ctu.h" #include "errorlogger.h" #include "errortypes.h" +#include "findtoken.h" #include "library.h" #include "mathlib.h" #include "settings.h" @@ -279,47 +280,72 @@ static bool isNullablePointer(const Token* tok) return false; } -void CheckNullPointer::nullPointerByDeRefAndChec() +static bool isPointerUnevaluated(const Token *tok) +{ + if (isUnevaluated(tok)) + return true; + + while (tok) { + if (isUnevaluated(tok->previous())) + return true; + + tok = tok->astParent(); + } + return false; +} + + +void CheckNullPointer::nullPointerByDeRefAndCheck() { const bool printInconclusive = (mSettings->certainty.isEnabled(Certainty::inconclusive)); - for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) { - if (isUnevaluated(tok)) { - tok = tok->linkAt(1); - continue; - } + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope * scope : symbolDatabase->functionScopes) { + auto pred = [printInconclusive](const Token* tok) -> bool { + if (!tok) + return false; - if (Token::Match(tok, "%num%|%char%|%str%")) - continue; + if (Token::Match(tok, "%num%|%char%|%str%")) + return false; - if (!isNullablePointer(tok) || - (tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning - continue; + if (!isNullablePointer(tok) || + (tok->str() == "." && isNullablePointer(tok->astOperand2()) && tok->astOperand2()->getValue(0))) // avoid duplicate warning + return false; - // Can pointer be NULL? - const ValueFlow::Value *value = tok->getValue(0); - if (!value) - continue; + // Can pointer be NULL? + const ValueFlow::Value *value = tok->getValue(0); + if (!value) + return false; - if (!printInconclusive && value->isInconclusive()) - continue; + if (!printInconclusive && value->isInconclusive()) + return false; - // Pointer dereference. - bool unknown = false; - if (!isPointerDeRef(tok,unknown)) { - if (unknown) - nullPointerError(tok, tok->expressionString(), value, true); - continue; - } + return true; + }; + std::vector tokens = findTokensSkipDeadCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred); + for (const Token *tok : tokens) { + if (isPointerUnevaluated(tok)) + continue; - nullPointerError(tok, tok->expressionString(), value, value->isInconclusive()); + const ValueFlow::Value *value = tok->getValue(0); + + // Pointer dereference. + bool unknown = false; + if (!isPointerDeRef(tok, unknown)) { + if (unknown) + nullPointerError(tok, tok->expressionString(), value, true); + continue; + } + + nullPointerError(tok, tok->expressionString(), value, value->isInconclusive()); + } } } void CheckNullPointer::nullPointer() { logChecker("CheckNullPointer::nullPointer"); - nullPointerByDeRefAndChec(); + nullPointerByDeRefAndCheck(); } namespace { diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h index f797a8f6d20..2b01c6b72a5 100644 --- a/lib/checknullpointer.h +++ b/lib/checknullpointer.h @@ -137,7 +137,7 @@ class CPPCHECKLIB CheckNullPointer : public Check { * @brief Does one part of the check for nullPointer(). * Dereferencing a pointer and then checking if it's NULL.. */ - void nullPointerByDeRefAndChec(); + void nullPointerByDeRefAndCheck(); /** undefined null pointer arithmetic */ void arithmetic(); diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 432b50fab9c..a9a2a82043d 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -158,6 +158,7 @@ class TestNullPointer : public TestFixture { TEST_CASE(nullpointer_in_typeid); TEST_CASE(nullpointer_in_alignof); // #11401 TEST_CASE(nullpointer_in_for_loop); + TEST_CASE(nullpointerDeadCode); // #11311 TEST_CASE(nullpointerDelete); TEST_CASE(nullpointerSubFunction); TEST_CASE(nullpointerExit); @@ -3657,6 +3658,47 @@ class TestNullPointer : public TestFixture { ASSERT_EQUALS("", errout_str()); } + void nullpointerDeadCode() { + // Ticket #11311 + check ("void f() {\n" + " if (0)\n" + " *(int *)0 = 1;\n" + " else\n" + " ;\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check ("void f() {\n" + " if (0)\n" + " *(int *)0 = 1;\n" + " else {\n" + " if (0)\n" + " *(int *)0 = 2;\n" + " else\n" + " ;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check ("void f() {\n" + " while(0)\n" + " *(int*)0 = 1;\n" + " do {\n" + " } while(0);\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check ("int f() {\n" + " return 0 ? *(int*)0 = 1 : 1;\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check ("int f() {\n" + " return 1 ? 1 : *(int*)0 = 1;\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + } + void nullpointerDelete() { check("void f() {\n" " K *k = getK();\n"