From 2ca2abdf0e4a7b59e81a4e47550b2f1449ffc4e6 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Mon, 4 Oct 2021 00:50:23 -0500 Subject: [PATCH] Remove duplicate uninit warnings (#3478) --- Makefile | 2 +- lib/checkuninitvar.cpp | 113 ++++++++++++++++++++++------------------ lib/forwardanalyzer.cpp | 16 +----- test/testuninitvar.cpp | 7 +-- 4 files changed, 67 insertions(+), 71 deletions(-) diff --git a/Makefile b/Makefile index d8f6441eb..8fc112a73 100644 --- a/Makefile +++ b/Makefile @@ -515,7 +515,7 @@ $(libcppdir)/errortypes.o: lib/errortypes.cpp lib/config.h lib/errortypes.h $(libcppdir)/exprengine.o: lib/exprengine.cpp lib/astutils.h lib/bughuntingchecks.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/exprengine.o $(libcppdir)/exprengine.cpp -$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/analyzer.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/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h +$(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/forwardanalyzer.o $(libcppdir)/forwardanalyzer.cpp $(libcppdir)/importproject.o: lib/importproject.cpp externals/picojson/picojson.h externals/tinyxml2/tinyxml2.h lib/astutils.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index c3c6f2ca0..56ec2e86d 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1508,57 +1508,70 @@ void CheckUninitVar::valueFlowUninit() { const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); - // check every executable scope - for (const Scope *scope : symbolDatabase->functionScopes) { - for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { - if (isSizeOfEtc(tok)) { - tok = tok->linkAt(1); - continue; + std::unordered_set ids; + for (bool subfunction : {false, true}) { + // check every executable scope + for (const Scope* scope : symbolDatabase->functionScopes) { + for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { + if (isSizeOfEtc(tok)) { + tok = tok->linkAt(1); + continue; + } + if (ids.count(tok->exprId()) > 0) + continue; + if (!tok->variable() && !tok->isUnaryOp("*")) + continue; + if (Token::Match(tok, "%name% (")) + continue; + const Token* parent = tok->astParent(); + while (Token::simpleMatch(parent, ".")) + parent = parent->astParent(); + if (parent && parent->isUnaryOp("&")) + continue; + if (isVoidCast(parent)) + continue; + auto v = std::find_if( + tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); + if (v == tok->values().end()) + continue; + if (v->tokvalue && ids.count(v->tokvalue->exprId()) > 0) + continue; + if (subfunction == (v->path == 0)) + continue; + if (v->isInconclusive()) + continue; + if (v->indirect > 1 || v->indirect < 0) + continue; + bool uninitderef = false; + if (tok->variable()) { + bool unknown; + const bool isarray = !tok->variable() || tok->variable()->isArray(); + const bool ispointer = astIsPointer(tok) && !isarray; + const bool deref = CheckNullPointer::isPointerDeRef(tok, unknown, mSettings); + if (ispointer && v->indirect == 1 && !deref) + continue; + if (isarray && !deref) + continue; + uninitderef = deref && v->indirect == 0; + const bool isleaf = isLeafDot(tok) || uninitderef; + if (Token::Match(tok->astParent(), ". %var%") && !isleaf) + continue; + } + if (!(Token::Match(tok->astParent(), ". %name% (") && uninitderef) && + isVariableChanged(tok, v->indirect, mSettings, mTokenizer->isCPP())) + continue; + bool inconclusive = false; + if (isVariableChangedByFunctionCall(tok, v->indirect, mSettings, &inconclusive) || inconclusive) + continue; + uninitvarError(tok, tok->expressionString(), v->errorPath); + ids.insert(tok->exprId()); + if (v->tokvalue) + ids.insert(v->tokvalue->exprId()); + const Token* nextTok = nextAfterAstRightmostLeaf(parent); + if (nextTok == scope->bodyEnd) + break; + tok = nextTok ? nextTok : tok; } - if (!tok->variable() && !tok->isUnaryOp("*")) - continue; - if (Token::Match(tok, "%name% (")) - continue; - const Token* parent = tok->astParent(); - while (Token::simpleMatch(parent, ".")) - parent = parent->astParent(); - if (parent && parent->isUnaryOp("&")) - continue; - if (isVoidCast(parent)) - continue; - auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); - if (v == tok->values().end()) - continue; - if (v->isInconclusive()) - continue; - if (v->indirect > 1 || v->indirect < 0) - continue; - bool uninitderef = false; - if (tok->variable()) { - bool unknown; - const bool isarray = !tok->variable() || tok->variable()->isArray(); - const bool ispointer = astIsPointer(tok) && !isarray; - const bool deref = CheckNullPointer::isPointerDeRef(tok, unknown, mSettings); - if (ispointer && v->indirect == 1 && !deref) - continue; - if (isarray && !deref) - continue; - uninitderef = deref && v->indirect == 0; - const bool isleaf = isLeafDot(tok) || uninitderef; - if (Token::Match(tok->astParent(), ". %var%") && !isleaf) - continue; - } - if (!(Token::Match(tok->astParent(), ". %name% (") && uninitderef) && - isVariableChanged(tok, v->indirect, mSettings, mTokenizer->isCPP())) - continue; - bool inconclusive = false; - if (isVariableChangedByFunctionCall(tok, v->indirect, mSettings, &inconclusive) || inconclusive) - continue; - uninitvarError(tok, tok->expressionString(), v->errorPath); - const Token* nextTok = nextAfterAstRightmostLeaf(parent); - if (nextTok == scope->bodyEnd) - break; - tok = nextTok ? nextTok : tok; } } } diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 76a362a6a..bdf648110 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -5,7 +5,6 @@ #include "symboldatabase.h" #include "token.h" #include "valueptr.h" -#include "checknullpointer.h" // UninitValue => isPointerDeref #include #include @@ -193,21 +192,8 @@ struct ForwardTraversal { Progress update(Token* tok) { Analyzer::Action action = analyzer->analyze(tok, Analyzer::Direction::Forward); actions |= action; - if (!action.isNone() && !analyzeOnly) { + if (!action.isNone() && !analyzeOnly) analyzer->update(tok, action, Analyzer::Direction::Forward); - - // uninit value => skip further analysis - auto v = std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isUninitValue)); - if (v != tok->values().end()) { - if (tok->valueType() && tok->valueType()->pointer > 0) { - bool unknown = false; - if (CheckNullPointer::isPointerDeRef(tok, unknown, settings)) - return Break(Analyzer::Terminate::Modified); - } - if (Token::Match(tok->astParent(), "[,(]") && !isSizeOfEtc(tok->astParent()->previous())) - return Break(Analyzer::Terminate::Modified); - } - } if (action.isInconclusive() && !analyzer->lowerToInconclusive()) return Break(Analyzer::Terminate::Inconclusive); if (action.isInvalid()) diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 993193a61..34224de0e 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -4563,8 +4563,7 @@ private: " arc << p;\n" // <- TODO initialization? " return *p;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: p\n" - "[test.cpp:4]: (error) Uninitialized variable: p\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: p\n", errout.str()); // #4320 valueFlowUninit("void f() {\n" @@ -4572,9 +4571,7 @@ private: " a << 1;\n" " return a;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: a\n" - "[test.cpp:4]: (error) Uninitialized variable: a\n", - errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: a\n", errout.str()); // #9750 valueFlowUninit("struct S {\n"