From 027e12eeeadb5b696449b8c572707b1135b3217e Mon Sep 17 00:00:00 2001 From: x023845 OLIVIER LEVON Date: Thu, 18 Jan 2018 17:15:57 +0100 Subject: [PATCH] add fixes from Riccardo Ghetta (rglarix, github pull request 25 to 29 https://github.com/KindDragon/vld/pull/29) --- src/callstack.cpp | 18 +++++++-------- src/callstack.h | 8 +++---- src/stdafx.h | 4 ++++ src/utility.cpp | 4 ++-- src/vld.cpp | 56 +++++++++++++++++++++++++++-------------------- src/vld.h | 2 +- 6 files changed, 51 insertions(+), 41 deletions(-) diff --git a/src/callstack.cpp b/src/callstack.cpp index 62d8d281..ba366217 100644 --- a/src/callstack.cpp +++ b/src/callstack.cpp @@ -90,10 +90,10 @@ CallStack::~CallStack () m_resolvedLength = 0; } -CallStack* CallStack::Create() +CallStack* CallStack::Create(BOOL safe_stack_walk) { CallStack* result = NULL; - if (g_vld.GetOptions() & VLD_OPT_SAFE_STACK_WALK) { + if (safe_stack_walk) { result = new SafeCallStack(); } else { @@ -330,10 +330,10 @@ bool CallStack::isCrtStartupAlloc() // // None. // -void CallStack::dump(BOOL showInternalFrames) +void CallStack::dump(BOOL showInternalFrames, BOOL skipStartupLeaks) { if (!m_resolved) { - resolve(showInternalFrames); + resolve(showInternalFrames, skipStartupLeaks); } // The stack was reoslved already @@ -356,7 +356,7 @@ void CallStack::dump(BOOL showInternalFrames) // // None. // -int CallStack::resolve(BOOL showInternalFrames) +int CallStack::resolve(BOOL showInternalFrames, BOOL skipStartupLeaks) { if (m_resolved) { @@ -383,9 +383,7 @@ int CallStack::resolve(BOOL showInternalFrames) IMAGEHLP_LINE64 sourceInfo = { 0 }; sourceInfo.SizeOfStruct = sizeof(IMAGEHLP_LINE64); - bool skipStartupLeaks = !!(g_vld.GetOptions() & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); - - // Use static here to increase performance, and avoid heap allocs. + // Use static here to increase performance, and avoid heap allocs. // It's thread safe because of g_heapMapLock lock. static WCHAR stack_line[MAXREPORTLENGTH + 1] = L""; bool isPrevFrameInternal = false; @@ -467,9 +465,9 @@ int CallStack::resolve(BOOL showInternalFrames) return unresolvedFunctionsCount; } -const WCHAR* CallStack::getResolvedCallstack( BOOL showinternalframes ) +const WCHAR* CallStack::getResolvedCallstack( BOOL showinternalframes, BOOL skipStartupLeaks ) { - resolve(showinternalframes); + resolve(showinternalframes, skipStartupLeaks); return m_resolved; } diff --git a/src/callstack.h b/src/callstack.h index 5029b5e2..7500b4f7 100644 --- a/src/callstack.h +++ b/src/callstack.h @@ -80,16 +80,16 @@ class CallStack public: CallStack (); virtual ~CallStack (); - static CallStack* Create(); + static CallStack* Create(BOOL safe_stack_walk); // Public APIs - see each function definition for details. VOID clear (); // Prints the call stack to one of either / or the debug output window and or // a log file. - VOID dump (BOOL showinternalframes); + VOID dump (BOOL showinternalframes, BOOL skipStartupLeaks); // Formats the stack frame into a human readable format, and saves it for later retrieval. - int resolve(BOOL showinternalframes); + int resolve(BOOL showinternalframes, BOOL skipStartupLeaks); // Formats the stack frame into a human readable format, and saves it for later retrieval. - CONST WCHAR* getResolvedCallstack(BOOL showinternalframes); + CONST WCHAR* getResolvedCallstack(BOOL showinternalframes, BOOL skipStartupLeaks); virtual DWORD getHashValue() const = 0; virtual VOID getStackTrace (UINT32 maxdepth, const context_t& context) = 0; bool isCrtStartupAlloc(); diff --git a/src/stdafx.h b/src/stdafx.h index a968c068..8340368e 100644 --- a/src/stdafx.h +++ b/src/stdafx.h @@ -4,6 +4,10 @@ #include #include #include +#include +#include +#pragma comment(lib, "ws2_32.lib") + #if _WIN32_WINNT < 0x0600 // Windows XP or earlier, no GetProcessIdOfThread() #include #endif diff --git a/src/utility.cpp b/src/utility.cpp index 77cc55a4..e7f57b30 100644 --- a/src/utility.cpp +++ b/src/utility.cpp @@ -607,7 +607,7 @@ BOOL PatchImport (HMODULE importmodule, moduleentry_t *patchModule) DbgReport(L"Hook dll \"%S\":\n", strrchr(pszBuffer, '\\') + 1); } - DbgReport(L"Import found %zu(\"%S\") for dll \"%S\".\n", + DbgReport(L"Import found %Iu(\"%S\") for dll \"%S\".\n", importname, patchModule->exportModuleName, importdllname); break; } @@ -878,7 +878,7 @@ VOID RestoreImport (HMODULE importmodule, moduleentry_t* module) DbgReport(L"UnHook dll \"%S\" import %S!%S()\n", strrchr(pszBuffer, '\\') + 1, module->exportModuleName, importname); } else { - DbgReport(L"UnHook dll \"%S\" import %S!%zu()\n", + DbgReport(L"UnHook dll \"%S\" import %S!%Iu()\n", strrchr(pszBuffer, '\\') + 1, module->exportModuleName, importname); } #endif diff --git a/src/vld.cpp b/src/vld.cpp index 6fd9fd62..26c2aa18 100644 --- a/src/vld.cpp +++ b/src/vld.cpp @@ -350,6 +350,7 @@ VisualLeakDetector::VisualLeakDetector () m_reportFile = NULL; wcsncpy_s(m_reportFilePath, MAX_PATH, VLD_DEFAULT_REPORT_FILE_NAME, _TRUNCATE); m_status = 0x0; + m_optionsLock.Initialize(); HMODULE ntdll = GetModuleHandleW(L"ntdll.dll"); if (ntdll) @@ -420,7 +421,6 @@ VisualLeakDetector::VisualLeakDetector () m_curAlloc = 0; m_maxAlloc = 0; m_loadedModules = new ModuleSet(); - m_optionsLock.Initialize(); m_modulesLock.Initialize(); m_selfTestFile = __FILE__; m_selfTestLine = 0; @@ -1293,10 +1293,22 @@ SIZE_T VisualLeakDetector::eraseDuplicates (const BlockMap::Iterator &element, S // tls_t* VisualLeakDetector::getTls () { + // save winsock last error because TlsGetValue resets it + int wsaerrpre = WSAGetLastError(); + + // save last error because TlsGetValue resets it + DWORD errpre = GetLastError(); + // Get the pointer to this thread's thread local storage structure. tls_t* tls = (tls_t*)TlsGetValue(m_tlsIndex); assert(GetLastError() == ERROR_SUCCESS); + // restore previous state of error + SetLastError(errpre); + + // restore previous state of ws error + WSASetLastError(wsaerrpre); + if (tls == NULL) { DWORD threadId = GetCurrentThreadId(); @@ -1346,8 +1358,6 @@ tls_t* VisualLeakDetector::getTls () // VOID VisualLeakDetector::mapBlock (HANDLE heap, LPCVOID mem, SIZE_T size, bool debugcrtalloc, bool ucrt, DWORD threadId, blockinfo_t* &pblockInfo) { - CriticalSectionLocker<> cs(g_heapMapLock); - // Record the block's information. blockinfo_t* blockinfo = new blockinfo_t(); blockinfo->callStack = NULL; @@ -1472,15 +1482,15 @@ VOID VisualLeakDetector::unmapBlock (HANDLE heap, LPCVOID mem, const context_t & Report(L"---------- Block %Iu at " ADDRESSFORMAT L": %Iu bytes ----------\n", alloc_block->serialNumber, mem, alloc_block->size); Report(L" TID: %u\n", alloc_block->threadId); Report(L" Call Stack:\n"); - alloc_block->callStack->dump(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES); + alloc_block->callStack->dump(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); // Now we need a way to print the current callstack at this point: - CallStack* stack_here = CallStack::Create(); + CallStack* stack_here = CallStack::Create(m_options & VLD_OPT_SAFE_STACK_WALK); stack_here->getStackTrace(m_maxTraceFrames, context); Report(L"Deallocation Call stack.\n"); Report(L"---------- Block %Iu at " ADDRESSFORMAT L": %Iu bytes ----------\n", alloc_block->serialNumber, mem, alloc_block->size); Report(L" Call Stack:\n"); - stack_here->dump(FALSE); + stack_here->dump(FALSE, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); // Now it should be safe to delete our temporary callstack delete stack_here; stack_here = NULL; @@ -1562,8 +1572,6 @@ VOID VisualLeakDetector::unmapHeap (HANDLE heap) VOID VisualLeakDetector::remapBlock (HANDLE heap, LPCVOID mem, LPCVOID newmem, SIZE_T size, bool debugcrtalloc, bool ucrt, DWORD threadId, blockinfo_t* &pblockInfo, const context_t &context) { - CriticalSectionLocker<> cs(g_heapMapLock); - if (newmem != mem) { // The block was not reallocated in-place. Instead the old block was // freed and a new block allocated to satisfy the new size. @@ -1913,7 +1921,7 @@ SIZE_T VisualLeakDetector::reportLeaks (heapinfo_t* heapinfo, bool &firstLeak, S else Report(L" Call Stack:\n"); if (info->callStack) - info->callStack->dump(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES); + info->callStack->dump(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); // Dump the data in the user data section of the memory block. if (m_maxDataDump != 0) { @@ -2360,6 +2368,8 @@ SIZE_T VisualLeakDetector::GetLeaksCount() return 0; } + LoaderLock ll; // scanning for module names needs ldrloc - getting it proactively to avoid deadlocks later + SIZE_T leaksCount = 0; // Generate a memory leak report for each heap in the process. CriticalSectionLocker<> cs(g_heapMapLock); @@ -2709,6 +2719,7 @@ void VisualLeakDetector::SetReportOptions(UINT32 option_mask, CONST WCHAR *filen else if ( m_reportFile ) { //Close the previous report file if needed. fclose(m_reportFile); m_reportFile = NULL; + SetReportFile(m_reportFile, m_options & VLD_OPT_REPORT_TO_DEBUGGER, m_options & VLD_OPT_REPORT_TO_STDOUT); } } @@ -2747,22 +2758,16 @@ void VisualLeakDetector::setupReporting() // Unicode data encoding has been enabled. Write the byte-order // mark before anything else gets written to the file. Open the // file for binary writing. - if (_wfopen_s(&m_reportFile, m_reportFilePath, L"wb") == EINVAL) { - // Couldn't open the file. - m_reportFile = NULL; - } - else if (m_reportFile) { + m_reportFile = _wfsopen(m_reportFilePath, L"wb", _SH_DENYWR); + if (m_reportFile) { fwrite(&bom, sizeof(WCHAR), 1, m_reportFile); SetReportEncoding(unicode); } } else { // Open the file in text mode for ASCII output. - if (_wfopen_s(&m_reportFile, m_reportFilePath, L"w") == EINVAL) { - // Couldn't open the file. - m_reportFile = NULL; - } - else if (m_reportFile) { + m_reportFile = _wfsopen(m_reportFilePath, L"w", _SH_DENYWR); + if (m_reportFile) { SetReportEncoding(ascii); } } @@ -2818,9 +2823,9 @@ const wchar_t* VisualLeakDetector::GetAllocationResolveResults(void* alloc, BOOL blockinfo_t* info = getAllocationBlockInfo(alloc); if (info != NULL) { - int unresolvedFunctionsCount = info->callStack->resolve(showInternalFrames); + int unresolvedFunctionsCount = info->callStack->resolve(showInternalFrames, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); _ASSERT(unresolvedFunctionsCount == 0); - return info->callStack->getResolvedCallstack(showInternalFrames); + return info->callStack->getResolvedCallstack(showInternalFrames, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); } return NULL; } @@ -2870,7 +2875,7 @@ int VisualLeakDetector::resolveStacks(heapinfo_t* heapinfo) // Dump the call stack. if (info->callStack) { - unresolvedFunctionsCount += info->callStack->resolve(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES); + unresolvedFunctionsCount += info->callStack->resolve(m_options & VLD_OPT_TRACE_INTERNAL_FRAMES, m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS); if ((m_options & VLD_OPT_SKIP_CRTSTARTUP_LEAKS) && info->callStack->isCrtStartupAlloc()) { info->reported = true; continue; @@ -2925,6 +2930,11 @@ CaptureContext::~CaptureContext() { return; if ((m_tls->blockWithoutGuard) && (!IsExcludedModule())) { + CallStack* callstack = CallStack::Create(g_vld.m_options & VLD_OPT_SAFE_STACK_WALK); + callstack->getStackTrace(g_vld.m_maxTraceFrames, m_tls->context); + + CriticalSectionLocker<> cs(g_heapMapLock); + blockinfo_t* pblockInfo = NULL; if (m_tls->newBlockWithoutGuard == NULL) { g_vld.mapBlock(m_tls->heap, @@ -2946,8 +2956,6 @@ CaptureContext::~CaptureContext() { pblockInfo, m_tls->context); } - CallStack* callstack = CallStack::Create(); - callstack->getStackTrace(g_vld.m_maxTraceFrames, m_tls->context); pblockInfo->callStack.reset(callstack); } diff --git a/src/vld.h b/src/vld.h index 8047b948..0310ac54 100644 --- a/src/vld.h +++ b/src/vld.h @@ -341,7 +341,7 @@ __declspec(dllexport) int VLDResolveCallstacks(); #define VLDGetOptions() (0) #define VLDGetReportFilename(a) #define VLDSetOptions(a, b, c) -#define VLDSetReportHook(a, b) +#define VLDSetReportHook(a, b) (-1) #define VLDSetModulesList(a) #define VLDGetModulesList(a, b) (FALSE) #define VLDSetReportOptions(a, b)