From 989d276dde17abbf0104b1720ad9707c29f8c3e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Fri, 3 Nov 2023 18:27:11 +0100 Subject: [PATCH] removed the need for `Check` object in `CTU::getUnsafeUsage()` callback / some CheckUninitVar cleanups (#5610) The `Check` objects were just created for that purpose so they basically just were wrappers for the pointers passed into them and were unnecessary. --- lib/checkbufferoverrun.cpp | 22 +++++++++------------- lib/checkbufferoverrun.h | 6 +++--- lib/checknullpointer.cpp | 8 +++----- lib/checkuninitvar.cpp | 20 +++++++------------- lib/checkuninitvar.h | 3 +-- lib/ctu.cpp | 8 ++++---- lib/ctu.h | 2 +- lib/valueflow.cpp | 2 +- 8 files changed, 29 insertions(+), 42 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index ad0c28e57..e50e5a2b1 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -906,14 +906,11 @@ namespace { }; } -bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset, int type) +bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings *settings, const Token *argtok, MathLib::bigint *offset, int type) { if (!offset) return false; - const CheckBufferOverrun *c = dynamic_cast(check); - if (!c) - return false; - if (!argtok->valueType() || argtok->valueType()->typeSize(c->mSettings->platform) == 0) + if (!argtok->valueType() || argtok->valueType()->typeSize(settings->platform) == 0) return false; const Token *indexTok = nullptr; if (type == 1 && Token::Match(argtok, "%name% [") && argtok->astParent() == argtok->next() && !Token::simpleMatch(argtok->linkAt(1), "] [")) @@ -926,27 +923,26 @@ bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Check *check, const Token return false; if (!indexTok->hasKnownIntValue()) return false; - *offset = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(c->mSettings->platform); + *offset = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(settings->platform); return true; } -bool CheckBufferOverrun::isCtuUnsafeArrayIndex(const Check *check, const Token *argtok, MathLib::bigint *offset) +bool CheckBufferOverrun::isCtuUnsafeArrayIndex(const Settings *settings, const Token *argtok, MathLib::bigint *offset) { - return CheckBufferOverrun::isCtuUnsafeBufferUsage(check, argtok, offset, 1); + return isCtuUnsafeBufferUsage(settings, argtok, offset, 1); } -bool CheckBufferOverrun::isCtuUnsafePointerArith(const Check *check, const Token *argtok, MathLib::bigint *offset) +bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings *settings, const Token *argtok, MathLib::bigint *offset) { - return CheckBufferOverrun::isCtuUnsafeBufferUsage(check, argtok, offset, 2); + return isCtuUnsafeBufferUsage(settings, argtok, offset, 2); } /** @brief Parse current TU and extract file info */ Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - CheckBufferOverrun checkBufferOverrun(tokenizer, settings, nullptr); MyFileInfo *fileInfo = new MyFileInfo; - fileInfo->unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, &checkBufferOverrun, isCtuUnsafeArrayIndex); - fileInfo->unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, &checkBufferOverrun, isCtuUnsafePointerArith); + fileInfo->unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafeArrayIndex); + fileInfo->unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafePointerArith); if (fileInfo->unsafeArrayIndex.empty() && fileInfo->unsafePointerArith.empty()) { delete fileInfo; return nullptr; diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 58cc9843a..5ae2786e6 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -130,9 +130,9 @@ private: ValueFlow::Value getBufferSize(const Token *bufTok) const; // CTU - static bool isCtuUnsafeBufferUsage(const Check *check, const Token *argtok, MathLib::bigint *offset, int type); - static bool isCtuUnsafeArrayIndex(const Check *check, const Token *argtok, MathLib::bigint *offset); - static bool isCtuUnsafePointerArith(const Check *check, const Token *argtok, MathLib::bigint *offset); + static bool isCtuUnsafeBufferUsage(const Settings *settings, const Token *argtok, MathLib::bigint *offset, int type); + static bool isCtuUnsafeArrayIndex(const Settings *settings, const Token *argtok, MathLib::bigint *offset); + static bool isCtuUnsafePointerArith(const Settings *settings, const Token *argtok, MathLib::bigint *offset); Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override; static bool analyseWholeProgram1(const std::map> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger); diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index b50574e29..86a6542f0 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -549,12 +549,11 @@ void CheckNullPointer::redundantConditionWarning(const Token* tok, const ValueFl } // NOLINTNEXTLINE(readability-non-const-parameter) - used as callback so we need to preserve the signature -static bool isUnsafeUsage(const Check *check, const Token *vartok, MathLib::bigint *value) +static bool isUnsafeUsage(const Settings *settings, const Token *vartok, MathLib::bigint *value) { (void)value; - const CheckNullPointer *checkNullPointer = dynamic_cast(check); bool unknown = false; - return checkNullPointer && checkNullPointer->isPointerDeRef(vartok, unknown); + return CheckNullPointer::isPointerDeRef(vartok, unknown, settings); } namespace { @@ -575,8 +574,7 @@ namespace { Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - CheckNullPointer check(tokenizer, settings, nullptr); - const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, &check, ::isUnsafeUsage); + const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, isUnsafeUsage); if (unsafeUsage.empty()) return nullptr; diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 9b53be93f..98a39768c 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1129,8 +1129,9 @@ static bool isVoidCast(const Token *tok) return Token::simpleMatch(tok, "(") && tok->isCast() && tok->valueType() && tok->valueType()->type == ValueType::Type::VOID && tok->valueType()->pointer == 0; } -const Token* CheckUninitVar::isVariableUsage(bool cpp, const Token *vartok, const Library& library, bool pointer, Alloc alloc, int indirect) +const Token* CheckUninitVar::isVariableUsage(const Token *vartok, const Library& library, bool pointer, Alloc alloc, int indirect) { + const bool cpp = vartok->isCpp(); const Token *valueExpr = vartok; // non-dereferenced , no address of value as variable while (Token::Match(valueExpr->astParent(), ".|::") && astIsRhs(valueExpr)) valueExpr = valueExpr->astParent(); @@ -1334,7 +1335,7 @@ const Token* CheckUninitVar::isVariableUsage(bool cpp, const Token *vartok, cons const Token* CheckUninitVar::isVariableUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect) const { - return CheckUninitVar::isVariableUsage(mTokenizer->isCPP(), vartok, mSettings->library, pointer, alloc, indirect); + return isVariableUsage(vartok, mSettings->library, pointer, alloc, indirect); } /*** @@ -1681,18 +1682,11 @@ void CheckUninitVar::valueFlowUninit() } } -Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const -{ - const CheckUninitVar checker(tokenizer, settings, nullptr); - return checker.getFileInfo(); -} - // NOLINTNEXTLINE(readability-non-const-parameter) - used as callback so we need to preserve the signature -static bool isVariableUsage(const Check *check, const Token *vartok, MathLib::bigint *value) +static bool isVariableUsage(const Settings *settings, const Token *vartok, MathLib::bigint *value) { (void)value; - const CheckUninitVar *c = dynamic_cast(check); - return c && c->isVariableUsage(vartok, true, CheckUninitVar::Alloc::ARRAY); + return CheckUninitVar::isVariableUsage(vartok, settings->library, true, CheckUninitVar::Alloc::ARRAY); } namespace { @@ -1710,9 +1704,9 @@ namespace { }; } -Check::FileInfo *CheckUninitVar::getFileInfo() const +Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const { - const std::list &unsafeUsage = CTU::getUnsafeUsage(mTokenizer, mSettings, this, ::isVariableUsage); + const std::list &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage); if (unsafeUsage.empty()) return nullptr; diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 94b600990..9b6352aec 100644 --- a/lib/checkuninitvar.h +++ b/lib/checkuninitvar.h @@ -71,7 +71,7 @@ public: enum Alloc { NO_ALLOC, NO_CTOR_CALL, CTOR_CALL, ARRAY }; - static const Token *isVariableUsage(bool cpp, const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0); + static const Token *isVariableUsage(const Token *vartok, const Library &library, bool pointer, Alloc alloc, int indirect = 0); const Token *isVariableUsage(const Token *vartok, bool pointer, Alloc alloc, int indirect = 0) const; private: @@ -129,7 +129,6 @@ private: void uninitStructMemberError(const Token *tok, const std::string &membername); std::set mUninitDiags; - Check::FileInfo* getFileInfo() const; void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const override { diff --git a/lib/ctu.cpp b/lib/ctu.cpp index a67254667..c5ab93eb2 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -436,7 +436,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer) return fileInfo; } -static std::list> getUnsafeFunction(const Tokenizer *tokenizer, const Settings *settings, const Scope *scope, int argnr, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value)) +static std::list> getUnsafeFunction(const Tokenizer *tokenizer, const Settings *settings, const Scope *scope, int argnr, bool (*isUnsafeUsage)(const Settings *settings, const Token *argtok, MathLib::bigint *value)) { std::list> ret; const Variable * const argvar = scope->function->getArgumentVar(argnr); @@ -460,7 +460,7 @@ static std::list> getUnsafeFunction(co if (tok2->variable() != argvar) continue; MathLib::bigint value = 0; - if (!isUnsafeUsage(check, tok2, &value)) + if (!isUnsafeUsage(settings, tok2, &value)) return ret; // TODO: Is this a read? then continue.. ret.emplace_back(tok2, value); return ret; @@ -468,7 +468,7 @@ static std::list> getUnsafeFunction(co return ret; } -std::list CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value)) +std::list CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, bool (*isUnsafeUsage)(const Settings *settings, const Token *argtok, MathLib::bigint *value)) { std::list unsafeUsage; @@ -482,7 +482,7 @@ std::list CTU::getUnsafeUsage(const Tokenizer *token // "Unsafe" functions unconditionally reads data before it is written.. for (int argnr = 0; argnr < function->argCount(); ++argnr) { - for (const std::pair &v : getUnsafeFunction(tokenizer, settings, &scope, argnr, check, isUnsafeUsage)) { + for (const std::pair &v : getUnsafeFunction(tokenizer, settings, &scope, argnr, isUnsafeUsage)) { const Token *tok = v.first; const MathLib::bigint val = v.second; unsafeUsage.emplace_back(CTU::getFunctionId(tokenizer, function), argnr+1, tok->str(), CTU::FileInfo::Location(tokenizer,tok), val); diff --git a/lib/ctu.h b/lib/ctu.h index 7fdd0f093..28e442187 100644 --- a/lib/ctu.h +++ b/lib/ctu.h @@ -146,7 +146,7 @@ namespace CTU { /** @brief Parse current TU and extract file info */ CPPCHECKLIB FileInfo *getFileInfo(const Tokenizer *tokenizer); - CPPCHECKLIB std::list getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value)); + CPPCHECKLIB std::list getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, bool (*isUnsafeUsage)(const Settings *settings, const Token *argtok, MathLib::bigint *value)); CPPCHECKLIB std::list loadUnsafeUsageListFromXml(const tinyxml2::XMLElement *xmlElement); } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 6f5922938..4280d8cab 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7658,7 +7658,7 @@ static void valueFlowSubFunction(TokenList& tokenlist, SymbolDatabase& symboldat }); // Remove uninit values if argument is passed by value if (argtok->variable() && !argtok->variable()->isPointer() && argvalues.size() == 1 && argvalues.front().isUninitValue()) { - if (CheckUninitVar::isVariableUsage(tokenlist.isCPP(), argtok, settings.library, false, CheckUninitVar::Alloc::NO_ALLOC, 0)) + if (CheckUninitVar::isVariableUsage(argtok, settings.library, false, CheckUninitVar::Alloc::NO_ALLOC, 0)) continue; }