2

ok, I'm fairly certain this is just down to the ever-so-buggy static code analyser in Visual Studio, but I figured more sets of eyes on this wouldn't hurt.

I've written my own collection of vector-style containers. They seem to work ok, but I've noticed the following warnings from the Visual Studio static code analyser:

warning C6386: Buffer overrun: accessing 'argument 1', the writable size is 'newspace*4' bytes, but '4194240' bytes might be written

The above warning occurs on the last line of the following code (the memcpy), and highlights all of the previous lines. All possible types that this particular vector class can hold are POD. mSpace contains the number of items the available memory in the vector can hold, and mSize the current number of items. mDynamic points to the memory block currently being managed by the vector.

Bool Insert( UInt32 index, TYPE value, UInt32 count = 1 )
{
    if( index < mSize )
    {
        UInt32  newsize = mSize + count;

        if( newsize > mSpace )
        {
            UInt32  newspace = std::max( Align( newsize, 8 ), mSpace << 1 );
            TYPE*   ptr = new TYPE[ newspace ];

            if( ptr )
            {
                memcpy( ptr, mDynamic, index * sizeof( TYPE ));
                // The remainder of the code copies the second half of the
                // existing data and inserts the new values...

I've looked at this until my eyes bleed (well, almost) and can't see why the analyser would generate this warning. I've seen the same warning in other places also with the rather strange value of 4194240, which is why I'm assuming it's yet another bug in the analyser.

Can anyone confirm this is a known bug in the analyser (a very quick search of connect didn't provide any results), or is there a blatantly obvious bug in my code that I'm just not seeing?

  • 4194240 is 2^22 - 2^6. Not sure if that's relevant... – Oliver Charlesworth Feb 19 '12 at 13:55
  • Looks like a pretty bogus warning to me... Does the warning only show up when you're instantiating it with a certain type? Like, would it show up if you isolated your class in a project without clients? – Rob I Feb 21 '12 at 15:16
  • And perhaps try running your code through another [tool](http://stackoverflow.com/questions/141498/what-open-source-c-static-analysis-tools-are-available)... – Rob I Feb 21 '12 at 15:17
  • No, it appears for lots of different types. I've tried several other different static code analysis tools and none of them mention anything about this code - another indicator that it's a VS2010 bug. I've also seen the warning appear for completely unrelated pieces of code - again with the magic 4194240 value, so I've pretty much written this off as not my problem. –  Mar 03 '12 at 10:59

1 Answers1

0

Not sure if it's related to the warning, but you do have an error here. If mSize is 2^32 - 1 then newsize will be set to 0 due to the integer overflow, which will end up with a buffer overrun. There's a similar error when you do index * sizeof( TYPE ) without checking that the result will fit within 32 bits.

More minor, but you should really be using size_t rather than UInt32, otherwise if you want to compile to 64-bit you'll have warning about types not matching size memcpy takes a size_t as the 3rd argument, and size_t is 64 bits on 64 bit Windows.

tloach
  • 7,934
  • 1
  • 31
  • 44
  • Actually, I'll run into overflow errors long before mSize hits 2^32-1. (Probably around the time mSize * sizeof( TYPE ) reaches 2^32.) By then though I'll have plenty of other problems, such as there not being any memory left for the code. ;) In reality on Windows XP it's pretty hard to allocate a single piece of memory larger than about 700MB right after your process launches. That number increases somewhat on Vista/7, but it's still nowhere close to the 32-bit address space limit. –  Mar 03 '12 at 11:03
  • As for 64-bit, I've been building on Win64 since the first 64-bit compilers were available from Microsoft. Using size_t is something I avoid whenever I can. I've had to fix too many bugs from other people where they've mixed size_t with int with hilarious consequences (mostly from expecting files and network connections to magically still work across different architectures). Add to that the fact that 64-bit types double memory requirements for no good reason (I don't *need* 64-bit sizes here) and the fact 64-bit instructions are prefixed on AMD64 which also increases code size unnecessarily. –  Mar 03 '12 at 11:13