Heap Corruption

[zb0S216C]

As always, I'm messing around with memory. Now, I've run into a heap corruption. To replicate the problem, you can compile this:

#include <iostream>

char *FreeStore( new char[ 10 ] );

char *NewHandle( char *&Store, const unsigned int Blocks )
{
    if( !Store )
        return NULL;
  
    else
    {
        if( ( Blocks >= 1 ) && ( Blocks <= 10 ) )
            return( new( Store ) char[ Blocks ] );

        else return NULL;
    }
}

int main( )
{
    // Initialize all 'FreeStore' elements.
    for( int Index( 0 ); Index < 10; Index++ )
        FreeStore[ Index ] = ( 'a' + Index );

    // Create a new handle.
    char *Handle( NewHandle( FreeStore, 5 ) );

    if( !Handle )
    {
        std::cout << "Allocation failure" << std::endl;
        delete [ ] FreeStore;
        std::cin.get( );
        return 1;
    }

    // Print the contents held by 'Handle'.
    for( int Index( 0 ); Index < 5; Index++ )
        std::cout << "Handle[" << Index << "] = " << Handle[ Index ] << std::endl;

    delete [ ] Handle;
    delete [ ] FreeStore;

    std::cin.get( );
    return 0;
}

It seems as though delete [ ] Handle is causing the corruption, but why? I've released the resources held by Handle. I can't see what's wrong.

Can anybody figure out what's wrong?

_Wazzak

[Disch]

You are deleting the same memory block twice. You only allocate 10 bytes of memory here... on line 3.

Line 13 does not allocate new memory, it uses placement new which just calls the object's constructor to place objects at a previously allocated location.

You can see this by putting this before you delete the buffers:

if(Handle == FreeStore)
  std::cout << "wtf the buffers are the same!!!";

So since you're only allocating memory once, you only delete it once.

[zb0S216C]

OK, so basically Handle is using the memory allocated by FreeStore? If that's the case, can Handle change the amount of resources it's allocating though another call to new[ ]? or would that cause a leak?

_Wazak

[Disch]

OK, so basically Handle is using the memory allocated by FreeStore?

Correct.

can Handle change the amount of resources it's allocating though another call to new[ ]?

You can't change how much FreeStore allocated, but you can allocate separate memory with another call to new, yes:

FreeStore = new char[whatever]; // OK 

or would that cause a leak?

It's only a leak if you don't delete[] it.

[zb0S216C]

Disch wrote:

FreeStore = new char[whatever];

I think you misunderstood me, Disch :) What I meant was this:

char *Handle( NewHandle( FreeStore, 5 ) );
Handle = ( NewHandle( FreeStore, 3 ) );        // Will this cause a leak? 

_Wazzak

[Disch]

No. NewHandle does not allocate any memory, so there's nothing that can leak. You could call it a million times and you wouldn't have any leak.

HOWEVER

It's worth mentioning that what you're doing is only going to work for basic types. If you try to do this with something like a string, you will have leaks and some other problems.

[zb0S216C]

Disch wrote:

If you try to do this with something like a string, you will have leaks and some other problems.

What about simple structures such as this:

struct BASIC
{
    int I;
    char C;
    float *P; // This doesn't allocate memory.
};

You've been a big help to me so far, Disch.

_Wazzak

[Disch]

In my experiments/research to answer this question, I discovered a big problem with what you're doing...



It's like this... there are 3 forms of new[], each have their own coresponding "clean up":

1) new[] (allocate memory + call ctors)
2) operator new (allocate memory only)
3) placement new (call ctors only)


Typically you either just do 1, or you do 2+3. You're doing 1 + a weird version of 3

1) new[]

T* foo = new T[10];  // allocate 10 objects

// appropriate clean up:
delete[] foo;

2) operator new

// allocate space for 10 objects, but do not construct them
T* foo = static_cast<T*>(operator new(sizeof(T) * 10));

// appropriate clean up:
operator delete(foo);

3) placement new

T* foo = some_memory_that_has_been_allocated_but_not_constructed;

for(int i = 0; i < 10; ++i)
  new (foo + i) T();          // call ctors for all 10 objects

// appropriate clean up:
for(int i = 0; i < 10; ++i)
  foo[i].~T();  // explicitly call the dtors 

Note here that I'm using new for placement and not new[]. I tried using new[] on my machine and it got ALL screwed up. It looks like using new[] uses an extra 4 bytes (presumably to record how many objects were constructed) but I'm figuring that's platform dependent, so I wouldn't advise relying on that.

So it's very possible that you are corrupting the heap if you do NewHandle(FreeStore, 10), or even NewHandle(FreeStore, 7)!

[zb0S216C]

I never realized that it was such a big problem. Given the explanation, I think I'll stick to the basic new[ ] operator. It was only a test to see what can be done. Also, in your 3^rd example, it appears as though there's a lot more code than necessary when cleaning up instead of the conventional delete [ ].

Thanks for all your help, Disch, I appreciate it :)

_Wazzak