Skip to content

Commit 47b4555

Browse files
author
Your Name
committed
Handle mulple size on dst
1 parent e3c596a commit 47b4555

2 files changed

Lines changed: 95 additions & 16 deletions

File tree

lib/checkstl.cpp

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3478,6 +3478,54 @@ static ElementCount getAvailableSpace(const IteratorPosition& position)
34783478
return available;
34793479
}
34803480

3481+
static bool isPossibleValue(const ValueFlow::Value* value)
3482+
{
3483+
return value->isPossible();
3484+
}
3485+
3486+
// Find iterator values and paired container sizes of the iterator that prove accessing <accessed>
3487+
// elements to be out of bounds, preferring a proof that does not rely on possible values
3488+
static ElementCount findInsufficientSpace(const Token* tok,
3489+
MathLib::bigint accessed,
3490+
MathLib::bigint sourcePath,
3491+
const Settings& settings)
3492+
{
3493+
ElementCount best;
3494+
bool bestIsCertain = false;
3495+
if (!tok)
3496+
return best;
3497+
const auto consider = [&](const ElementCount& candidate) {
3498+
if (!candidate)
3499+
return;
3500+
if (candidate.count < 0 || accessed <= candidate.count)
3501+
return; // the space is sufficient
3502+
const bool certain = std::none_of(candidate.values.cbegin(), candidate.values.cend(), isPossibleValue);
3503+
if (best && (bestIsCertain || !certain))
3504+
return;
3505+
best = candidate;
3506+
bestIsCertain = certain;
3507+
};
3508+
for (const ValueFlow::Value& value : tok->values()) {
3509+
if (!isUsableValue(value, settings) || !value.isIteratorValue())
3510+
continue;
3511+
if (value.path != 0 && sourcePath != 0 && value.path != sourcePath)
3512+
continue;
3513+
IteratorPosition position;
3514+
position.value = &value;
3515+
if (position.fromEnd()) { // the available space does not depend on the container size
3516+
consider(getAvailableSpace(position));
3517+
continue;
3518+
}
3519+
for (const ValueFlow::Value& sizeValue : tok->values()) {
3520+
if (!isUsableValue(sizeValue, settings) || !sizeValue.isContainerSizeValue() || sizeValue.path != value.path)
3521+
continue;
3522+
position.sizeValue = &sizeValue;
3523+
consider(getAvailableSpace(position));
3524+
}
3525+
}
3526+
return best;
3527+
}
3528+
34813529
// Get the number of accessed elements of a count-based algorithm such as std::fill_n
34823530
static const ValueFlow::Value* getCountValue(const Token* tok, const Settings& settings)
34833531
{
@@ -3550,24 +3598,14 @@ void CheckStlImpl::algorithmOutOfBounds()
35503598
}
35513599
if (accessed.count <= 0)
35523600
continue;
3601+
const MathLib::bigint sourcePath = accessed.values.front()->path;
35533602
for (const std::size_t argnr : iterArgs) {
3554-
const IteratorPosition dest = getIteratorPosition(args[argnr], mSettings);
3555-
if (!dest)
3556-
continue;
3557-
const MathLib::bigint sourcePath = accessed.values.front()->path;
3558-
if (dest.value->path != 0 && sourcePath != 0 && dest.value->path != sourcePath)
3559-
continue;
3560-
const ElementCount available = getAvailableSpace(dest);
3603+
const ElementCount available = findInsufficientSpace(args[argnr], accessed.count, sourcePath, mSettings);
35613604
if (!available)
35623605
continue;
3563-
if (available.count < 0 || accessed.count <= available.count)
3564-
continue;
3565-
const auto isPossible = [](const ValueFlow::Value* value) {
3566-
return value->isPossible();
3567-
};
35683606
// do not warn when the values on both sides are only possible
3569-
if (std::any_of(accessed.values.cbegin(), accessed.values.cend(), isPossible) &&
3570-
std::any_of(available.values.cbegin(), available.values.cend(), isPossible))
3607+
if (std::any_of(accessed.values.cbegin(), accessed.values.cend(), isPossibleValue) &&
3608+
std::any_of(available.values.cbegin(), available.values.cend(), isPossibleValue))
35713609
continue;
35723610
const ValueFlow::Value* conditionValue = nullptr;
35733611
bool inconclusiveValues = false;
@@ -3580,8 +3618,7 @@ void CheckStlImpl::algorithmOutOfBounds()
35803618
}
35813619
if (conditionValue && !mSettings.severity.isEnabled(Severity::warning))
35823620
continue;
3583-
const ValueFlow::Value* pathValue =
3584-
conditionValue ? conditionValue : (dest.sizeValue ? dest.sizeValue : dest.value);
3621+
const ValueFlow::Value* pathValue = conditionValue ? conditionValue : available.values.back();
35853622
algorithmOutOfBoundsError(args[argnr],
35863623
"std::" + nameTok->str(),
35873624
accessed.count,

test/teststl.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2674,6 +2674,48 @@ class TestStl : public TestFixture {
26742674
" std::fill_n(v.begin(), n, 0);\n"
26752675
"}\n");
26762676
ASSERT_EQUALS("", errout_str());
2677+
2678+
// all container size values are checked, not only the first one
2679+
check("void f(bool b) {\n"
2680+
" const std::vector<int> v0{1,2,3,4,5};\n"
2681+
" std::vector<int> v1;\n"
2682+
" if (b)\n"
2683+
" v1.resize(3);\n"
2684+
" else\n"
2685+
" v1.resize(10);\n"
2686+
" std::copy(v0.begin(), v0.end(), v1.begin());\n"
2687+
"}\n");
2688+
ASSERT_EQUALS("[test.cpp:8:45]: (error) The algorithm 'std::copy' accesses 5 elements through the iterator 'v1.begin()' but only 3 elements are available. [algorithmOutOfBounds]\n",
2689+
errout_str());
2690+
2691+
check("void f(bool b) {\n"
2692+
" const std::vector<int> v0{1,2,3,4,5};\n"
2693+
" std::vector<int> v1;\n"
2694+
" if (b)\n"
2695+
" v1.resize(5);\n"
2696+
" else\n"
2697+
" v1.resize(10);\n"
2698+
" std::copy(v0.begin(), v0.end(), v1.begin());\n"
2699+
"}\n");
2700+
ASSERT_EQUALS("", errout_str());
2701+
2702+
// all iterator position values are checked as well
2703+
check("void f(bool b) {\n"
2704+
" const std::vector<int> v0{1,2,3,4};\n"
2705+
" std::vector<int> v1(5);\n"
2706+
" auto it = b ? v1.begin() : v1.begin() + 3;\n"
2707+
" std::copy(v0.begin(), v0.end(), it);\n"
2708+
"}\n");
2709+
ASSERT_EQUALS("[test.cpp:5:37]: (error) The algorithm 'std::copy' accesses 4 elements through the iterator 'it' but only 2 elements are available. [algorithmOutOfBounds]\n",
2710+
errout_str());
2711+
2712+
// do not combine possible values on both sides
2713+
check("void f(bool b, std::vector<int>& v0, std::vector<int>& v1) {\n"
2714+
" if (b) v0.resize(5); else v0.resize(2);\n"
2715+
" if (b) v1.resize(3); else v1.resize(10);\n"
2716+
" std::copy(v0.begin(), v0.end(), v1.begin());\n"
2717+
"}\n");
2718+
ASSERT_EQUALS("", errout_str());
26772719
}
26782720

26792721
// Dereferencing invalid pointer

0 commit comments

Comments
 (0)