Skip to content

Commit 3181f13

Browse files
autoantwortLeander Schulten
autoantwort
and
Leander Schulten
authored
ValueFlow: don't reset the struct size if there is a unknown member + empty structs have a size of 1 (danmar#7560)
Fixes https://trac.cppcheck.net/ticket/13696 + danmar#6257 (comment) One could argue that the size of a member of an unknown type should be 1 instead of 0 because the member will be at least 1 byte large (if not empty and annotated with `[[no_unique_address]]`) --------- Co-authored-by: Leander Schulten <[email protected]>
1 parent e006e02 commit 3181f13

File tree

12 files changed

+127
-55
lines changed

12 files changed

+127
-55
lines changed

‎lib/checkclass.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3402,7 +3402,9 @@ void CheckClass::checkReturnByReference()
34023402
const bool isView = isContainer && var->valueType()->container->view;
34033403
bool warn = isContainer && !isView;
34043404
if (!warn && !isView) {
3405-
const std::size_t size = ValueFlow::getSizeOf(*var->valueType(), *mSettings);
3405+
const std::size_t size = ValueFlow::getSizeOf(*var->valueType(),
3406+
*mSettings,
3407+
ValueFlow::Accuracy::LowerBound);
34063408
if (size > 2 * mSettings->platform.sizeof_pointer)
34073409
warn = true;
34083410
}

‎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::ExactOrZero);
328+
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt,
329+
settings,
330+
ValueFlow::Accuracy::ExactOrZero);
327331
if (!(sizeSrc > 0 && sizeTgt > 0 && sizeSrc < sizeTgt))
328332
return false;
329333

‎lib/library.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
937937
const char *argattr = argnode->Attribute("arg");
938938
if (!argattr)
939939
return Error(ErrorCode::MISSING_ATTRIBUTE, "arg");
940-
if (strlen(argattr) != 1 || argattr[0]<'0' || argattr[0]>'9')
940+
if (strlen(argattr) != 1 || argattr[0]<'0' || argattr[0]> '9')
941941
return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, argattr);
942942

943943
ac.minsizes.reserve(type == ArgumentChecks::MinSize::Type::MUL ? 2 : 1);
@@ -946,7 +946,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
946946
const char *arg2attr = argnode->Attribute("arg2");
947947
if (!arg2attr)
948948
return Error(ErrorCode::MISSING_ATTRIBUTE, "arg2");
949-
if (strlen(arg2attr) != 1 || arg2attr[0]<'0' || arg2attr[0]>'9')
949+
if (strlen(arg2attr) != 1 || arg2attr[0]<'0' || arg2attr[0]> '9')
950950
return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, arg2attr);
951951
ac.minsizes.back().arg2 = arg2attr[0] - '0';
952952
}

‎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::ExactOrZero);
788788
}
789789
return getStrArraySize(tok) * sizeofType;
790790
}

‎lib/valueflow.cpp‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,16 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
424424
result.path = value1.path;
425425
}
426426

427+
namespace {
428+
struct Result
429+
{
430+
size_t total;
431+
bool success;
432+
};
433+
}
427434

428435
template<class F>
429-
static size_t accumulateStructMembers(const Scope* scope, F f)
436+
static Result accumulateStructMembers(const Scope* scope, F f, ValueFlow::Accuracy accuracy)
430437
{
431438
size_t total = 0;
432439
std::set<const Scope*> anonScopes;
@@ -435,7 +442,7 @@ static size_t accumulateStructMembers(const Scope* scope, F f)
435442
continue;
436443
if (const ValueType* vt = var.valueType()) {
437444
if (vt->type == ValueType::Type::RECORD && vt->typeScope == scope)
438-
return 0;
445+
return {0, false};
439446
const MathLib::bigint dim = std::accumulate(var.dimensions().cbegin(), var.dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension& dim) {
440447
return i1 * dim.num;
441448
});
@@ -447,10 +454,10 @@ static size_t accumulateStructMembers(const Scope* scope, F f)
447454
else
448455
total = f(total, *vt, dim);
449456
}
450-
if (total == 0)
451-
return 0;
457+
if (accuracy == ValueFlow::Accuracy::ExactOrZero && total == 0)
458+
return {0, false};
452459
}
453-
return total;
460+
return {total, true};
454461
}
455462

456463
static size_t bitCeil(size_t x)
@@ -467,37 +474,38 @@ static size_t bitCeil(size_t x)
467474
return x + 1;
468475
}
469476

470-
static size_t getAlignOf(const ValueType& vt, const Settings& settings, int maxRecursion = 0)
477+
static size_t getAlignOf(const ValueType& vt, const Settings& settings, ValueFlow::Accuracy accuracy, int maxRecursion = 0)
471478
{
472479
if (maxRecursion == settings.vfOptions.maxAlignOfRecursion) {
473480
// TODO: add bailout message
474481
return 0;
475482
}
476483
if (vt.pointer || vt.reference != Reference::None || vt.isPrimitive()) {
477-
auto align = ValueFlow::getSizeOf(vt, settings);
484+
auto align = ValueFlow::getSizeOf(vt, settings, accuracy);
478485
return align == 0 ? 0 : bitCeil(align);
479486
}
480487
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
481488
auto accHelper = [&](size_t max, const ValueType& vt2, size_t /*dim*/) {
482-
size_t a = getAlignOf(vt2, settings, ++maxRecursion);
489+
size_t a = getAlignOf(vt2, settings, accuracy, ++maxRecursion);
483490
return std::max(max, a);
484491
};
485-
size_t total = 0;
492+
Result result = accumulateStructMembers(vt.typeScope, accHelper, accuracy);
493+
size_t total = result.total;
486494
if (const Type* dt = vt.typeScope->definedType) {
487495
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
488496
if (bi.type && bi.type->classScope)
489-
v += accumulateStructMembers(bi.type->classScope, accHelper);
497+
v += accumulateStructMembers(bi.type->classScope, accHelper, accuracy).total;
490498
return v;
491499
});
492500
}
493-
return total + accumulateStructMembers(vt.typeScope, accHelper);
501+
return result.success ? std::max<size_t>(1, total) : total;
494502
}
495503
if (vt.type == ValueType::Type::CONTAINER)
496504
return settings.platform.sizeof_pointer; // Just guess
497505
return 0;
498506
}
499507

500-
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int maxRecursion)
508+
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, Accuracy accuracy, int maxRecursion)
501509
{
502510
if (maxRecursion == settings.vfOptions.maxSizeOfRecursion) {
503511
// TODO: add bailout message
@@ -527,27 +535,29 @@ size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int m
527535
return 3 * settings.platform.sizeof_pointer; // Just guess
528536
if (vt.type == ValueType::Type::RECORD && vt.typeScope) {
529537
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);
538+
size_t n = ValueFlow::getSizeOf(vt2, settings,accuracy, ++maxRecursion);
539+
size_t a = getAlignOf(vt2, settings, accuracy);
532540
if (n == 0 || a == 0)
533-
return 0;
541+
return accuracy == Accuracy::ExactOrZero ? 0 : total;
534542
n *= dim;
535543
size_t padding = (a - (total % a)) % a;
536544
return vt.typeScope->type == ScopeType::eUnion ? std::max(total, n) : total + padding + n;
537545
};
538-
size_t total = accumulateStructMembers(vt.typeScope, accHelper);
546+
Result result = accumulateStructMembers(vt.typeScope, accHelper, accuracy);
547+
size_t total = result.total;
539548
if (const Type* dt = vt.typeScope->definedType) {
540549
total = std::accumulate(dt->derivedFrom.begin(), dt->derivedFrom.end(), total, [&](size_t v, const Type::BaseInfo& bi) {
541550
if (bi.type && bi.type->classScope)
542-
v += accumulateStructMembers(bi.type->classScope, accHelper);
551+
v += accumulateStructMembers(bi.type->classScope, accHelper, accuracy).total;
543552
return v;
544553
});
545554
}
546-
if (total == 0)
555+
if (accuracy == Accuracy::ExactOrZero && total == 0 && !result.success)
547556
return 0;
548-
size_t align = getAlignOf(vt, settings);
557+
total = std::max(size_t{1}, total);
558+
size_t align = getAlignOf(vt, settings, accuracy);
549559
if (align == 0)
550-
return 0;
560+
return accuracy == Accuracy::ExactOrZero ? 0 : total;
551561
total += (align - (total % align)) % align;
552562
return total;
553563
}
@@ -3610,8 +3620,8 @@ static bool isTruncated(const ValueType* src, const ValueType* dst, const Settin
36103620
if (src->smartPointer && dst->smartPointer)
36113621
return false;
36123622
if ((src->isIntegral() && dst->isIntegral()) || (src->isFloat() && dst->isFloat())) {
3613-
const size_t srcSize = ValueFlow::getSizeOf(*src, settings);
3614-
const size_t dstSize = ValueFlow::getSizeOf(*dst, settings);
3623+
const size_t srcSize = ValueFlow::getSizeOf(*src, settings, ValueFlow::Accuracy::LowerBound);
3624+
const size_t dstSize = ValueFlow::getSizeOf(*dst, settings, ValueFlow::Accuracy::LowerBound);
36153625
if (srcSize > dstSize)
36163626
return true;
36173627
if (srcSize == dstSize && src->sign != dst->sign)
@@ -4134,10 +4144,10 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
41344144
if (!dst || !dst->isIntegral())
41354145
return values;
41364146

4137-
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
4147+
const size_t sz = ValueFlow::getSizeOf(*dst, settings, ValueFlow::Accuracy::ExactOrZero);
41384148

41394149
if (src) {
4140-
const size_t osz = ValueFlow::getSizeOf(*src, settings);
4150+
const size_t osz = ValueFlow::getSizeOf(*src, settings, ValueFlow::Accuracy::ExactOrZero);
41414151
if (osz >= sz && dst->sign == ValueType::Sign::SIGNED && src->sign == ValueType::Sign::UNSIGNED) {
41424152
values.remove_if([&](const ValueFlow::Value& value) {
41434153
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 : std::uint8_t {
64+
ExactOrZero,
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
@@ -355,7 +355,9 @@ struct ValueFlowAnalyzer : Analyzer {
355355
/* Truncate value */
356356
const ValueType *dst = tok->valueType();
357357
if (dst) {
358-
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
358+
const size_t sz = ValueFlow::getSizeOf(*dst,
359+
settings,
360+
ValueFlow::Accuracy::ExactOrZero);
359361
if (sz > 0 && sz < sizeof(MathLib::biguint)) {
360362
MathLib::bigint newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign);
361363

‎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::ExactOrZero);
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::ExactOrZero)
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::ExactOrZero);
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::ExactOrZero)
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::ExactOrZero);
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::ExactOrZero);
291298
const Token* brac = tok2->astParent();
292299
while (Token::simpleMatch(brac, "[")) {
293300
const Token* num = brac->astOperand2();

0 commit comments

Comments
 (0)