Using dynamically-allocated arrays causes C6386 Buffer Overrun warning from Code Analysis

Question

When I use a dynamically-allocated array in C++ I get the C6386 warning issued from the Visual Studio 2019 Code Analysis.

Here is a minimal reproducible example:

#include <vector>

int main()
{
    std::vector<int> someInts{ 1, 2, 3 };

    int* dynamicInts = new int[someInts.size()];

    for (size_t i = 0; i < someInts.size(); i++)
    {
        dynamicInts[i] = someInts[i];
    }

    delete[] dynamicInts;
}

The intent here is that we have a vector of ints and we want to copy them to a dynamically-allocated array of ints for some reason. (Slightly silly example but it shows the problem). It uses the size() of the vector during allocation of the dynamicInts array and as the upper bound of the loop, so there should be no issue with buffer overrun that I can see.

But this code generates:

main.cpp(11): warning C6386: Buffer overrun while writing to 'dynamicInts':  the writable size is 'someInts.public: unsigned int __thiscall std::vector<int,class std::allocator<int> >::size(void)const ()*4' bytes, but '8' bytes might be written.

The Detailed Explanation in the Error List explains this as:

7: 'dynamicInts' is an array of 1 elements (4 bytes)
9: Enter this loop. (assume 'i<someInts.size()')
9: 'i' may equal 1
9: Continue this loop. (assume 'i<someInts.size()')
11: 'i' is an In/Out argument to 'std::vector<int,std::allocator<int> >::[]'
11: Invalid to write to 'dynamicInts[1]'. (writable range is 0 to 0)

These messages suggest that the analyser thinks the dynamicsInts array is only one element.

That seems odd. I know "Code Analysis Is Hard", but surely the analyser has all the information it needs right there to derive the array length and loop bound?

So is this just a false negative from the code analysis? Or am I missing something?

If it is a false negative then what's the best way to avoid this warning without adding suppressing and assuming that we need to stick with dynamically-allocated arrays?

GrahamS GrahamS · Accepted Answer · 2020-07-12T19:51:14

Interestingly I can make the warning go away by introducing a pointless intermediate variable for the size of the array, like this:

#include <vector>

int main()
{
    std::vector<int> someInts{ 1, 2, 3 };
    size_t numberOfInts = someInts.size();

    int* dynamicInts = new int[numberOfInts];

    for (size_t i = 0; i < numberOfInts; i++)
    {
        dynamicInts[i] = someInts[i];
    }

    delete[] dynamicInts;
}

I have no idea why the Code Analyser likes this, but doesn't like the original code.

The Analyser is also happy if I use a smart pointer to hold the dynamic allocation, which is a bit neater and probably the way it should have been written in the first place.

#include <vector>
#include <memory>

int main()
{
    std::vector<int> someInts{ 1, 2, 3 };

    std::unique_ptr<int[]> dynamicInts(new int[someInts.size()]);

    for (size_t i = 0; i < someInts.size(); i++)
    {
        dynamicInts[i] = someInts[i];
    }
}

Using dynamically-allocated arrays causes C6386 Buffer Overrun warning from Code Analysis

1 Answers