Skip to content

Commit

Permalink
Use ValueFlow::getSizeOf() in CheckOther (danmar#6257)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrchr-github
chrchr-github committed Apr 10, 2024
1 parent 4f8f7cc commit bca43c2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 59 deletions.
57 changes: 4 additions & 53 deletions lib/checkother.cpp
View file
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
40 changes: 34 additions & 6 deletions lib/valueflow.cpp
View file
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

0 comments on commit bca43c2

Please sign in to comment.