Skip to content

Use ValueFlow::getSizeOf() in CheckOther #6257

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 5 commits into from
Apr 10, 2024
Merged

Use ValueFlow::getSizeOf() in CheckOther #6257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
21976cb
Update valueflow.cpp
chrchr-github Apr 8, 2024
4a69b61
Update checkother.cpp
chrchr-github Apr 8, 2024
b624c9c
Update valueflow.cpp
chrchr-github Apr 8, 2024
dab0a3e
Format
chrchr-github Apr 8, 2024
10bac03
clang-tidy
chrchr-github Apr 8, 2024
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
57 changes: 4 additions & 53 deletions lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1206,55 +1206,6 @@ void CheckOther::commaSeparatedReturnError(const Token *tok)
"macro is then used in a return statement, it is less likely such code is misunderstood.", CWE398, Certainty::normal);
}

//---------------------------------------------------------------------------
// Check for function parameters that should be passed by const reference
//---------------------------------------------------------------------------
static int estimateSize(const Type* type, const Settings* settings, const SymbolDatabase* symbolDatabase, int recursionDepth = 0)
{
if (recursionDepth > 20)
return 0;

int cumulatedSize = 0;
const bool isUnion = type->classScope->type == Scope::ScopeType::eUnion;
const auto accumulateSize = [](int& cumulatedSize, int size, bool isUnion) -> void {
if (isUnion)
cumulatedSize = std::max(cumulatedSize, size);
else
cumulatedSize += size;
};
std::set<const Scope*> anonScopes;
for (const Variable& var : type->classScope->varlist) {
int size = 0;
if (var.isStatic())
continue;
if (var.isPointer() || var.isReference())
size = settings->platform.sizeof_pointer;
else if (var.type() && var.type()->classScope)
size = estimateSize(var.type(), settings, symbolDatabase, recursionDepth + 1);
else if (var.valueType() && var.valueType()->type == ValueType::Type::CONTAINER)
size = 3 * settings->platform.sizeof_pointer; // Just guess
else if (var.nameToken()->scope() != type->classScope && var.nameToken()->scope()->definedType) { // anonymous union
const auto ret = anonScopes.insert(var.nameToken()->scope());
if (ret.second)
size = estimateSize(var.nameToken()->scope()->definedType, settings, symbolDatabase, recursionDepth + 1);
}
else
size = symbolDatabase->sizeOfType(var.typeStartToken());

if (var.isArray())
size *= std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), 1, [](int v, const Dimension& d) {
return v *= d.num;
});

accumulateSize(cumulatedSize, size, isUnion);
}
return std::accumulate(type->derivedFrom.cbegin(), type->derivedFrom.cend(), cumulatedSize, [&](int v, const Type::BaseInfo& baseInfo) {
if (baseInfo.type && baseInfo.type->classScope)
v += estimateSize(baseInfo.type, settings, symbolDatabase, recursionDepth + 1);
return v;
});
}

void CheckOther::checkPassByReference()
{
if (!mSettings->severity.isEnabled(Severity::performance) || mTokenizer->isC())
Expand Down Expand Up @@ -1288,7 +1239,7 @@ void CheckOther::checkPassByReference()
// Ensure that it is a large object.
if (!var->type()->classScope)
inconclusive = true;
else if (estimateSize(var->type(), mSettings, symbolDatabase) <= 2 * mSettings->platform.sizeof_pointer)
else if (!var->valueType() || ValueFlow::getSizeOf(*var->valueType(), *mSettings) <= 2 * mSettings->platform.sizeof_pointer)
continue;
}
else
Expand Down Expand Up @@ -2928,7 +2879,7 @@ void CheckOther::checkRedundantCopy()
const Token* varTok = fScope->bodyEnd->tokAt(-2);
if (varTok->variable() && !varTok->variable()->isGlobal() &&
(!varTok->variable()->type() || !varTok->variable()->type()->classScope ||
estimateSize(varTok->variable()->type(), mSettings, symbolDatabase) > 2 * mSettings->platform.sizeof_pointer))
(varTok->variable()->valueType() && ValueFlow::getSizeOf(*varTok->variable()->valueType(), *mSettings) > 2 * mSettings->platform.sizeof_pointer)))
redundantCopyError(startTok, startTok->str());
}
}
Expand Down Expand Up @@ -3041,8 +2992,8 @@ void CheckOther::checkIncompleteArrayFill()
int size = mTokenizer->sizeOfType(var->typeStartToken());
if (size == 0 && var->valueType()->pointer)
size = mSettings->platform.sizeof_pointer;
else if (size == 0 && var->type())
size = estimateSize(var->type(), mSettings, symbolDatabase);
else if (size == 0 && var->valueType())
size = ValueFlow::getSizeOf(*var->valueType(), *mSettings);
const Token* tok3 = tok->next()->astOperand2()->astOperand1()->astOperand1();
if ((size != 1 && size != 100 && size != 0) || var->isPointer()) {
if (printWarning)
Expand Down
22 changes: 11 additions & 11 deletions lib/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,17 @@ class CPPCHECKLIB Platform {
nonneg int long_long_bit; /// bits in long long

/** size of standard types */
nonneg int sizeof_bool;
nonneg int sizeof_short;
nonneg int sizeof_int;
nonneg int sizeof_long;
nonneg int sizeof_long_long;
nonneg int sizeof_float;
nonneg int sizeof_double;
nonneg int sizeof_long_double;
nonneg int sizeof_wchar_t;
nonneg int sizeof_size_t;
nonneg int sizeof_pointer;
std::size_t sizeof_bool;
std::size_t sizeof_short;
std::size_t sizeof_int;
std::size_t sizeof_long;
std::size_t sizeof_long_long;
std::size_t sizeof_float;
std::size_t sizeof_double;
std::size_t sizeof_long_double;
std::size_t sizeof_wchar_t;
std::size_t sizeof_size_t;
std::size_t sizeof_pointer;

char defaultSign; // unsigned:'u', signed:'s', unknown:'\0'

Expand Down
40 changes: 34 additions & 6 deletions lib/valueflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,7 @@ template<class F>
static size_t accumulateStructMembers(const Scope* scope, F f)
{
size_t total = 0;
std::set<const Scope*> anonScopes;
for (const Variable& var : scope->varlist) {
if (var.isStatic())
continue;
Expand All @@ -1119,7 +1120,13 @@ static size_t accumulateStructMembers(const Scope* scope, F f)
const MathLib::bigint dim = std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension& dim) {
return i1 * dim.num;
});
total = f(total, *vt, dim);
if (var.nameToken()->scope() != scope && var.nameToken()->scope()->definedType) { // anonymous union
const auto ret = anonScopes.insert(var.nameToken()->scope());
if (ret.second)
total = f(total, *vt, dim);
}
else
total = f(total, *vt, dim);
}
if (total == 0)
return 0;
Expand Down Expand Up @@ -1148,11 +1155,22 @@ static size_t getAlignOf(const ValueType& vt, const Settings& settings)
return align == 0 ? 0 : bitCeil(align);
}
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
return accumulateStructMembers(vt.typeScope, [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
size_t a = getAlignOf(vt2, settings);
return std::max(max, a);
});
};
size_t total = 0;
if (const Type* dt = vt.typeScope->definedType) {
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
if (bi.type && bi.type->classScope)
v += accumulateStructMembers(bi.type->classScope, accHelper);
return v;
});
}
return total + accumulateStructMembers(vt.typeScope, accHelper);
}
if (vt.type == ValueType::Type::CONTAINER)
return settings.platform.sizeof_pointer; // Just guess
return 0;
}

Expand Down Expand Up @@ -1185,16 +1203,26 @@ size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings)
return settings.platform.sizeof_double;
if (vt.type == ValueType::Type::LONGDOUBLE)
return settings.platform.sizeof_long_double;
if (vt.type == ValueType::Type::CONTAINER)
return 3 * settings.platform.sizeof_pointer; // Just guess
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
size_t total = accumulateStructMembers(vt.typeScope, [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
auto accHelper = [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
size_t n = ValueFlow::getSizeOf(vt2, settings);
size_t a = getAlignOf(vt2, settings);
if (n == 0 || a == 0)
return 0;
n *= dim;
size_t padding = (a - (total % a)) % a;
return total + padding + n;
});
return vt.typeScope->type == Scope::eUnion ? std::max(total, n) : total + padding + n;
};
size_t total = accumulateStructMembers(vt.typeScope, accHelper);
if (const Type* dt = vt.typeScope->definedType) {
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
if (bi.type && bi.type->classScope)
v += accumulateStructMembers(bi.type->classScope, accHelper);
return v;
});
}
if (total == 0)
return 0;
size_t align = getAlignOf(vt, settings);
Expand Down