Skip to content

Commit 40cf3c3

Browse files
Fix #14305 Wrong buffer sizes computed by valueFlowDynamicBufferSize() (#8006)
Co-authored-by: chrchr-github <[email protected]>
1 parent 514dc56 commit 40cf3c3

19 files changed

+278
-280
lines changed

lib/check64bit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static bool is32BitIntegerReturn(const Function* func, const Settings* settings)
4545
if (settings->platform.sizeof_pointer != 8)
4646
return false;
4747
const ValueType* vt = func->arg->valueType();
48-
return vt && vt->pointer == 0 && vt->isIntegral() && vt->typeSize(settings->platform) == 4;
48+
return vt && vt->pointer == 0 && vt->isIntegral() && vt->getSizeOf(*settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer) == 4;
4949
}
5050

5151
static bool isFunctionPointer(const Token* tok)

lib/checkbufferoverrun.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ static int getMinFormatStringOutputLength(const std::vector<const Token*> &param
9696
std::string digits_string;
9797
bool i_d_x_f_found = false;
9898
int parameterLength = 0;
99-
int inputArgNr = formatStringArgNr;
99+
nonneg int inputArgNr = formatStringArgNr;
100100
for (std::size_t i = 1; i + 1 < formatString.length(); ++i) {
101101
if (formatString[i] == '\\') {
102102
if (i < formatString.length() - 1 && formatString[i + 1] == '0')
@@ -229,7 +229,8 @@ static bool getDimensionsEtc(const Token * const arrayToken, const Settings &set
229229
Dimension dim;
230230
dim.known = value->isKnown();
231231
dim.tok = nullptr;
232-
const MathLib::bigint typeSize = array->valueType()->typeSize(settings.platform, array->valueType()->pointer > 1);
232+
const auto sizeOf = array->valueType()->pointer > 1 ? ValueType::SizeOf::Pointer : ValueType::SizeOf::Pointee;
233+
const size_t typeSize = array->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, sizeOf);
233234
if (typeSize == 0)
234235
return false;
235236
dim.num = value->intvalue / typeSize;
@@ -585,7 +586,7 @@ ValueFlow::Value CheckBufferOverrun::getBufferSize(const Token *bufTok) const
585586
if (var->isPointerArray())
586587
v.intvalue = dim * mSettings->platform.sizeof_pointer;
587588
else {
588-
const MathLib::bigint typeSize = bufTok->valueType()->typeSize(mSettings->platform);
589+
const size_t typeSize = bufTok->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
589590
v.intvalue = dim * typeSize;
590591
}
591592

@@ -929,7 +930,7 @@ bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings &settings, const
929930
{
930931
if (!offset)
931932
return false;
932-
if (!argtok->valueType() || argtok->valueType()->typeSize(settings.platform) == 0)
933+
if (!argtok->valueType() || argtok->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee) == 0)
933934
return false;
934935
const Token *indexTok = nullptr;
935936
if (type == 1 && Token::Match(argtok, "%name% [") && argtok->astParent() == argtok->next() && !Token::simpleMatch(argtok->linkAt(1), "] ["))
@@ -942,7 +943,7 @@ bool CheckBufferOverrun::isCtuUnsafeBufferUsage(const Settings &settings, const
942943
return false;
943944
if (!indexTok->hasKnownIntValue())
944945
return false;
945-
offset->value = indexTok->getKnownIntValue() * argtok->valueType()->typeSize(settings.platform);
946+
offset->value = indexTok->getKnownIntValue() * argtok->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
946947
return true;
947948
}
948949

@@ -1102,7 +1103,7 @@ void CheckBufferOverrun::objectIndex()
11021103
continue;
11031104
}
11041105
if (obj->valueType() && var->valueType() && (obj->isCast() || (obj->isCpp() && isCPPCast(obj)) || obj->valueType()->pointer)) { // allow cast to a different type
1105-
const auto varSize = var->valueType()->typeSize(mSettings->platform);
1106+
const auto varSize = var->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
11061107
if (varSize == 0)
11071108
continue;
11081109
if (obj->valueType()->type != var->valueType()->type) {

lib/checkclass.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3459,9 +3459,7 @@ void CheckClass::checkReturnByReference()
34593459
const bool isView = isContainer && var->valueType()->container->view;
34603460
bool warn = isContainer && !isView;
34613461
if (!warn && !isView) {
3462-
const std::size_t size = ValueFlow::getSizeOf(*var->valueType(),
3463-
*mSettings,
3464-
ValueFlow::Accuracy::LowerBound);
3462+
const std::size_t size = var->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
34653463
if (size > 2 * mSettings->platform.sizeof_pointer)
34663464
warn = true;
34673465
}

lib/checkleakautovar.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,12 @@ void CheckLeakAutoVar::leakIfAllocated(const Token *vartok,
11491149
}
11501150
}
11511151

1152+
static bool isSafeCast(const ValueType* vt, const Settings& settings)
1153+
{
1154+
const size_t sizeOf = vt->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
1155+
return sizeOf == 0 || sizeOf >= settings.platform.sizeof_pointer;
1156+
}
1157+
11521158
void CheckLeakAutoVar::ret(const Token *tok, VarInfo &varInfo, const bool isEndOfScope)
11531159
{
11541160
const std::map<int, VarInfo::AllocInfo> &alloctype = varInfo.alloctype;
@@ -1182,8 +1188,7 @@ void CheckLeakAutoVar::ret(const Token *tok, VarInfo &varInfo, const bool isEndO
11821188
while (tok3 && tok3->isCast() &&
11831189
(!tok3->valueType() ||
11841190
tok3->valueType()->pointer ||
1185-
(tok3->valueType()->typeSize(mSettings->platform) == 0) ||
1186-
(tok3->valueType()->typeSize(mSettings->platform) >= mSettings->platform.sizeof_pointer)))
1191+
isSafeCast(tok3->valueType(), *mSettings)))
11871192
tok3 = tok3->astOperand2() ? tok3->astOperand2() : tok3->astOperand1();
11881193
if (tok3 && tok3->varId() == varid)
11891194
tok2 = tok3->next();

lib/checkmemoryleak.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,8 +1071,8 @@ void CheckMemoryLeakNoVar::checkForUnreleasedInputArgument(const Scope *scope)
10711071
const Variable* argvar = tok->function()->getArgumentVar(argnr);
10721072
if (!argvar || !argvar->valueType())
10731073
continue;
1074-
const MathLib::bigint argSize = argvar->valueType()->typeSize(mSettings->platform, /*p*/ true);
1075-
if (argSize <= 0 || argSize >= mSettings->platform.sizeof_pointer)
1074+
const size_t argSize = argvar->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
1075+
if (argSize == 0 || argSize >= mSettings->platform.sizeof_pointer)
10761076
continue;
10771077
}
10781078
functionCallLeak(arg, arg->str(), functionName);

lib/checkother.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1524,7 +1524,7 @@ static bool isLargeContainer(const Variable* var, const Settings& settings)
15241524
return false;
15251525
}
15261526
const ValueType vtElem = ValueType::parseDecl(vt->containerTypeToken, settings);
1527-
const auto elemSize = std::max<std::size_t>(ValueFlow::getSizeOf(vtElem, settings, ValueFlow::Accuracy::LowerBound), 1);
1527+
const auto elemSize = std::max<std::size_t>(vtElem.getSizeOf(settings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer), 1);
15281528
const auto arraySize = var->dimension(0) * elemSize;
15291529
return arraySize > maxByValueSize;
15301530
}
@@ -1564,7 +1564,7 @@ void CheckOther::checkPassByReference()
15641564
// Ensure that it is a large object.
15651565
if (!var->type()->classScope)
15661566
inconclusive = true;
1567-
else if (!var->valueType() || ValueFlow::getSizeOf(*var->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound) <= 2 * mSettings->platform.sizeof_pointer)
1567+
else if (!var->valueType() || var->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer) <= 2 * mSettings->platform.sizeof_pointer)
15681568
continue;
15691569
}
15701570
else
@@ -3327,7 +3327,8 @@ void CheckOther::checkRedundantCopy()
33273327
const Token* varTok = fScope->bodyEnd->tokAt(-2);
33283328
if (varTok->variable() && !varTok->variable()->isGlobal() &&
33293329
(!varTok->variable()->type() || !varTok->variable()->type()->classScope ||
3330-
(varTok->variable()->valueType() && ValueFlow::getSizeOf(*varTok->variable()->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound) > 2 * mSettings->platform.sizeof_pointer)))
3330+
(varTok->variable()->valueType() &&
3331+
varTok->variable()->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer) > 2 * mSettings->platform.sizeof_pointer)))
33313332
redundantCopyError(startTok, startTok->str());
33323333
}
33333334
}
@@ -3447,7 +3448,7 @@ void CheckOther::checkIncompleteArrayFill()
34473448
if (size == 0 && var->valueType()->pointer)
34483449
size = mSettings->platform.sizeof_pointer;
34493450
else if (size == 0 && var->valueType())
3450-
size = ValueFlow::getSizeOf(*var->valueType(), *mSettings, ValueFlow::Accuracy::LowerBound);
3451+
size = var->valueType()->getSizeOf(*mSettings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
34513452
const Token* tok3 = tok->next()->astOperand2()->astOperand1()->astOperand1();
34523453
if ((size != 1 && size != 100 && size != 0) || var->isPointer()) {
34533454
if (printWarning)
@@ -4430,8 +4431,7 @@ static UnionMember parseUnionMember(const Variable &var,
44304431
if (var.isArray()) {
44314432
size = var.dimension(0);
44324433
} else if (vt != nullptr) {
4433-
size = ValueFlow::getSizeOf(*vt, settings,
4434-
ValueFlow::Accuracy::ExactOrZero);
4434+
size = vt->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer);
44354435
}
44364436
return UnionMember(nameToken->str(), size);
44374437
}
@@ -4523,7 +4523,7 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
45234523
bufToken = expr->astOperand1()->astOperand1();
45244524
offsetToken = expr->astOperand1()->astOperand2();
45254525
if (expr->astOperand1()->valueType())
4526-
elementSize = ValueFlow::getSizeOf(*expr->astOperand1()->valueType(), settings, ValueFlow::Accuracy::LowerBound);
4526+
elementSize = expr->astOperand1()->valueType()->getSizeOf(settings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
45274527
} else if (Token::Match(expr, "+|-") && expr->isBinaryOp()) {
45284528
const bool pointer1 = (expr->astOperand1()->valueType() && expr->astOperand1()->valueType()->pointer > 0);
45294529
const bool pointer2 = (expr->astOperand2()->valueType() && expr->astOperand2()->valueType()->pointer > 0);
@@ -4532,13 +4532,13 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
45324532
offsetToken = expr->astOperand2();
45334533
auto vt = *expr->astOperand1()->valueType();
45344534
--vt.pointer;
4535-
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
4535+
elementSize = vt.getSizeOf(settings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
45364536
} else if (!pointer1 && pointer2) {
45374537
bufToken = expr->astOperand2();
45384538
offsetToken = expr->astOperand1();
45394539
auto vt = *expr->astOperand2()->valueType();
45404540
--vt.pointer;
4541-
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
4541+
elementSize = vt.getSizeOf(settings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
45424542
} else {
45434543
return false;
45444544
}
@@ -4547,7 +4547,7 @@ static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigin
45474547
*offset = 0;
45484548
auto vt = *expr->valueType();
45494549
--vt.pointer;
4550-
elementSize = ValueFlow::getSizeOf(vt, settings, ValueFlow::Accuracy::LowerBound);
4550+
elementSize = vt.getSizeOf(settings, ValueType::Accuracy::LowerBound, ValueType::SizeOf::Pointer);
45514551
if (elementSize > 0) {
45524552
*offset *= elementSize;
45534553
if (sizeValue)

lib/checktype.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,8 @@ 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,
326-
settings,
327-
ValueFlow::Accuracy::ExactOrZero);
328-
const std::size_t sizeTgt = ValueFlow::getSizeOf(tgt,
329-
settings,
330-
ValueFlow::Accuracy::ExactOrZero);
325+
const std::size_t sizeSrc = src.getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer);
326+
const std::size_t sizeTgt = tgt.getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer);
331327
if (!(sizeSrc > 0 && sizeTgt > 0 && sizeSrc < sizeTgt))
332328
return false;
333329

lib/clangimport.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,16 +1586,16 @@ static void setValues(const Tokenizer &tokenizer, const SymbolDatabase *symbolDa
15861586
return v * dim.num;
15871587
});
15881588
if (var.valueType())
1589-
typeSize += mul * var.valueType()->typeSize(settings.platform, true);
1589+
typeSize += mul * var.valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer);
15901590
}
15911591
scope.definedType->sizeOf = typeSize;
15921592
}
15931593

15941594
for (auto *tok = const_cast<Token*>(tokenizer.tokens()); tok; tok = tok->next()) {
15951595
if (Token::simpleMatch(tok, "sizeof (")) {
15961596
ValueType vt = ValueType::parseDecl(tok->tokAt(2), settings);
1597-
const MathLib::bigint sz = vt.typeSize(settings.platform, true);
1598-
if (sz <= 0)
1597+
const size_t sz = vt.getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointer);
1598+
if (sz == 0)
15991599
continue;
16001600
long long mul = 1;
16011601
for (const Token *arrtok = tok->linkAt(1)->previous(); arrtok; arrtok = arrtok->previous()) {

lib/ctu.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
368368
functionCall.location = FileInfo::Location(tokenizer, tok);
369369
functionCall.callArgNr = argnr + 1;
370370
functionCall.callArgumentExpression = argtok->expressionString();
371-
const auto typeSize = argtok->valueType()->typeSize(tokenizer.getSettings().platform);
371+
const auto typeSize = argtok->valueType()->getSizeOf(tokenizer.getSettings(), ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
372372
functionCall.callArgValue.value = typeSize > 0 ? argtok->variable()->dimension(0) * typeSize : -1;
373373
functionCall.warning = false;
374374
fileInfo->functionCalls.push_back(std::move(functionCall));
@@ -382,7 +382,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer &tokenizer)
382382
functionCall.location = FileInfo::Location(tokenizer, tok);
383383
functionCall.callArgNr = argnr + 1;
384384
functionCall.callArgumentExpression = argtok->expressionString();
385-
functionCall.callArgValue.value = argtok->astOperand1()->valueType()->typeSize(tokenizer.getSettings().platform);
385+
functionCall.callArgValue.value = argtok->astOperand1()->valueType()->getSizeOf(tokenizer.getSettings(), ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
386386
functionCall.warning = false;
387387
fileInfo->functionCalls.push_back(std::move(functionCall));
388388
}

0 commit comments

Comments
 (0)