Back to General and Gameplay Programming

Why is delete [] causing heap corruption?

General and Gameplay Programming Programming

Started by sammyjojo August 11, 2006 11:25 PM

21 comments, last by GameDev.net 17 years, 8 months ago

sammyjojo

122

Author

August 11, 2006 11:25 PM

I can't for the life of me figure this error out and it's driving me crazy, so hopefully someone on here can help me out.


// allocate a temporary buffer
int bufferSize = bitmapPtr->infoHeader.biSizeImage / 4;
unsigned int *tempBuffer = new unsigned int[ bufferSize ];

// read in the buffer, bitmapFile is an ifstream
bitmapFile.read( (char *)tempBuffer, bitmapPtr->infoHeader.biSizeImage );

// this doesn't work
delete [] tempBuffer;

When that code runs, I get this error: HEAP CORRUPTION DETECTED: after Normal block(#134) at 0x00386CA0. CRT detected that the application wrote to memory after end of heap buffer. But I don't get the error with this code...


// allocate a temporary buffer
int bufferSize = bitmapPtr->infoHeader.biSizeImage / 4;
unsigned int *tempBuffer = new unsigned int[ bufferSize ];

// this does work, why!?
delete [] tempBuffer;

// read in the buffer, bitmapFile is an ifstream
bitmapFile.read( (char *)tempBuffer, bitmapPtr->infoHeader.biSizeImage );

What is happening to tempBuffer during the bitmapFile.read to cause it have heap curruption if I try to delete it after that line? Am I'm using the pointer wrong by casting it to a char pointer? The problem isn't directy in this part of the code, but I added the delete there to figure out where my program was messing up. The real problem happens later when I try to delete the memory that I allocated here and I get the crazy heap corruption business :( I figure this little snippet of code is basically the problem. I'm using VS2005, not sure if that helps or not or if I'm missing some other information, but if I forgot something important, tell me and I'll let you know.

mattd

1,078

August 11, 2006 11:34 PM

You a creating a buffer of size bitmapPtr->infoHeader.biSizeImage / 4 bytes.
You are then trying to read in bitmapPtr->infoHeader.biSizeImage bytes from the stream, causing it to write over the end of the buffer, and corrupting your heap.

Solution:
Allocate a buffer of size bitmapPtr->infoHeader.biSizeImage instead.

sammyjojo

122

Author

August 11, 2006 11:57 PM

Quote:Original post by mattd
You a creating a buffer of size bitmapPtr->infoHeader.biSizeImage / 4 bytes.
You are then trying to read in bitmapPtr->infoHeader.biSizeImage bytes from the stream, causing it to write over the end of the buffer, and corrupting your heap.

Solution:
Allocate a buffer of size bitmapPtr->infoHeader.biSizeImage instead.

I'm reading in bitmapPtr->infoHeader.biSizeImage and not bitmapPtr->infoHeader.biSizeImage/4 because the it's a char* in bitmapFile.read(), instead of an unsigned int*. So isn't that what I'm supposed to do?

mattd

1,078

August 12, 2006 12:06 AM

Quote:Original post by sammyjojo
Quote:Original post by mattd
You a creating a buffer of size bitmapPtr->infoHeader.biSizeImage / 4 bytes.
You are then trying to read in bitmapPtr->infoHeader.biSizeImage bytes from the stream, causing it to write over the end of the buffer, and corrupting your heap.

Solution:
Allocate a buffer of size bitmapPtr->infoHeader.biSizeImage instead.

I'm reading in bitmapPtr->infoHeader.biSizeImage and not bitmapPtr->infoHeader.biSizeImage/4 because the it's a char* in bitmapFile.read(), instead of an unsigned int*. So isn't that what I'm supposed to do?

Oops, sorry, you're correct :)

Are you sure that bitmapPtr->infoHeader.biSizeImage is unsigned int aligned, ie, a multiple of 4? If it wasn't, the read call could be trying to write an extra 1-3 bytes that are unaccounted for by the integer division by 4.

BMPs are meant to have a DWORD-aligned pitch, but some BMP writers don't do this.

Fruny

1,658

August 12, 2006 12:08 AM

Why don't you directly do new char[whatever] ?

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan

hplus0603

11,916

August 12, 2006 12:15 AM

Quote:You a creating a buffer of size bitmapPtr->infoHeader.biSizeImage / 4 bytes

Wrong. He's creating biSizeImage/4 ints -- each int is 4 bytes, so it multiplies out, unless biSizeImage is not divisible by 4. You could add one to the number of ints you create to make really sure.

In debug builds, there's a function available called _CrtCheckMemory(), which will stop with a breakpoint if something in the heap is corrupted. It's not super fast, but it's thorough. You might want to scatter calls to _CrtCheckMemory() around your source, so that you can catch problems sooner rather than later -- ideally, call it right before you allocate or delete any memory, plus after every line in pieces of code where you suspect problems.

If you don't have the header, you can just forward declare it (assuming MSVC here):

extern "C" int _CrtCheckMemory();

More expensive tools, such as BoundsChecker or Purify, will catch all your memory writes (and reads) and validate that they are always within a valid block. They do cost money (but may have demo versions).

enum Bool { True, False, FileNotFound };

sammyjojo

122

Author

August 12, 2006 01:17 AM

Quote:Original post by Fruny
Why don't you directly do new char[whatever] ?

I could just have the pointer be char* instead of unsigned int*, but I'm just wondering what I'm doing wrong. In my mind the code that I posted that doesn't work... should work, but it for some reason it doesn't and I don't understand why. The main reason I'm using unsigned ints is because I'm working with a 32-bit pixels.

Fruny

1,658

August 12, 2006 01:33 AM

Compare the address at which the corruption happens with the address of the beginning and the end of the array (i.e; print them out). That might give us a hint.

Paulius Maruska

566

August 12, 2006 01:39 AM

This is probably not that important, but for portability sake you should change the code like this:

// allocate a temporary bufferint bufferSize = bitmapPtr->infoHeader.biSizeImage / sizeof(unsigned int);// <= not 4unsigned int *tempBuffer = new unsigned int[ bufferSize ];// read in the buffer, bitmapFile is an ifstreambitmapFile.read( (char *)tempBuffer, bitmapPtr->infoHeader.biSizeImage );// this doesn't workdelete [] tempBuffer;

Enigma

1,410

August 12, 2006 06:28 AM

I can see two possible issues with the original code (besides the portability issue noted elsewhere):

As already mentioned, bitmapPtr->infoHeader.biSizeImage may not be a multiple of four, in which case the division truncates and (bitmapPtr->infoHeader.biSizeImage / 4) * 4 < bitmapPtr->infoHeader.biSizeImage. To test for this print out the value of bitmapPtr->infoHeader.biSizeImage.

bitmapPtr could be a dangling pointer such that bitmapPtr->infoHeader.biSizeImage happens to point to the same memory location as tempBuffer and so gets overwritten. To test for this print out the addresses of bitmapPtr->infoHeader.biSizeImage and tempBuffer.

In addition you should use an RAII wrapper like std::vector or boost::scoped_array to ensure exception safety, even in the face of future changes.

Σnigma

Why is delete [] causing heap corruption?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Why is delete [] causing heap corruption?

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines