Jump to content
  • Advertisement
Sign in to follow this  
sammyjojo

Why is delete [] causing heap corruption?

This topic is 4447 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.

Share this post


Link to post
Share on other sites
Advertisement
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.

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
This is probably not that important, but for portability sake you should change the code like this:

// allocate a temporary buffer
int bufferSize = bitmapPtr->infoHeader.biSizeImage / sizeof(unsigned int);// <= not 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;

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!