Skip to content

Commit 769cbfe

Browse files
autoantwortautoantwort
committed
ValueFlow: don't reset the struct size if there is a unknown member + empty structs have a size of 1
1 parent 69f7241 commit 769cbfe

11 files changed

Lines changed: 107 additions & 49 deletions

‎lib/checkclass.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3397,7 +3397,9 @@ void CheckClass::checkReturnByReference()
33973397
const bool isView = isContainer && var->valueType()->container->view;
33983398
bool warn = isContainer && !isView;
33993399
if (!warn && !isView) {
3400-
const std::size_t size = ValueFlow::getSizeOf(*var->valueType(), *mSettings);
3400+
const std::size_t size = ValueFlow::getSizeOf(*var->valueType(),
3401+
*mSettings,
3402+
ValueFlow::Accuracy::LowerBound);
34013403
if (size > 2 * mSettings->platform.sizeof_pointer)
34023404
warn = true;
34033405
}

‎lib/checkother.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,7 +1522,7 @@ static bool isLargeContainer(const Variable* var, const Settings& settings)
15221522
return false;
15231523
}
15241524
const ValueType vtElem = ValueType::parseDecl(vt->containerTypeToken, settings);
1525-
const auto elemSize = std::max<std::size_t>(ValueFlow::getSizeOf(vtElem, settings), 1);
1525+
const auto elemSize = std::max<std::size_t>(ValueFlow::getSizeOf(vtElem, settings, ValueFlow::Accuracy::LowerBound), 1);
15261526
const auto arraySize = var->dimension(0) * elemSize;
15271527
return arraySize > maxByValueSize;
15281528
}
@@ -1562,7 +1562,7 @@ void CheckOther::checkPassByReference()
15621562
// Ensure that it is a large object.
15631563
if (!var->type()->classScope)
15641564
inconclusive = true;
1565-
else if (!var->valueType() || ValueFlow::getSizeOf(*var->valueType(), *mSettings) <= 2 * mSettings->platform.sizeof_pointer)
1565+
else if (!var->valueType() || ValueFlow::getSizeOf(*var->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound) <= 2 * mSettings->platform.sizeof_pointer)
15661566
continue;
15671567
}
15681568
else
@@ -3283,7 +3283,7 @@ void CheckOther::checkRedundantCopy()
32833283
const Token* varTok = fScope->bodyEnd->tokAt(-2);
32843284
if (varTok->variable() && !varTok->variable()->isGlobal() &&
32853285
(!varTok->variable()->type() || !varTok->variable()->type()->classScope ||
3286-
(varTok->variable()->valueType() && ValueFlow::getSizeOf(*varTok->variable()->valueType(), *mSettings) > 2 * mSettings->platform.sizeof_pointer)))
3286+
(varTok->variable()->valueType() && ValueFlow::getSizeOf(*varTok->variable()->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound) > 2 * mSettings->platform.sizeof_pointer)))
32873287
redundantCopyError(startTok, startTok->str());
32883288
}
32893289
}
@@ -3403,7 +3403,7 @@ void CheckOther::checkIncompleteArrayFill()
34033403
if (size == 0 && var->valueType()->pointer)
34043404
size = mSettings->platform.sizeof_pointer;
34053405
else if (size == 0 && var->valueType())
3406-
size = ValueFlow::getSizeOf(*var->valueType(), *mSettings);
3406+
size = ValueFlow::getSizeOf(*var->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound);
34073407
const Token* tok3 = tok->next()->astOperand2()->astOperand1()->astOperand1();
34083408
if ((size != 1 && size != 100 && size != 0) || var->isPointer()) {
34093409
if (printWarning)
@@ -4346,7 +4346,7 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
43464346
bufToken = expr->astOperand1()->astOperand1();
43474347
offsetToken = expr->astOperand1()->astOperand2();
43484348
if (expr->astOperand1()->valueType())
4349-
elementSize = ValueFlow::getSizeOf(*expr->astOperand1()->valueType(), settings);
4349+
elementSize = ValueFlow::getSizeOf(*expr->astOperand1()->valueType(), settings, ValueFlow::Accuracy::LowerBound);
43504350
} else if (Token::Match(expr, "+|-") && expr->isBinaryOp()) {
43514351
const bool pointer1 = (expr->astOperand1()->valueType() && expr->astOperand1()->valueType()->pointer > 0);
43524352
const bool pointer2 = (expr->astOperand2()->valueType() && expr->astOperand2()->valueType()->pointer > 0);
@@ -4355,13 +4355,13 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
43554355
offsetToken = expr->astOperand2();
43564356
auto vt = *expr->astOperand1()->valueType();
43574357
--vt.pointer;
4358-
elementSize = ValueFlow::getSizeOf(vt, settings);
4358+
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
43594359
} else if (!pointer1 && pointer2) {
43604360
bufToken = expr->astOperand2();
43614361
offsetToken = expr->astOperand1();
43624362
auto vt = *expr->astOperand2()->valueType();
43634363
--vt.pointer;
4364-
elementSize = ValueFlow::getSizeOf(vt, settings);
4364+
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
43654365
} else {
43664366
return false;
43674367
}
@@ -4370,7 +4370,7 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
43704370
*offset = 0;
43714371
auto vt = *expr->valueType();
43724372
--vt.pointer;
4373-
elementSize = ValueFlow::getSizeOf(vt, settings);
4373+
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
43744374
if (elementSize > 0) {
43754375
*offset *= elementSize;
43764376
if (sizeValue)

‎lib/checktype.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,12 @@ static bool checkTypeCombination(ValueType src, ValueType tgt, const Settings& s
322322
src.reference = Reference::None;
323323
tgt.reference = Reference::None;
324324

325-
const std::size_t sizeSrc = ValueFlow::getSizeOf(src, settings);
326-
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt, settings);
325+
const std::size_t sizeSrc = ValueFlow::getSizeOf(src,
326+
settings,
327+
ValueFlow::Accuracy::ExcactOrZero);
328+
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt,
329+
settings,
330+
ValueFlow::Accuracy::ExcactOrZero);
327331
if (!(sizeSrc > 0 && sizeTgt > 0 && sizeSrc < sizeTgt))
328332
return false;
329333

‎lib/token.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ nonneg int Token::getStrSize(const Token *tok, const Settings &settings)
784784
if (tok->valueType()) {
785785
ValueType vt(*tok->valueType());
786786
vt.pointer = 0;
787-
sizeofType = ValueFlow::getSizeOf(vt, settings);
787+
sizeofType = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::ExcactOrZero);
788788
}
789789
return getStrArraySize(tok) * sizeofType;
790790
}

‎lib/valueflow.cpp‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
426426

427427

428428
template<class F>
429-
static size_t accumulateStructMembers(const Scope* scope, F f)
429+
static size_t accumulateStructMembers(const Scope* scope, F f, ValueFlow::Accuracy accuracy)
430430
{
431431
size_t total = 0;
432432
std::set<const Scope*> anonScopes;
@@ -447,7 +447,7 @@ static size_t accumulateStructMembers(const Scope* scope, F f)
447447
else
448448
total = f(total, *vt, dim);
449449
}
450-
if (total == 0)
450+
if (accuracy == ValueFlow::Accuracy::ExcactOrZero && total == 0)
451451
return 0;
452452
}
453453
return total;
@@ -467,37 +467,37 @@ static size_t bitCeil(size_t x)
467467
return x + 1;
468468
}
469469

470-
static size_t getAlignOf(const ValueType& vt, const Settings& settings, int maxRecursion = 0)
470+
static size_t getAlignOf(const ValueType& vt, const Settings& settings, ValueFlow::Accuracy accuracy, int maxRecursion = 0)
471471
{
472472
if (maxRecursion == settings.vfOptions.maxAlignOfRecursion) {
473473
// TODO: add bailout message
474474
return 0;
475475
}
476476
if (vt.pointer || vt.reference != Reference::None || vt.isPrimitive()) {
477-
auto align = ValueFlow::getSizeOf(vt, settings);
477+
auto align = ValueFlow::getSizeOf(vt, settings, accuracy);
478478
return align == 0 ? 0 : bitCeil(align);
479479
}
480480
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
481481
auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
482-
size_t a = getAlignOf(vt2, settings, ++maxRecursion);
482+
size_t a = getAlignOf(vt2, settings, accuracy, ++maxRecursion);
483483
return std::max(max, a);
484484
};
485485
size_t total = 0;
486486
if (const Type* dt = vt.typeScope->definedType) {
487487
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
488488
if (bi.type && bi.type->classScope)
489-
v += accumulateStructMembers(bi.type->classScope, accHelper);
489+
v += accumulateStructMembers(bi.type->classScope, accHelper, accuracy);
490490
return v;
491491
});
492492
}
493-
return total + accumulateStructMembers(vt.typeScope, accHelper);
493+
return total + accumulateStructMembers(vt.typeScope, accHelper, accuracy);
494494
}
495495
if (vt.type == ValueType::Type::CONTAINER)
496496
return settings.platform.sizeof_pointer; // Just guess
497497
return 0;
498498
}
499499

500-
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int maxRecursion)
500+
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, Accuracy accuracy, int maxRecursion)
501501
{
502502
if (maxRecursion == settings.vfOptions.maxSizeOfRecursion) {
503503
// TODO: add bailout message
@@ -527,27 +527,28 @@ size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int m
527527
return 3 * settings.platform.sizeof_pointer; // Just guess
528528
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
529529
auto accHelper = [&](size_t total, const ValueType& vt2, size_t dim) -> size_t {
530-
size_t n = ValueFlow::getSizeOf(vt2, settings, ++maxRecursion);
531-
size_t a = getAlignOf(vt2, settings);
530+
size_t n = ValueFlow::getSizeOf(vt2, settings,accuracy, ++maxRecursion);
531+
size_t a = getAlignOf(vt2, settings, accuracy);
532532
if (n == 0 || a == 0)
533-
return 0;
533+
return accuracy == Accuracy::ExcactOrZero ? 0 : total;
534534
n *= dim;
535535
size_t padding = (a - (total % a)) % a;
536536
return vt.typeScope->type == ScopeType::eUnion ? std::max(total, n) : total + padding + n;
537537
};
538-
size_t total = accumulateStructMembers(vt.typeScope, accHelper);
538+
size_t total = accumulateStructMembers(vt.typeScope, accHelper, accuracy);
539539
if (const Type* dt = vt.typeScope->definedType) {
540540
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
541541
if (bi.type && bi.type->classScope)
542-
v += accumulateStructMembers(bi.type->classScope, accHelper);
542+
v += accumulateStructMembers(bi.type->classScope, accHelper, accuracy);
543543
return v;
544544
});
545545
}
546-
if (total == 0)
546+
if (accuracy == Accuracy::ExcactOrZero && total == 0)
547547
return 0;
548-
size_t align = getAlignOf(vt, settings);
548+
total = std::max(size_t{1}, total);
549+
size_t align = getAlignOf(vt, settings, accuracy);
549550
if (align == 0)
550-
return 0;
551+
return accuracy == Accuracy::ExcactOrZero ? 0 : total;
551552
total += (align - (total % align)) % align;
552553
return total;
553554
}
@@ -3614,8 +3615,8 @@ static bool isTruncated(const ValueType* src, const ValueType* dst, const Settin
36143615
if (src->smartPointer && dst->smartPointer)
36153616
return false;
36163617
if ((src->isIntegral() && dst->isIntegral()) || (src->isFloat() && dst->isFloat())) {
3617-
const size_t srcSize = ValueFlow::getSizeOf(*src, settings);
3618-
const size_t dstSize = ValueFlow::getSizeOf(*dst, settings);
3618+
const size_t srcSize = ValueFlow::getSizeOf(*src, settings, ValueFlow::Accuracy::LowerBound);
3619+
const size_t dstSize = ValueFlow::getSizeOf(*dst, settings, ValueFlow::Accuracy::LowerBound);
36193620
if (srcSize > dstSize)
36203621
return true;
36213622
if (srcSize == dstSize && src->sign != dst->sign)
@@ -4138,10 +4139,10 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
41384139
if (!dst || !dst->isIntegral())
41394140
return values;
41404141

4141-
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
4142+
const size_t sz = ValueFlow::getSizeOf(*dst, settings, ValueFlow::Accuracy::ExcactOrZero);
41424143

41434144
if (src) {
4144-
const size_t osz = ValueFlow::getSizeOf(*src, settings);
4145+
const size_t osz = ValueFlow::getSizeOf(*src, settings, ValueFlow::Accuracy::ExcactOrZero);
41454146
if (osz >= sz && dst->sign == ValueType::Sign::SIGNED && src->sign == ValueType::Sign::UNSIGNED) {
41464147
values.remove_if([&](const ValueFlow::Value& value) {
41474148
if (!value.isIntValue())

‎lib/valueflow.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,15 @@ namespace ValueFlow {
6060

6161
std::string eitherTheConditionIsRedundant(const Token *condition);
6262

63-
size_t getSizeOf(const ValueType &vt, const Settings &settings, int maxRecursion = 0);
63+
enum class Accuracy {
64+
ExcactOrZero,
65+
LowerBound,
66+
};
67+
68+
size_t getSizeOf(const ValueType &vt,
69+
const Settings &settings,
70+
Accuracy accuracy,
71+
int maxRecursion = 0);
6472

6573
const Value* findValue(const std::list<Value>& values,
6674
const Settings& settings,

‎lib/vf_analyzers.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,9 @@ struct ValueFlowAnalyzer : Analyzer {
351351
/* Truncate value */
352352
const ValueType *dst = tok->valueType();
353353
if (dst) {
354-
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
354+
const size_t sz = ValueFlow::getSizeOf(*dst,
355+
settings,
356+
ValueFlow::Accuracy::ExcactOrZero);
355357
if (sz > 0 && sz < sizeof(MathLib::biguint)) {
356358
MathLib::bigint newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign);
357359

‎lib/vf_common.cpp‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ namespace ValueFlow
114114
{
115115
const ValueType &valueType = ValueType::parseDecl(typeTok, settings);
116116

117-
return getSizeOf(valueType, settings);
117+
return getSizeOf(valueType, settings, ValueFlow::Accuracy::ExcactOrZero);
118118
}
119119

120120
// Handle various constants..
@@ -124,7 +124,9 @@ namespace ValueFlow
124124
try {
125125
MathLib::bigint signedValue = MathLib::toBigNumber(tok);
126126
const ValueType* vt = tok->valueType();
127-
if (vt && vt->sign == ValueType::UNSIGNED && signedValue < 0 && getSizeOf(*vt, settings) < sizeof(MathLib::bigint)) {
127+
if (vt && vt->sign == ValueType::UNSIGNED && signedValue < 0
128+
&& getSizeOf(*vt, settings, ValueFlow::Accuracy::ExcactOrZero)
129+
< sizeof(MathLib::bigint)) {
128130
MathLib::bigint minValue{}, maxValue{};
129131
if (getMinMaxValues(tok->valueType(), settings.platform, minValue, maxValue))
130132
signedValue += maxValue + 1;
@@ -158,7 +160,9 @@ namespace ValueFlow
158160
(tok->next()->astOperand2()->valueType()->pointer == 0 || // <- TODO this is a bailout, abort when there are array->pointer conversions
159161
(tok->next()->astOperand2()->variable() && !tok->next()->astOperand2()->variable()->isArray())) &&
160162
!tok->next()->astOperand2()->valueType()->isEnum()) { // <- TODO this is a bailout, handle enum with non-int types
161-
const size_t sz = getSizeOf(*tok->next()->astOperand2()->valueType(), settings);
163+
const size_t sz = getSizeOf(*tok->next()->astOperand2()->valueType(),
164+
settings,
165+
ValueFlow::Accuracy::ExcactOrZero);
162166
if (sz) {
163167
Value value(sz);
164168
value.setKnown();
@@ -177,7 +181,8 @@ namespace ValueFlow
177181
}
178182
if (Token::simpleMatch(tok, "sizeof ( *")) {
179183
const ValueType *vt = tok->tokAt(2)->valueType();
180-
const size_t sz = vt ? getSizeOf(*vt, settings) : 0;
184+
const size_t sz = vt ? getSizeOf(*vt, settings, ValueFlow::Accuracy::ExcactOrZero)
185+
: 0;
181186
if (sz > 0) {
182187
Value value(sz);
183188
if (!tok2->isTemplateArg() && settings.platform.type != Platform::Type::Unspecified)
@@ -238,7 +243,9 @@ namespace ValueFlow
238243
if (var->type()->classScope && var->type()->classScope->enumType)
239244
size = getSizeOfType(var->type()->classScope->enumType, settings);
240245
} else if (var->valueType()) {
241-
size = getSizeOf(*var->valueType(), settings);
246+
size = getSizeOf(*var->valueType(),
247+
settings,
248+
ValueFlow::Accuracy::ExcactOrZero);
242249
} else if (!var->type()) {
243250
size = getSizeOfType(var->typeStartToken(), settings);
244251
}
@@ -287,7 +294,7 @@ namespace ValueFlow
287294
}
288295
} else if (!tok2->type()) {
289296
const ValueType& vt = ValueType::parseDecl(tok2, settings);
290-
size_t sz = getSizeOf(vt, settings);
297+
size_t sz = getSizeOf(vt, settings, ValueFlow::Accuracy::ExcactOrZero);
291298
const Token* brac = tok2->astParent();
292299
while (Token::simpleMatch(brac, "[")) {
293300
const Token* num = brac->astOperand2();

0 commit comments

Comments
 (0)