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.
This commit is contained in:
Oliver Stöneberg 2023-11-03 18:27:11 +01:00 committed by GitHub
parent f62caa6739
commit 989d276dde
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 29 additions and 42 deletions

View File

@ -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<const CheckBufferOverrun *>(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;

View File

@ -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<std::string, std::list<const CTU::FileInfo::CallBase *>> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger);

View File

@ -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<const CheckNullPointer *>(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<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, &check, ::isUnsafeUsage);
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, isUnsafeUsage);
if (unsafeUsage.empty())
return nullptr;

View File

@ -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<const CheckUninitVar *>(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<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(mTokenizer, mSettings, this, ::isVariableUsage);
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage);
if (unsafeUsage.empty())
return nullptr;

View File

@ -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<const Token*> mUninitDiags;
Check::FileInfo* getFileInfo() const;
void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const override
{

View File

@ -436,7 +436,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
return fileInfo;
}
static std::list<std::pair<const Token *, MathLib::bigint>> 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<std::pair<const Token *, MathLib::bigint>> 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<std::pair<const Token *, MathLib::bigint>> ret;
const Variable * const argvar = scope->function->getArgumentVar(argnr);
@ -460,7 +460,7 @@ static std::list<std::pair<const Token *, MathLib::bigint>> 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<std::pair<const Token *, MathLib::bigint>> getUnsafeFunction(co
return ret;
}
std::list<CTU::FileInfo::UnsafeUsage> 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::FileInfo::UnsafeUsage> CTU::getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, bool (*isUnsafeUsage)(const Settings *settings, const Token *argtok, MathLib::bigint *value))
{
std::list<CTU::FileInfo::UnsafeUsage> unsafeUsage;
@ -482,7 +482,7 @@ std::list<CTU::FileInfo::UnsafeUsage> 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<const Token *, MathLib::bigint> &v : getUnsafeFunction(tokenizer, settings, &scope, argnr, check, isUnsafeUsage)) {
for (const std::pair<const Token *, MathLib::bigint> &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);

View File

@ -146,7 +146,7 @@ namespace CTU {
/** @brief Parse current TU and extract file info */
CPPCHECKLIB FileInfo *getFileInfo(const Tokenizer *tokenizer);
CPPCHECKLIB std::list<FileInfo::UnsafeUsage> getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, const Check *check, bool (*isUnsafeUsage)(const Check *check, const Token *argtok, MathLib::bigint *value));
CPPCHECKLIB std::list<FileInfo::UnsafeUsage> getUnsafeUsage(const Tokenizer *tokenizer, const Settings *settings, bool (*isUnsafeUsage)(const Settings *settings, const Token *argtok, MathLib::bigint *value));
CPPCHECKLIB std::list<FileInfo::UnsafeUsage> loadUnsafeUsageListFromXml(const tinyxml2::XMLElement *xmlElement);
}

View File

@ -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;
}