From 08396e94aa1cef3561fb5a4e2c14dcdd0e4d8bde Mon Sep 17 00:00:00 2001 From: edgchen1 <18449977+edgchen1@users.noreply.github.com> Date: Thu, 24 Apr 2025 18:38:04 -0700 Subject: [PATCH 1/4] Fix memleakdbg call stack output. --- .../core/platform/windows/debug_alloc.cc | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/onnxruntime/core/platform/windows/debug_alloc.cc b/onnxruntime/core/platform/windows/debug_alloc.cc index f3520b4f7f7f5..ad9bc380289e4 100644 --- a/onnxruntime/core/platform/windows/debug_alloc.cc +++ b/onnxruntime/core/platform/windows/debug_alloc.cc @@ -75,45 +75,43 @@ struct SymbolHelper { SymbolHelper() = default; - static constexpr size_t kInitialBufferSize = sizeof(SYMBOL_INFO) + MAX_SYM_NAME; + static constexpr size_t kSymbolBufferSize = sizeof(SYMBOL_INFO) + MAX_SYM_NAME; - bool LoookupSymAndInitialize(const ULONG_PTR address, char* buffer, size_t buffer_size, SYMBOL_INFO* symbol) { - if (SymFromAddr(process_handle_, address, 0, symbol) != TRUE) { + bool LookupSymAndInitialize(const void* address, SYMBOL_INFO* symbol, std::ostream& message) { + if (SymFromAddr(process_handle_, reinterpret_cast(address), 0, symbol) != TRUE) { if (GetLastError() == ERROR_INVALID_HANDLE) { // Try to initialize first - if (!InitializeWhenNeeded() || SymFromAddr(process_handle_, address, 0, symbol) != TRUE) { - _snprintf_s(buffer, buffer_size, _TRUNCATE, "0x%08IX (Unknown symbol)", address); + if (!InitializeWhenNeeded() || + SymFromAddr(process_handle_, reinterpret_cast(address), 0, symbol) != TRUE) { + message << "0x" << address << " (Unknown symbol)"; return false; } } else { - _snprintf_s(buffer, buffer_size, _TRUNCATE, "0x%08IX (Unknown symbol)", address); + message << "0x" << address << " (Unknown symbol)"; return false; } } return true; } - void Lookup(std::string& string, const ULONG_PTR address) { - alignas(SYMBOL_INFO) char buffer[kInitialBufferSize] = {0}; - SYMBOL_INFO* symbol = reinterpret_cast(buffer); + void Lookup(const void* address, std::ostream& message) { + alignas(SYMBOL_INFO) char symbol_buffer[kSymbolBufferSize] = {0}; + SYMBOL_INFO* symbol = reinterpret_cast(symbol_buffer); symbol->SizeOfStruct = sizeof(SYMBOL_INFO); symbol->MaxNameLen = MAX_SYM_NAME; - if (!LoookupSymAndInitialize(address, buffer, kInitialBufferSize, symbol)) { - string.append(buffer); + if (!LookupSymAndInitialize(address, symbol, message)) { return; } Line line; DWORD displacement; - if (SymGetLineFromAddr(process_handle_, address, &displacement, &line) == false) { - _snprintf_s(buffer, _TRUNCATE, "(unknown file & line number): %s", symbol->Name); - string.append(buffer); + if (SymGetLineFromAddr(process_handle_, reinterpret_cast(address), &displacement, &line) == false) { + message << "(unknown file & line number): " << symbol->Name; return; } - _snprintf_s(buffer, _TRUNCATE, "%s(%d): %s", line.FileName, static_cast(line.LineNumber), symbol->Name); - string.append(buffer); + message << line.FileName << "(" << line.LineNumber << "): " << symbol->Name; } struct Line : IMAGEHLP_LINE { @@ -221,17 +219,17 @@ Memory_LeakCheck::~Memory_LeakCheck() { const MemoryBlock& block = *static_cast(entry.lpData); const BYTE* pBlock = static_cast(entry.lpData) + sizeof(MemoryBlock); - std::string string; - char buffer[1024]; - _snprintf_s(buffer, _TRUNCATE, "%Iu bytes at location 0x%08IX\n", entry.cbData - sizeof(MemoryBlock), - UINT_PTR(pBlock)); - string.append(buffer); + std::ostringstream message; + message << (entry.cbData - sizeof(MemoryBlock)) << " bytes at location 0x" << static_cast(pBlock) + << "\n"; for (auto& p : block.m_pTraces) { if (!p) break; - symbols.Lookup(string, reinterpret_cast(p)); - string.push_back('\n'); + symbols.Lookup(p, message); + message << "\n"; } + const std::string string = message.str(); + // Google test has memory leaks that they haven't fixed. One such issue is tracked here: https://github.com/google/googletest/issues/692 // // In gtest-port.cc in function: static ThreadIdToThreadLocals* GetThreadLocalsMapLocked() @@ -271,12 +269,8 @@ Memory_LeakCheck::~Memory_LeakCheck() { if (leaked_bytes) { DebugPrint("-----Ending Heap Trace-----\n\n"); - std::string string; - char buffer[1024]; - _snprintf_s(buffer, _TRUNCATE, "%d bytes of memory leaked in %d allocations", static_cast(leaked_bytes), static_cast(leak_count)); - string.append(buffer); - - std::cout << "\n----- MEMORY LEAKS: " << string.c_str() << "\n"; + std::cout << "\n----- MEMORY LEAKS: " << leaked_bytes << " bytes of memory leaked in " + << leak_count << " allocations" << "\n"; if (!IsDebuggerPresent()) { exit(-1); } From 02763702b8595fe9cfb83d4d407474cdc8de72d0 Mon Sep 17 00:00:00 2001 From: Edward Chen <18449977+edgchen1@users.noreply.github.com> Date: Thu, 24 Apr 2025 18:49:31 -0700 Subject: [PATCH 2/4] Update onnxruntime/core/platform/windows/debug_alloc.cc --- onnxruntime/core/platform/windows/debug_alloc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onnxruntime/core/platform/windows/debug_alloc.cc b/onnxruntime/core/platform/windows/debug_alloc.cc index ad9bc380289e4..aba70cfa07738 100644 --- a/onnxruntime/core/platform/windows/debug_alloc.cc +++ b/onnxruntime/core/platform/windows/debug_alloc.cc @@ -270,7 +270,7 @@ Memory_LeakCheck::~Memory_LeakCheck() { DebugPrint("-----Ending Heap Trace-----\n\n"); std::cout << "\n----- MEMORY LEAKS: " << leaked_bytes << " bytes of memory leaked in " - << leak_count << " allocations" << "\n"; + << leak_count << " allocations\n"; if (!IsDebuggerPresent()) { exit(-1); } From 9d3159bcb40144e6c7c9673a8ac6c129939c95ba Mon Sep 17 00:00:00 2001 From: edgchen1 <18449977+edgchen1@users.noreply.github.com> Date: Fri, 25 Apr 2025 09:52:29 -0700 Subject: [PATCH 3/4] use SYMBOL_INFO_PACKAGE instead of char buffer --- onnxruntime/core/platform/windows/debug_alloc.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/onnxruntime/core/platform/windows/debug_alloc.cc b/onnxruntime/core/platform/windows/debug_alloc.cc index ad9bc380289e4..6458815bee56d 100644 --- a/onnxruntime/core/platform/windows/debug_alloc.cc +++ b/onnxruntime/core/platform/windows/debug_alloc.cc @@ -95,10 +95,10 @@ struct SymbolHelper { } void Lookup(const void* address, std::ostream& message) { - alignas(SYMBOL_INFO) char symbol_buffer[kSymbolBufferSize] = {0}; - SYMBOL_INFO* symbol = reinterpret_cast(symbol_buffer); + SYMBOL_INFO_PACKAGE symbol_info_package{}; + SYMBOL_INFO* symbol = &symbol_info_package.si; symbol->SizeOfStruct = sizeof(SYMBOL_INFO); - symbol->MaxNameLen = MAX_SYM_NAME; + symbol->MaxNameLen = std::size(symbol_info_package.name); if (!LookupSymAndInitialize(address, symbol, message)) { return; From c7934a7ab973d631c404f15a838b1e7afe1e7e76 Mon Sep 17 00:00:00 2001 From: edgchen1 <18449977+edgchen1@users.noreply.github.com> Date: Fri, 25 Apr 2025 09:55:29 -0700 Subject: [PATCH 4/4] remove unused variable --- onnxruntime/core/platform/windows/debug_alloc.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/onnxruntime/core/platform/windows/debug_alloc.cc b/onnxruntime/core/platform/windows/debug_alloc.cc index 9076bf9aaec84..fed61854860f0 100644 --- a/onnxruntime/core/platform/windows/debug_alloc.cc +++ b/onnxruntime/core/platform/windows/debug_alloc.cc @@ -75,8 +75,6 @@ struct SymbolHelper { SymbolHelper() = default; - static constexpr size_t kSymbolBufferSize = sizeof(SYMBOL_INFO) + MAX_SYM_NAME; - bool LookupSymAndInitialize(const void* address, SYMBOL_INFO* symbol, std::ostream& message) { if (SymFromAddr(process_handle_, reinterpret_cast(address), 0, symbol) != TRUE) { if (GetLastError() == ERROR_INVALID_HANDLE) {