From d2f546c68c5ca8509e743c23d67883c2e623fe10 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Sat, 25 May 2024 23:29:39 +0200 Subject: [PATCH 1/5] Fix #11311 Do not search for null pointer in dead code --- lib/checknullpointer.cpp | 78 ++++++++++++++++++++++++++-------------- lib/checknullpointer.h | 2 +- test/testnullpointer.cpp | 42 ++++++++++++++++++++++ 3 files changed, 95 insertions(+), 27 deletions(-) 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" From 9e7c76dc90e573e58eb7886656eee663f5c8f200 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 3 Jun 2024 09:44:01 +0200 Subject: [PATCH 2/5] fixup! Fix #11311 Do not search for null pointer in dead code --- Makefile | 2 +- oss-fuzz/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f6b520e0671..08b0ab14b7e 100644 --- a/Makefile +++ b/Makefile @@ -533,7 +533,7 @@ $(libcppdir)/checkleakautovar.o: lib/checkleakautovar.cpp lib/addoninfo.h lib/as $(libcppdir)/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkmemoryleak.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp -$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h +$(libcppdir)/checknullpointer.o: lib/checknullpointer.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checknullpointer.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/findtoken.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp $(libcppdir)/checkother.o: lib/checkother.cpp lib/addoninfo.h lib/astutils.h lib/check.h lib/checkother.h lib/config.h lib/errortypes.h lib/fwdanalysis.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index 5d4a77403a4..e599093014f 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -212,7 +212,7 @@ $(libcppdir)/checkleakautovar.o: ../lib/checkleakautovar.cpp ../lib/addoninfo.h $(libcppdir)/checkmemoryleak.o: ../lib/checkmemoryleak.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkmemoryleak.h ../lib/color.h ../lib/config.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checkmemoryleak.cpp -$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h +$(libcppdir)/checknullpointer.o: ../lib/checknullpointer.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checknullpointer.h ../lib/color.h ../lib/config.h ../lib/ctu.h ../lib/errorlogger.h ../lib/errortypes.h ../lib/findtoken.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/checknullpointer.cpp $(libcppdir)/checkother.o: ../lib/checkother.cpp ../lib/addoninfo.h ../lib/astutils.h ../lib/check.h ../lib/checkother.h ../lib/config.h ../lib/errortypes.h ../lib/fwdanalysis.h ../lib/library.h ../lib/mathlib.h ../lib/platform.h ../lib/settings.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/suppressions.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenize.h ../lib/tokenlist.h ../lib/utils.h ../lib/valueflow.h ../lib/vfvalue.h From d56945f2ec79aded6c50387f290a2543b4433269 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Tue, 11 Jun 2024 16:39:14 +0200 Subject: [PATCH 3/5] fixup! fixup! Fix #11311 Do not search for null pointer in dead code --- lib/checknullpointer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 513bc710708..15877f8b3b7 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -320,13 +320,13 @@ void CheckNullPointer::nullPointerByDeRefAndCheck() if (!printInconclusive && value->isInconclusive()) return false; + if (isPointerUnevaluated(tok)) + return false; + return true; }; std::vector tokens = findTokensSkipDeadCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred); for (const Token *tok : tokens) { - if (isPointerUnevaluated(tok)) - continue; - const ValueFlow::Value *value = tok->getValue(0); // Pointer dereference. From 1b5cbc73264a8688a0580427e2c370d9902abda9 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Thu, 13 Jun 2024 23:45:38 +0200 Subject: [PATCH 4/5] fixup! fixup! fixup! Fix #11311 Do not search for null pointer in dead code --- lib/checknullpointer.cpp | 20 +--------------- lib/findtoken.h | 52 ++++++++++++++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 15877f8b3b7..cd55f8c2961 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -280,21 +280,6 @@ static bool isNullablePointer(const Token* tok) return false; } -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)); @@ -320,12 +305,9 @@ void CheckNullPointer::nullPointerByDeRefAndCheck() if (!printInconclusive && value->isInconclusive()) return false; - if (isPointerUnevaluated(tok)) - return false; - return true; }; - std::vector tokens = findTokensSkipDeadCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred); + std::vector tokens = findTokensSkipDeadAndUnevaluatedCode(mSettings->library, scope->bodyStart, scope->bodyEnd, pred); for (const Token *tok : tokens) { const ValueFlow::Value *value = tok->getValue(0); diff --git a/lib/findtoken.h b/lib/findtoken.h index 647cbda7746..300dd5a791f 100644 --- a/lib/findtoken.h +++ b/lib/findtoken.h @@ -84,7 +84,8 @@ bool findTokensSkipDeadCodeImpl(const Library& library, const Token* end, const Predicate& pred, Found found, - const Evaluate& evaluate) + const Evaluate& evaluate, + bool skipUnevaluated) { for (T* tok = start; precedes(tok, end); tok = tok->next()) { if (pred(tok)) { @@ -98,7 +99,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library, auto result = evaluate(condTok); if (result.empty()) continue; - if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate)) + if (findTokensSkipDeadCodeImpl(library, tok->next(), tok->linkAt(1), pred, found, evaluate, skipUnevaluated)) return true; T* thenStart = tok->linkAt(1)->next(); T* elseStart = nullptr; @@ -108,7 +109,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library, int r = result.front(); if (r == 0) { if (elseStart) { - if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate)) + if (findTokensSkipDeadCodeImpl(library, elseStart, elseStart->link(), pred, found, evaluate, skipUnevaluated)) return true; if (isReturnScope(elseStart->link(), library)) return true; @@ -117,7 +118,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library, tok = thenStart->link(); } } else { - if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate)) + if (findTokensSkipDeadCodeImpl(library, thenStart, thenStart->link(), pred, found, evaluate, skipUnevaluated)) return true; if (isReturnScope(thenStart->link(), library)) return true; @@ -137,7 +138,7 @@ bool findTokensSkipDeadCodeImpl(const Library& library, if (!cond) { next = colon; } else { - if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate)) + if (findTokensSkipDeadCodeImpl(library, tok->astParent()->next(), colon, pred, found, evaluate, skipUnevaluated)) return true; next = nextAfterAstRightmostLeaf(colon); } @@ -164,6 +165,12 @@ bool findTokensSkipDeadCodeImpl(const Library& library, else tok = afterCapture; } + if (skipUnevaluated && isUnevaluated(tok)) { + T *next = tok->linkAt(1); + if (!next) + continue; + tok = next; + } } return false; } @@ -185,7 +192,8 @@ std::vector findTokensSkipDeadCode(const Library& library, result.push_back(tok); return false; }, - evaluate); + evaluate, + false); return result; } @@ -195,6 +203,35 @@ std::vector findTokensSkipDeadCode(const Library& library, T* start, const T return findTokensSkipDeadCode(library, start, end, pred, &evaluateKnownValues); } +template )> +std::vector findTokensSkipDeadAndUnevaluatedCode(const Library& library, + T* start, + const Token* end, + const Predicate& pred, + const Evaluate& evaluate) +{ + std::vector result; + (void)findTokensSkipDeadCodeImpl( + library, + start, + end, + pred, + [&](T* tok) { + result.push_back(tok); + return false; + }, + evaluate, + true); + return result; +} + +template )> +std::vector findTokensSkipDeadAndUnevaluatedCode(const Library& library, T* start, const Token* end, const Predicate& pred) +{ + return findTokensSkipDeadAndUnevaluatedCode(library, start, end, pred, &evaluateKnownValues); +} + + template )> T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, const Predicate& pred, const Evaluate& evaluate) { @@ -208,7 +245,8 @@ T* findTokenSkipDeadCode(const Library& library, T* start, const Token* end, con result = tok; return true; }, - evaluate); + evaluate, + false); return result; } From f0eb67b7fe694d1d9cf655c54b5bf119b1b3da55 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Fri, 14 Jun 2024 09:07:07 +0200 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! Fix #11311 Do not search for null pointer in dead code --- lib/findtoken.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/findtoken.h b/lib/findtoken.h index 300dd5a791f..251eacf4fa3 100644 --- a/lib/findtoken.h +++ b/lib/findtoken.h @@ -205,10 +205,10 @@ std::vector findTokensSkipDeadCode(const Library& library, T* start, const T template )> std::vector findTokensSkipDeadAndUnevaluatedCode(const Library& library, - T* start, - const Token* end, - const Predicate& pred, - const Evaluate& evaluate) + T* start, + const Token* end, + const Predicate& pred, + const Evaluate& evaluate) { std::vector result; (void)findTokensSkipDeadCodeImpl(