deleting an array the wrong way

Question

Possible Duplicate:
How could pairing new[] with delete possibly lead to memory leak only?

I was always told that it's not safe to call delete on an array allocated with new[]. You should always pair new with delete and new[] with delete[].

So I was surprised to discover that the following code compiles and runs ok, in both Debug and Release mode under VS2008.

class CBlah
{
public:
    CBlah() : m_i(0) {}

private:
    int m_i;
};

int _tmain(int argc, _TCHAR* argv[])
{
    for(;;)
    {
        CBlah * p = new CBlah[1000]; // with []
        delete p;                    // no []
    }
    return 0;
}

It took me a while to figure out why this works at all, and I think it's just luck and some undefined behaviour.

BUT... it made me wonder... why doesn't Visual Studio pick this up, at least in the Debug memory manager? Is it because there's lots of code out there that makes this mistake and they don't want to break it, or do they feel it's not the job of the Debug memory manager to catch this kind of mistake?

Any thoughts? Is this kind of misuse common?

Answer 1

It will certainly compile ok, because there is no information in the pointer (compile-time) which will see if pointer points to array or what. For example:

int* p;

cin>>x;
if(x == 0)
  p = new int;
else
  p = new int [10];

delete p; //correct or not? :)

Now , about running ok. This is called undefined behavior in C++, that is, there is no guarantee what will happen - everything can run OK, you can get a segfault, you can get just wrong behavior, or your computer may decide to call 911. UB <=> no guarantee

Answer 2

It's undefined behavior and everything is fair in love, war and undefined behavior...:)

Answer 3

According to MDSN, it translates delete to delete[] when trying to delete an array. (See there, for instance). Though you should have a warning after compiling.

Answer 4

The reason the Debug Memory Manager does not pick up on this error is probably because it it not implemented at the level of new/delete, but at the level of the memory manager that gets invoked by new/delete to allocate the required memory.
At that point, the distinction between array new and scalar new is gone.

Answer 5

You can read these SO answers and links about delete and delete[]: About delete, operator delete, delete[], ...

Answer 6

I don't know what makes you think it "works ok". It compiles and completes without crashing. That does not mean necessarily there was no leak or heap corruption. Also if you got away with it this time, it doesn't necessarily make it a safe thing to do.

Sometimes even a buffer overwrite is something you will "get away with" because the bytes you have written to were not used (maybe they are padding for alignment). Still you should not go around doing it.

Incidentally new T[1] is a form of new[] and still requires a delete[] even though in this instance there is only one element.

Answer 7

Interesting point. Once I did a code review and tried to convince programmers to fix new[]-delete mismatch. I've argumented with "Item 5" from Effective C++ by Scott Meyers. However, they argumented with "What do you want, it works well!" and proved, that there was no memory leakage. However, it worked only with POD-types. Looks like, MS tries to fix the mismatch as pointed out by Raveline.

What would happen, if you added a destructor?

#include <iostream>
class CBlah
{
  static int instCnt;
public:
    CBlah() : m_i(0) {++instCnt;}
    ~CBlah()
    {
      std::cout <<"d-tor on "<< instCnt <<" instance."<<std::endl;
      --instCnt;
    }
private:
    int m_i;
};

int CBlah::instCnt=0;

int main()
{
    //for(;;)
    {
        CBlah * p = new CBlah[10]; // with []
        delete p;                    // no []
    }
    return 0;
}

Whatever silly"inteligence" fix is added to VS, the code is not portable.

Answer 8

Remember that "works properly" is within the universe of "undefined behavior". It is quite possible for a particular version of a particular compiler to implement this in such a way that it works for all intents and purposes. The important thing to remember is that this is not guaranteed and you can't really ever be sure it's working 100%, and you can't know that it will work with the next version of the compiler. It's also not portable, since another compiler might work in a different fashion.

Answer 9

This works because the particular C++ runtime library it was linked with uses the same heap for both operator new and operator new[]. Many do, but some don't, which is why the practice is not recommended.

The other big difference is that if CBlah had a non-trivial destructor, the delete p; would only call it for the first object in the array, whereas delete[] p; is sure to call it for all the objects.