EDN Admin
Well-known member
Hi! Ive recently come upon a design bug in VC++2012 runtime library.
The situation: Overrunning a CRT debug heap block by more than 4 bytes.
The problem: When more than 4 bytes are overrun, an ambiguous breakpoint is triggered instead of a detailed message(buffer was overrun at the end of ...).
Example code to duplicate this issue: /**
* By overrunning only the no-mans land after the buffer
* The Windows heap isnt actually corrupted, so it works correctly.
*/
char* mem1 = (char*)malloc(24);
memset(mem1 + 24, 0, 4); // overrun 4 bytes (size of no-mans land)
free(mem1); // correct assert reported: buffer overrun at the end of buffer
/**
* By overrunning more than 4 bytes, we actually corrupt the
* Windows heap and cause Win32 HeapValidate to fail:
*/
char* mem2 = (char*)malloc(24);
memset(mem2 + 24, 0, 5); // overrun 5 bytes (only 1 byte beyond no-mans land)
free(mem2); // ambigous breakpoint is triggered with no info what happened
Since the VC++ team has been very helpful by actually including the source code of the C runtime library, it has been easy to find the source of the problem:// dbgheap.c : line 1322
/*
* If this ASSERT fails, a bad pointer has been passed in. It may be
* totally bogus, or it may have been allocated from another heap.
* The pointer MUST come from the local heap.
*/
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
/* get a pointer to memory block header */
pHead = pHdr(pUserData);
/* verify block type */
_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));
/* if we didnt already check entire heap, at least check this object */
if (!(_crtDbgFlag & _CRTDBG_CHECK_ALWAYS_DF))
{
/* check no-mans-land gaps */
}
The point that triggers the ambiguous breakpoint is _CrtIsValidHeapPointer, which basically just calls: HeapValidate(_crtheap, 0, pHdr(pUserData)), which validates the current memory block. Of course this Win32 call will trigger an interrupt, since the next heap block has been corrupted by the overrun. As describe my MSDN documentation onHeapValidate, the interrupt is triggered if a debugger is attached.
I can also see the reasoning to validate the pointer: To figure out if the pointer actually belongs in the heap. If someone calls free on a totally bogus pointer, you need a way to catch that. On the contrary though, it doesnt actually display any real usable information of what went wrong even with that.
Since the following line generates an assertion anyways, the program will crash anyways (!).
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
The solution: I think it would greatly improve the CRT if that function wouldnt focus on very bogus edge cases that crash anyways. It should focus on the most obvious path (valid heap pointer) first and report an actual useful error message. After those checks, it can run HeapValidate and crash if it likes: /* get a pointer to memory block header */
pHead = pHdr(pUserData);
/* verify block type */
_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));
/* if we didnt already check entire heap, at least check this object */
if (!(_crtDbgFlag & _CRTDBG_CHECK_ALWAYS_DF))
{
/* check no-mans-land gaps */
}
/* catch any other corruption of the heap */
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
This will make the debug heap report a buffer overrun at the end of the memory block and will then proceed to crash. The improvement is that this time an actual message is shown to the programmer.
I hope this bug reaches the VC++ development team and that in future releases the debug code can be improved.
Regards,
Jorma Rebane
View the full article
The situation: Overrunning a CRT debug heap block by more than 4 bytes.
The problem: When more than 4 bytes are overrun, an ambiguous breakpoint is triggered instead of a detailed message(buffer was overrun at the end of ...).
Example code to duplicate this issue: /**
* By overrunning only the no-mans land after the buffer
* The Windows heap isnt actually corrupted, so it works correctly.
*/
char* mem1 = (char*)malloc(24);
memset(mem1 + 24, 0, 4); // overrun 4 bytes (size of no-mans land)
free(mem1); // correct assert reported: buffer overrun at the end of buffer
/**
* By overrunning more than 4 bytes, we actually corrupt the
* Windows heap and cause Win32 HeapValidate to fail:
*/
char* mem2 = (char*)malloc(24);
memset(mem2 + 24, 0, 5); // overrun 5 bytes (only 1 byte beyond no-mans land)
free(mem2); // ambigous breakpoint is triggered with no info what happened
Since the VC++ team has been very helpful by actually including the source code of the C runtime library, it has been easy to find the source of the problem:// dbgheap.c : line 1322
/*
* If this ASSERT fails, a bad pointer has been passed in. It may be
* totally bogus, or it may have been allocated from another heap.
* The pointer MUST come from the local heap.
*/
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
/* get a pointer to memory block header */
pHead = pHdr(pUserData);
/* verify block type */
_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));
/* if we didnt already check entire heap, at least check this object */
if (!(_crtDbgFlag & _CRTDBG_CHECK_ALWAYS_DF))
{
/* check no-mans-land gaps */
}
The point that triggers the ambiguous breakpoint is _CrtIsValidHeapPointer, which basically just calls: HeapValidate(_crtheap, 0, pHdr(pUserData)), which validates the current memory block. Of course this Win32 call will trigger an interrupt, since the next heap block has been corrupted by the overrun. As describe my MSDN documentation onHeapValidate, the interrupt is triggered if a debugger is attached.
I can also see the reasoning to validate the pointer: To figure out if the pointer actually belongs in the heap. If someone calls free on a totally bogus pointer, you need a way to catch that. On the contrary though, it doesnt actually display any real usable information of what went wrong even with that.
Since the following line generates an assertion anyways, the program will crash anyways (!).
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
The solution: I think it would greatly improve the CRT if that function wouldnt focus on very bogus edge cases that crash anyways. It should focus on the most obvious path (valid heap pointer) first and report an actual useful error message. After those checks, it can run HeapValidate and crash if it likes: /* get a pointer to memory block header */
pHead = pHdr(pUserData);
/* verify block type */
_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));
/* if we didnt already check entire heap, at least check this object */
if (!(_crtDbgFlag & _CRTDBG_CHECK_ALWAYS_DF))
{
/* check no-mans-land gaps */
}
/* catch any other corruption of the heap */
_ASSERTE(_CrtIsValidHeapPointer(pUserData));
This will make the debug heap report a buffer overrun at the end of the memory block and will then proceed to crash. The improvement is that this time an actual message is shown to the programmer.
I hope this bug reaches the VC++ development team and that in future releases the debug code can be improved.
Regards,
Jorma Rebane
View the full article
