Skip to content

Commit bca43c2

Browse files
chrchr-githubchrchr-github
authored
Use ValueFlow::getSizeOf() in CheckOther (#6257)
1 parent 4f8f7cc commit bca43c2

2 files changed

Lines changed: 38 additions & 59 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 4 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,55 +1206,6 @@ void CheckOther::commaSeparatedReturnError(const Token *tok)
12061206
"macro is then used in a return statement, it is less likely such code is misunderstood.", CWE398, Certainty::normal);
12071207
}
12081208

1209-
//---------------------------------------------------------------------------
1210-
// Check for function parameters that should be passed by const reference
1211-
//---------------------------------------------------------------------------
1212-
static int estimateSize(const Type* type, const Settings* settings, const SymbolDatabase* symbolDatabase, int recursionDepth = 0)
1213-
{
1214-
if (recursionDepth > 20)
1215-
return 0;
1216-
1217-
int cumulatedSize = 0;
1218-
const bool isUnion = type->classScope->type == Scope::ScopeType::eUnion;
1219-
const auto accumulateSize = [](int& cumulatedSize, int size, bool isUnion) -> void {
1220-
if (isUnion)
1221-
cumulatedSize = std::max(cumulatedSize, size);
1222-
else
1223-
cumulatedSize += size;
1224-
};
1225-
std::set<const Scope*> anonScopes;
1226-
for (const Variable& var : type->classScope->varlist) {
1227-
int size = 0;
1228-
if (var.isStatic())
1229-
continue;
1230-
if (var.isPointer() || var.isReference())
1231-
size = settings->platform.sizeof_pointer;
1232-
else if (var.type() && var.type()->classScope)
1233-
size = estimateSize(var.type(), settings, symbolDatabase, recursionDepth + 1);
1234-
else if (var.valueType() && var.valueType()->type == ValueType::Type::CONTAINER)
1235-
size = 3 * settings->platform.sizeof_pointer; // Just guess
1236-
else if (var.nameToken()->scope() != type->classScope && var.nameToken()->scope()->definedType) { // anonymous union
1237-
const auto ret = anonScopes.insert(var.nameToken()->scope());
1238-
if (ret.second)
1239-
size = estimateSize(var.nameToken()->scope()->definedType, settings, symbolDatabase, recursionDepth + 1);
1240-
}
1241-
else
1242-
size = symbolDatabase->sizeOfType(var.typeStartToken());
1243-
1244-
if (var.isArray())
1245-
size *= std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), 1, [](int v, const Dimension& d) {
1246-
return v *= d.num;
1247-
});
1248-
1249-
accumulateSize(cumulatedSize, size, isUnion);
1250-
}
1251-
return std::accumulate(type->derivedFrom.cbegin(), type->derivedFrom.cend(), cumulatedSize, [&](int v, const Type::BaseInfo& baseInfo) {
1252-
if (baseInfo.type && baseInfo.type->classScope)
1253-
v += estimateSize(baseInfo.type, settings, symbolDatabase, recursionDepth + 1);
1254-
return v;
1255-
});
1256-
}
1257-
12581209
void CheckOther::checkPassByReference()
12591210
{
12601211
if (!mSettings->severity.isEnabled(Severity::performance) || mTokenizer->isC())
@@ -1288,7 +1239,7 @@ void CheckOther::checkPassByReference()
12881239
// Ensure that it is a large object.
12891240
if (!var->type()->classScope)
12901241
inconclusive = true;
1291-
else if (estimateSize(var->type(), mSettings, symbolDatabase) <= 2 * mSettings->platform.sizeof_pointer)
1242+
else if (!var->valueType() || ValueFlow::getSizeOf(*var->valueType(), *mSettings) <= 2 * mSettings->platform.sizeof_pointer)
12921243
continue;
12931244
}
12941245
else
@@ -2928,7 +2879,7 @@ void CheckOther::checkRedundantCopy()
29282879
const Token* varTok = fScope->bodyEnd->tokAt(-2);
29292880
if (varTok->variable() && !varTok->variable()->isGlobal() &&
29302881
(!varTok->variable()->type() || !varTok->variable()->type()->classScope ||
2931-
estimateSize(varTok->variable()->type(), mSettings, symbolDatabase) > 2 * mSettings->platform.sizeof_pointer))
2882+
(varTok->variable()->valueType() && ValueFlow::getSizeOf(*varTok->variable()->valueType(), *mSettings) > 2 * mSettings->platform.sizeof_pointer)))
29322883
redundantCopyError(startTok, startTok->str());
29332884
}
29342885
}
@@ -3041,8 +2992,8 @@ void CheckOther::checkIncompleteArrayFill()
30412992
int size = mTokenizer->sizeOfType(var->typeStartToken());
30422993
if (size == 0 && var->valueType()->pointer)
30432994
size = mSettings->platform.sizeof_pointer;
3044-
else if (size == 0 && var->type())
3045-
size = estimateSize(var->type(), mSettings, symbolDatabase);
2995+
else if (size == 0 && var->valueType())
2996+
size = ValueFlow::getSizeOf(*var->valueType(), *mSettings);
30462997
const Token* tok3 = tok->next()->astOperand2()->astOperand1()->astOperand1();
30472998
if ((size != 1 && size != 100 && size != 0) || var->isPointer()) {
30482999
if (printWarning)

‎lib/valueflow.cpp‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,7 @@ template<class F>
11101110
static size_t accumulateStructMembers(const Scope* scope, F f)
11111111
{
11121112
size_t total = 0;
1113+
std::set<const Scope*> anonScopes;
11131114
for (const Variable& var : scope->varlist) {
11141115
if (var.isStatic())
11151116
continue;
@@ -1119,7 +1120,13 @@ static size_t accumulateStructMembers(const Scope* scope, F f)
11191120
const MathLib::bigint dim = std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension& dim) {
11201121
return i1 * dim.num;
11211122
});
1122-
total = f(total, *vt, dim);
1123+
if (var.nameToken()->scope() != scope && var.nameToken()->scope()->definedType) { // anonymous union
1124+
const auto ret = anonScopes.insert(var.nameToken()->scope());
1125+
if (ret.second)
1126+
total = f(total, *vt, dim);
1127+
}
1128+
else
1129+
total = f(total, *vt, dim);
11231130
}
11241131
if (total == 0)
11251132
return 0;
@@ -1148,11 +1155,22 @@ static size_t getAlignOf(const ValueType& vt, const Settings& settings)
11481155
return align == 0 ? 0 : bitCeil(align);
11491156
}
11501157
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
1151-
return accumulateStructMembers(vt.typeScope, [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
1158+
auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
11521159
size_t a = getAlignOf(vt2, settings);
11531160
return std::max(max, a);
1154-
});
1161+
};
1162+
size_t total = 0;
1163+
if (const Type* dt = vt.typeScope->definedType) {
1164+
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
1165+
if (bi.type && bi.type->classScope)
1166+
v += accumulateStructMembers(bi.type->classScope, accHelper);
1167+
return v;
1168+
});
1169+
}
1170+
return total + accumulateStructMembers(vt.typeScope, accHelper);
11551171
}
1172+
if (vt.type == ValueType::Type::CONTAINER)
1173+
return settings.platform.sizeof_pointer; // Just guess
11561174
return 0;
11571175
}
11581176

@@ -1185,16 +1203,26 @@ size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings)
11851203
return settings.platform.sizeof_double;
11861204
if (vt.type == ValueType::Type::LONGDOUBLE)
11871205
return settings.platform.sizeof_long_double;
1206+
if (vt.type == ValueType::Type::CONTAINER)
1207+
return 3 * settings.platform.sizeof_pointer; // Just guess
11881208
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
1189-
size_t total = accumulateStructMembers(vt.typeScope, [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
1209+
auto accHelper = [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
11901210
size_t n = ValueFlow::getSizeOf(vt2, settings);
11911211
size_t a = getAlignOf(vt2, settings);
11921212
if (n == 0 || a == 0)
11931213
return 0;
11941214
n *= dim;
11951215
size_t padding = (a - (total % a)) % a;
1196-
return total + padding + n;
1197-
});
1216+
return vt.typeScope->type == Scope::eUnion ? std::max(total, n) : total + padding + n;
1217+
};
1218+
size_t total = accumulateStructMembers(vt.typeScope, accHelper);
1219+
if (const Type* dt = vt.typeScope->definedType) {
1220+
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
1221+
if (bi.type && bi.type->classScope)
1222+
v += accumulateStructMembers(bi.type->classScope, accHelper);
1223+
return v;
1224+
});
1225+
}
11981226
if (total == 0)
11991227
return 0;
12001228
size_t align = getAlignOf(vt, settings);

0 commit comments

Comments
 (0)