Skip to content

removed the need for Check object in CTU::getUnsafeUsage() callback / some CheckUninitVar cleanups #5610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 3 commits into from
Nov 3, 2023
Merged

removed the need for Check object in CTU::getUnsafeUsage() callback / some CheckUninitVar cleanups #5610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4467570
removed the need for `Check` object in `CTU::getUnsafeUsage()` callback
firewave Nov 2, 2023
db68d97
CheckUninitVar: removed unnecessary `getFileInfo()` member function
firewave Nov 2, 2023
6f37793
CheckUninitVar: removed unnecessary `cpp` parameter from `isVariableU…
firewave Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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), "] ["))
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions lib/checkbufferoverrun.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,9 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
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);
Expand Down
8 changes: 3 additions & 5 deletions lib/checknullpointer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;

Expand Down
20 changes: 7 additions & 13 deletions lib/checkuninitvar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Copy Markdown
Collaborator Author

@firewave firewave Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can simply be taken from Token as it just points back to the TokenList it belongs to. The same applies to Tokenizer::isCPP().

const Token *valueExpr = vartok; // non-dereferenced , no address of value as variable
while (Token::Match(valueExpr->astParent(), ".|::") && astIsRhs(valueExpr))
valueExpr = valueExpr->astParent();
Expand Down Expand Up @@ -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);
}

/***
Expand Down Expand Up @@ -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 {
Expand All @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions lib/checkuninitvar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {

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:
Expand Down Expand Up @@ -129,7 +129,6 @@ class CPPCHECKLIB CheckUninitVar : public Check {
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
{
Expand Down
8 changes: 4 additions & 4 deletions lib/ctu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -460,15 +460,15 @@ 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;
}
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;

Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/ctu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down