From 3cec88bd12708fc56d4a56e95a5398b15a2956b4 Mon Sep 17 00:00:00 2001 From: Changming Sun Date: Fri, 18 Aug 2023 17:10:33 -0700 Subject: [PATCH] FIX: memory leak checker is incompatible with std::stacktrace (#17209) ### Description When I worked on PR #17173, I didn't notice that onnxruntime\core\platform\windows\debug_alloc.cc also needs to call dbghelp functions like SymInitialize. So, if we use vc runtime's stacktrace functionality, vc runtime will initialize/uninitialize the dbghelp library independently and vc runtime's stacktrace helper DLLs get unloaded before our memory leak checker starts get work. Then we call SymSetOptions, it crashes. More details: In VC runtime the C++23 stacktrace functions are implemented on top of dbgeng.dll. In C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\crt\src\stl\stacktrace.cpp, you can see it has: ``` dbgeng = LoadLibraryExW(L"dbgeng.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32); ``` The dbgeng.dll is a wrapper around dbghelp.dll. It calls SymInitialize and SymCleanup. dbgeng.dll gets unloaded before our memory leak check starts to run. In theory we should be able to call SymInitialize again if the previous user who called SymInitialize has also called SymCleanup. However, users can use SymRegisterCallback/SymRegisterCallback64/SymRegisterCallbackW64 to register callback functions to dbghelp.dll. These callback functions need to be alive when SymSetOptions(and some other dbghelp APIs) get called. ### Motivation and Context --- cmake/onnxruntime_common.cmake | 6 ++++-- onnxruntime/core/platform/windows/stacktrace.cc | 9 +++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/cmake/onnxruntime_common.cmake b/cmake/onnxruntime_common.cmake index 906f0271c1..687a231e05 100644 --- a/cmake/onnxruntime_common.cmake +++ b/cmake/onnxruntime_common.cmake @@ -87,8 +87,10 @@ source_group(TREE ${REPO_ROOT} FILES ${onnxruntime_common_src}) onnxruntime_add_static_library(onnxruntime_common ${onnxruntime_common_src}) if(WIN32) - set_property(TARGET onnxruntime_common PROPERTY CXX_STANDARD 23) - target_compile_options(onnxruntime_common PRIVATE "/Zc:char8_t-") + if("cxx_std_23" IN_LIST CMAKE_CXX_COMPILE_FEATURES) + set_property(TARGET onnxruntime_common PROPERTY CXX_STANDARD 23) + target_compile_options(onnxruntime_common PRIVATE "/Zc:char8_t-") + endif() endif() if (onnxruntime_USE_TELEMETRY) set_target_properties(onnxruntime_common PROPERTIES COMPILE_FLAGS "/FI${ONNXRUNTIME_INCLUDE_DIR}/core/platform/windows/TraceLoggingConfigPrivate.h") diff --git a/onnxruntime/core/platform/windows/stacktrace.cc b/onnxruntime/core/platform/windows/stacktrace.cc index 2def304638..cac6f4f290 100644 --- a/onnxruntime/core/platform/windows/stacktrace.cc +++ b/onnxruntime/core/platform/windows/stacktrace.cc @@ -5,6 +5,11 @@ #include #include #include +#ifdef __has_include +#if __has_include() +#include +#endif +#endif #include #include "core/common/logging/logging.h" @@ -27,7 +32,7 @@ class CaptureStackTrace { std::vector GetStackTrace() { #ifndef NDEBUG // TVM need to run with shared CRT, so won't work with debug helper now -#if !(defined _OPSCHEMA_LIB_) && !(defined _GAMING_XBOX) +#if (defined __cpp_lib_stacktrace) && !(defined _OPSCHEMA_LIB_) && !(defined _GAMING_XBOX) && !(defined ONNXRUNTIME_ENABLE_MEMLEAK_CHECK) return detail::CaptureStackTrace().Trace(); #else return {}; @@ -39,7 +44,7 @@ std::vector GetStackTrace() { namespace detail { #ifndef NDEBUG -#if !(defined _OPSCHEMA_LIB_) && !(defined _GAMING_XBOX) +#if (defined __cpp_lib_stacktrace) && !(defined _OPSCHEMA_LIB_) && !(defined _GAMING_XBOX) && !(defined ONNXRUNTIME_ENABLE_MEMLEAK_CHECK) std::vector CaptureStackTrace::Trace() const { std::vector stacktrace;