search

KindDragon / vld

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

Use CaptureContext class to simplify code in hooks #13

Closed ioannis-e closed 8 years ago

ioannis-e commented 8 years ago

Document special case in vld_unload test Enable capturing vector_new... in msvcrtPatch Skip stack trace for frames internal to vld Use CaptureContext class to simplify code in hooks

KindDragon commented 8 years ago

:cry: https://ci.appveyor.com/project/KindDragon/vld/build/140/job/4m8viq3lfecrlcpf/tests I think that this bug not always reproduced. I'll try to rebuild it https://ci.appveyor.com/project/KindDragon/vld/build/141 , I don't know how fix it

KindDragon commented 8 years ago

https://ci.appveyor.com/project/KindDragon/vld/build/146/job/vc1g5ofkc7ee05ut a lot of messages "VLD: New allocation at already allocated address: 0x00E09700 with size: 164 and new size: 164"

ioannis-e commented 8 years ago

I think the root of the failure with the following builds is that the debug symbols for mfc120ud.dll are not loaded for some reason, and therefore the frame pointers are not resolved properly. https://ci.appveyor.com/project/KindDragon/vld/build/146 https://ci.appveyor.com/project/KindDragon/vld/build/147

If you take a look at the following call stack, taken from the above builds, before the call to MSVCR120D.dll!initterm_e() there should be a call to _CRT_INIT which we would normally catch and ignore.

Never the less skipping startup CRT allocations depends on all the debug symbols being loaded, so we should investigate why these symbols were not loaded in the first place.

[00:00:49] ---------- Block 6635 at 0x00DF2038: 3722304989 bytes ----------
[00:00:49]   Leak Hash: 0x64A52A3C, Count: 1, Total 3722304989 bytes
[00:00:49]   Call Stack (TID 628):
[00:00:49]     MSVCR120D.dll!malloc_dbg()
[00:00:49]     mfc120ud.dll!0x712CE2AB()
[00:00:49]     MSVCR120D.dll!initterm_e() + 0x28 bytes
[00:00:49]     mfc120ud.dll!0x712CE3C6()
[00:00:49]     mfc120ud.dll!0x712CE6DC()
[00:00:49]     mfc120ud.dll!0x712CE61F()
[00:00:49]     c:\projects\vld\src\vld.cpp (99): vld_x86.dll!LdrpCallInitRoutine() + 0xC bytes
[00:00:49]     ntdll.dll!RtlInitializeCriticalSection() + 0x10E bytes
[00:00:49]     ntdll.dll!RtlInitializeCriticalSection() + 0x88 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x2A5 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x1ED bytes
[00:00:49]     ntdll.dll!RtlGetVersion() + 0x7C0 bytes
[00:00:49]     ntdll.dll!RtlIsCriticalSectionLockedByThread() + 0x155 bytes
[00:00:49]     ntdll.dll!LdrResFindResourceDirectory() + 0x210 bytes
[00:00:49]     ntdll.dll!LdrLoadDll() + 0x6F bytes
[00:00:49]     KERNELBASE.dll!LoadLibraryExW() + 0xC6 bytes
[00:00:49]     KERNEL32.DLL!LoadLibraryW() + 0x12 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\loadtests.cpp (86): dynamic_app.exe!RunMFCLoaderTests() + 0xD bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (27): dynamic_app.exe!Call_LoaderLocks() + 0xC bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (44): dynamic_app.exe!Call_Three() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (49): dynamic_app.exe!Call_Two() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (54): dynamic_app.exe!Call_One() + 0x9 bytes
[00:00:49]     c:\projects\vld\src\tests\dynamic_app\threadtest.cpp (60): dynamic_app.exe!Dynamic_Thread_Procedure() + 0x9 bytes
[00:00:49]     MSVCR120D.dll!beginthreadex() + 0x1A1 bytes
[00:00:49]     MSVCR120D.dll!endthreadex() + 0x181 bytes
[00:00:49]     KERNEL32.DLL!BaseThreadInitThunk() + 0x24 bytes
[00:00:49]     ntdll.dll!RtlInitializeExceptionChain() + 0x8F bytes
[00:00:49]     ntdll.dll!RtlInitializeExceptionChain() + 0x5A bytes
[00:00:49]   Data:
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD DD DD     ........ ........
[00:00:49]     DD DD DD DD    DD DD DD DD    DD DD DD DD    DD DD 99 7E     ........ .......~
[00:00:49]     49 1B 5A 59    73 01 00 8C    A8 B0 E1 00    08 19 DD 00     I.ZYs... ........
[00:00:49]     88 DA D7 70    9C 00 00 00    40 00 00 00    01 00 00 00     ...p.... @.......
[00:00:49]     BD 22 00 00    FD FD FD FD    A0 D7 35 71    17 00 00 00     ."...... ..5q....
[00:00:49]     17 00 00 00    01 00 00 00    34 00 20 00    70 00 61 00     ........ 4...p.a.
[00:00:49]     72 00 61 00    6D 00 65 00    74 00 65 00    72 00 20 00     r.a.m.e. t.e.r...
[00:00:49]     6E 00 65 00    77 00 20 00    66 00 6F 00    72 00 20 00     n.e.w... f.o.r...
[00:00:49]     6D 00 66 00    63 00 00 00    FD FD FD FD    DD DD DD DD     m.f.c... ........
[00:00:49]     47 1B 68 59    DD 02 00 8C    F8 B1 E1 00    C8 15 DD 00     G.hY.... ........
[00:00:49]     88 DA D7 70    9C 00 00 00    40 00 00 00    01 00 00 00     ...p.... @.......
[00:00:49]     71 22 00 00    FD FD FD FD    A0 D7 35 71    17 00 00 00     q"...... ..5q....
[00:00:49]     17 00 00 00    01 00 00 00    34 00 20 00    70 00 61 00     ........ 4...p.a.
KindDragon commented 8 years ago

Hmm, appveyor always reproduce this bug with VldStackWalkMethod=fast, Toolset=v120_xp

ioannis-e commented 8 years ago

Maybe instead of file paths we should test for function names that are called within startup crt and make allocations ? I'll make some tests to see if we get better results.

Did you manage to understand why these messages arise? "VLD: New allocation at already allocated address: 0x00E09700 with size: 164 and new size: 164"

KindDragon commented 8 years ago

I will try reproduce it locally

KindDragon commented 8 years ago 
VLD: New allocation at already allocated address: 0x013CA708 with size: 164 and new size: 164
  TID: 12604
  Call Stack:
    f:\dd\vctools\crt\crtw32\misc\dbgheap.c (159): MSVCR120D.dll!_malloc_dbg()
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (163): mfc120ud.dll!pre_c_init() + 0x17 bytes
    f:\dd\vctools\crt\crtw32\startup\crt0dat.c (1005): MSVCR120D.dll!_initterm_e() + 0x7 bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (287): mfc120ud.dll!_CRT_INIT() + 0xF bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (502): mfc120ud.dll!__DllMainCRTStartup() + 0x11 bytes
    f:\dd\vctools\crt\crtw32\dllstuff\crtdll.c (472): mfc120ud.dll!_DllMainCRTStartup() + 0x11 bytes
    d:\work\vld\src\vld.cpp (99): vld_x86.dll!LdrpCallInitRoutine() + 0xC bytes
    ntdll.dll!LdrxCallInitRoutine() + 0x16 bytes
    ntdll.dll!LdrpCallInitRoutine() + 0x43 bytes
    ntdll.dll!LdrpInitializeNode() + 0x101 bytes
    ntdll.dll!LdrpInitializeGraphRecurse() + 0x71 bytes
    ntdll.dll!LdrpInitializeGraphRecurse() + 0x88 bytes
    ntdll.dll!LdrpPrepareModuleForExecution() + 0x8B bytes
    ntdll.dll!LdrpLoadDllInternal() + 0x121 bytes
    ntdll.dll!LdrpLoadDll() + 0x92 bytes
    ntdll.dll!LdrLoadDll() + 0xD9 bytes
    KERNELBASE.dll!LoadLibraryExW() + 0x138 bytes
    KERNELBASE.dll!LoadLibraryW() + 0x11 bytes
    d:\work\vld\src\tests\dynamic_app\loadtests.cpp (86): dynamic_app.exe!RunMFCLoaderTests() + 0xD bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (27): dynamic_app.exe!Call_LoaderLocks() + 0xC bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (44): dynamic_app.exe!Call_Three() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (49): dynamic_app.exe!Call_Two() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (54): dynamic_app.exe!Call_One() + 0x9 bytes
    d:\work\vld\src\tests\dynamic_app\threadtest.cpp (60): dynamic_app.exe!Dynamic_Thread_Procedure() + 0x9 bytes
    f:\dd\vctools\crt\crtw32\startup\threadex.c (376): MSVCR120D.dll!_callthreadstartex() + 0xF bytes
    f:\dd\vctools\crt\crtw32\startup\threadex.c (359): MSVCR120D.dll!_threadstartex()
    KERNEL32.DLL!BaseThreadInitThunk() + 0x24 bytes
    ntdll.dll!__RtlUserThreadStart() + 0x2F bytes
    ntdll.dll!_RtlUserThreadStart() + 0x1B bytes
KindDragon commented 8 years ago

I think it's bug in v120 CRT

ioannis-e commented 8 years ago

I'm trying some chages in 6e23df9f3020ec23d34e4826e441a3736f3f62e1 Waiting for appyeor results

KindDragon commented 8 years ago

One of two :smile:

ioannis-e commented 8 years ago

:+1:

ioannis-e commented 8 years ago

Will you release the RC now, or are there any more pending matters ?

KindDragon commented 8 years ago

We have new false positive leaks in VS2015 :cry: https://gist.github.com/akaStiX/fde381ec7e29a7264b9b

ioannis-e commented 8 years ago

Can you include a test case for https://gist.github.com/akaStiX/fde381ec7e29a7264b9b maybe in vld_main ?

I think adding || beginWith(functionName, len, L"std::_Crt_new_delete::operator new") in CallStack::isCrtStartupFunction should do the trick, but without a test case, i wouldn't know how it affects all configurations

KindDragon commented 8 years ago

Can you include a test case for https://gist.github.com/akaStiX/fde381ec7e29a7264b9b maybe in vld_main ?

I can't reproduce it ATM in vld tests :cry:

ioannis-e commented 8 years ago

Try in vld_main the following code

#include <iostream>
int Test()
{
    std::cout << "cout";
    std::cerr << "cerr";
    ...
}
ioannis-e commented 8 years ago

Does VS2015 std library dynamically initialize any other objects appart from the ones below ?

std::`dynamic initializer for 'cout''
std::`dynamic initializer for 'fout''
std::`dynamic initializer for 'cerr''
std::`dynamic initializer for 'ferr''

We could simply use the following to exclude all std dynamic initializations || beginWith(functionName, len, L"std::dynamic initializer for '")`

ioannis-e commented 8 years ago

Tested the above and it seems to work perfectly :) Do you want to make the change ?

KindDragon commented 8 years ago

Do you want to make the change ?

I'll do later

KindDragon commented 8 years ago

Try in vld_main the following code

Maybe I'm doing something wrong, but I can't reproduce it.

ioannis-e commented 8 years ago

VS2015, v140, Release_StaticCrt

KindDragon commented 8 years ago

This commit 1ad587e should fix issue

ioannis-e commented 8 years ago

I was working on cleaning up 3 commits https://github.com/ioannis-e/vld/commits/develop Can you take a look ?

KindDragon commented 8 years ago

Changes from 120d6220f3df8746c5e002c09a05417ee0c7d88e no longer needed

ioannis-e commented 8 years ago

I'd suggest changes in bool CallStack::isCrtStartupAlloc() and int CallStack::resolve(BOOL showInternalFrames) they eliminate some control paths In any case cherry pick what you like

ioannis-e commented 8 years ago

Does 120d6220f3df8746c5e002c09a05417ee0c7d88e resolve this ? Note that this is with VS2013 with the code posted above.

---------- Block 67 at 0x00701770: 24 bytes ----------
  Leak Hash: 0x8320BEEF, Count: 1, Total 24 bytes
  Call Stack (TID 1252):
    ntdll.dll!RtlAllocateHeap()
    f:\dd\vctools\crt\crtw32\heap\malloc.c (92): vld_main.exe!malloc() + 0x3B bytes
    f:\dd\vctools\crt\crtw32\heap\new.cpp (59): vld_main.exe!operator new() + 0x8 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\xlocale (2473): vld_main.exe!std::ctype<char>::_Getcat() + 0x7 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\xlocale (578): vld_main.exe!std::use_facet<std::ctype<char> >() + 0xC bytes
    f:\dd\vctools\crt\crtw32\stdhpp\ios (127): vld_main.exe!std::basic_ios<char,std::char_traits<char> >::widen() + 0x13 bytes
    f:\dd\vctools\crt\crtw32\stdhpp\ios (172): vld_main.exe!std::basic_ios<char,std::char_traits<char> >::init()
    f:\dd\vctools\crt\crtw32\stdhpp\ostream (56): vld_main.exe!std::basic_ostream<char,std::char_traits<char> >::basic_ostream<char,std::char_traits<char> >()
    f:\dd\vctools\crt\crtw32\stdcpp\cout.cpp (16): vld_main.exe!std::`dynamic initializer for 'cout''() + 0x13 bytes
    f:\dd\vctools\crt\crtw32\startup\crt0dat.c (321): vld_main.exe!_cinit()
    f:\dd\vctools\crt\crtw32\startup\crt0.c (237): vld_main.exe!__tmainCRTStartup() + 0x7 bytes
    kernel32.dll!BaseThreadInitThunk() + 0xE bytes
    ntdll.dll!__RtlUserThreadStart() + 0x70 bytes
    ntdll.dll!_RtlUserThreadStart() + 0x1B bytes
  Data:
    F0 F2 F6 00    01 00 00 00    00 00 00 00    80 19 70 00     ........ ......p.
    01 00 00 00    00 00 00 00                                   ........ ........
KindDragon commented 8 years ago

Does 120d622 resolve this ? Note that this is with VS2013 with the code posted above.

I'll try your code again

ioannis-e commented 8 years ago

no need to include both cout and cerr. Just try cout, as cerr may interfere with appveyor

KindDragon commented 8 years ago

It's only in Release with StaticCrt https://ci.appveyor.com/project/KindDragon/vld/build/161

ioannis-e commented 8 years ago

Yes the fix with ucrt is only for debug mode! Maybe we should rename the function to properly reflect what is being handled ie isReleaseStaticCrtStartupFunction hehe

As i thought appveyor reports cerr to its console in red

vld_main.exe : cerr
At line:43 char:9
+         & $_.FullName "--gtest_output=`"xml:$testfile`""
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (cerr:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
KindDragon commented 8 years ago

@ioannis-e gitter chat to simplify discussion on issues https://gitter.im/KindDragon/vld