KindDragon / vld

Visual Leak Detector for Visual C++ 2008-2015
https://kinddragon.github.io/vld/
GNU Lesser General Public License v2.1
1.02k stars 314 forks source link

VLD: New allocation at already allocated address from realloc() #45

Open chris19782 opened 6 years ago

chris19782 commented 6 years ago

It's maybe working as intented but then it'll raise it anyway. Found this discussed here https://gitter.im/KindDragon/vld/archives/2016/01/14 but answer link is no longer accessible.

In a multithreaded application calling realloc()/malloc() Thread 1: realloc() -> HeapReAlloc() -> block X is freed for the Heap, will give block Y Thread2: malloc() -> HeapAlloc() -> block X is given back But the trackings done in CaptureContext::~CaptureContext() wrapper will happen later, resulting in "VLD: New allocation at already allocated address" Reported in remapBlock().

This is maybe a potential problem if the following sequence then occurs Thread2: mapBlock X (+ warning) Thread1: unmapBlock X Thread1: mapBlockY

Could then block X reference be lost ?

Note that the sizes might still differ. HeapAlloc() does sometimes give back the same buffer even if sizes are slightly differnts.

As that warning was suspicious and "probably by some mechanism unknown to VLD" comment not encouraging, I added an unmapBlock() before the call in _RtlReAllocateHeap()/_HeapReAlloc() wrappers as already done in _RtlFreeHeap/_HeapFree.

This somehow renders remapBlock() mechanism irrelevant in this scenario.

`LPVOID VisualLeakDetector::_RtlReAllocateHeap (HANDLE heap, DWORD flags, LPVOID mem, SIZE_T size) { PRINT_HOOKED_FUNCTION();

if (!g_DbgHelp.IsLockedByCurrentThread()) // skip dbghelp.dll calls
{
    // Record the current frame pointer.
    CAPTURE_CONTEXT();
    context_.func = reinterpret_cast<UINT_PTR>(RtlReAllocateHeap);

    // Unmap the block from the specified heap.
    g_vld.unmapBlock(heap, mem, context_);
}

 // Reallocate the block.
 LPVOID newmem = RtlReAllocateHeap(heap, flags, mem, size);
if ((newmem == NULL) || !g_vld.enabled())
    return newmem;

if (!g_DbgHelp.IsLockedByCurrentThread()) { // skip dbghelp.dll calls
    CAPTURE_CONTEXT();
    CaptureContext cc(RtlReAllocateHeap, context_);
    cc.Set(heap, mem, newmem, size);
}

return newmem;

}

// for kernel32.dll LPVOID VisualLeakDetector::_HeapReAlloc (HANDLE heap, DWORD flags, LPVOID mem, SIZE_T size) { PRINT_HOOKED_FUNCTION();

if (!g_DbgHelp.IsLockedByCurrentThread()) // skip dbghelp.dll calls
{
    // Record the current frame pointer.
    CAPTURE_CONTEXT();
    context_.func = reinterpret_cast<UINT_PTR>(HeapReAlloc);

    // Unmap the block from the specified heap.
    g_vld.unmapBlock(heap, mem, context_);
}

// Reallocate the block.
LPVOID newmem = HeapReAlloc(heap, flags, mem, size);
if ((newmem == NULL) || !g_vld.enabled())
    return newmem;

if (!g_DbgHelp.IsLockedByCurrentThread()) { // skip dbghelp.dll calls
    CAPTURE_CONTEXT();
    CaptureContext cc(HeapReAlloc, context_);
    cc.Set(heap, mem, newmem, size);
}

return newmem;

}`

I was looking at a heap corruption and this Report warning was my only lead. I found out later that it was https://github.com/KindDragon/vld/pull/26 Thanks @rglarix for your patches