From 5249b7ab7cee8fc6f58e7a448552aa68e11f7000 Mon Sep 17 00:00:00 2001 From: Changming Sun Date: Wed, 16 Aug 2023 16:07:49 -0700 Subject: [PATCH] Re-implement stacktrace (#17173) ### Description Re-implement stacktrace. The new implementation doesn't directly use Windows API, hence can avoid problems regarding to initialize/uninitialize the dbghelp library. ### Motivation and Context --- cmake/onnxruntime_common.cmake | 5 +- .../platform/EigenNonBlockingThreadPool.h | 2 +- onnxruntime/core/platform/windows/env.h | 2 +- .../core/platform/windows/stacktrace.cc | 102 ++---------------- 4 files changed, 12 insertions(+), 99 deletions(-) diff --git a/cmake/onnxruntime_common.cmake b/cmake/onnxruntime_common.cmake index 5db0556672..906f0271c1 100644 --- a/cmake/onnxruntime_common.cmake +++ b/cmake/onnxruntime_common.cmake @@ -86,7 +86,10 @@ endif() 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-") +endif() if (onnxruntime_USE_TELEMETRY) set_target_properties(onnxruntime_common PROPERTIES COMPILE_FLAGS "/FI${ONNXRUNTIME_INCLUDE_DIR}/core/platform/windows/TraceLoggingConfigPrivate.h") endif() diff --git a/include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h b/include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h index 542b9052d4..a57385f6e2 100644 --- a/include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h +++ b/include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h @@ -757,7 +757,7 @@ class ThreadPoolTempl : public onnxruntime::concurrency::ExtendedThreadPoolInter return v_; } - bool operator==(Tag& other) const { + bool operator==(const Tag& other) const { return v_ == other.v_; } diff --git a/onnxruntime/core/platform/windows/env.h b/onnxruntime/core/platform/windows/env.h index 0df4c2b949..79739db9e5 100644 --- a/onnxruntime/core/platform/windows/env.h +++ b/onnxruntime/core/platform/windows/env.h @@ -51,7 +51,7 @@ class WindowsEnv : public Env { #if defined(_MSC_VER) && !defined(__clang__) #pragma warning(pop) #endif - void WindowsEnv::SleepForMicroseconds(int64_t micros) const override; + void SleepForMicroseconds(int64_t micros) const override; static int DefaultNumCores(); int GetNumPhysicalCpuCores() const override; std::vector GetDefaultThreadAffinities() const override; diff --git a/onnxruntime/core/platform/windows/stacktrace.cc b/onnxruntime/core/platform/windows/stacktrace.cc index 954c3c2a5d..2def304638 100644 --- a/onnxruntime/core/platform/windows/stacktrace.cc +++ b/onnxruntime/core/platform/windows/stacktrace.cc @@ -5,9 +5,7 @@ #include #include #include - -#include -#include +#include #include "core/common/logging/logging.h" #include "core/common/gsl.h" @@ -22,10 +20,6 @@ class CaptureStackTrace { std::vector Trace() const; private: - std::string Lookup(void* address_in) const; - - HANDLE process_ = GetCurrentProcess(); - static const int kCallstackLimit = 64; // Maximum depth of callstack }; } // namespace detail @@ -46,103 +40,19 @@ std::vector GetStackTrace() { namespace detail { #ifndef NDEBUG #if !(defined _OPSCHEMA_LIB_) && !(defined _GAMING_XBOX) -class SymbolHelper { - public: - SymbolHelper() noexcept { - SymSetOptions(SymGetOptions() | SYMOPT_DEFERRED_LOADS); - // this could have been called earlier by a higher level component, so failure doesn't necessarily mean - // this won't work. however we should only call SymCleanup if it was successful. - if (SymInitialize(process_, nullptr, true)) { - cleanup_ = true; - } else { - // Log it so we know it happened. Can't do anything else about it. - LOGS_DEFAULT(WARNING) << "Failed to initialize symbols for providing stack trace. Error: 0x" - << std::hex << GetLastError(); - } - } - - struct Symbol : SYMBOL_INFO { - Symbol() noexcept { - SizeOfStruct = sizeof(SYMBOL_INFO); - GSL_SUPPRESS(bounds .3) - MaxNameLen = _countof(buffer); - } - - char buffer[1024] = {0}; - }; - - struct Line : IMAGEHLP_LINE64 { - Line() noexcept { - SizeOfStruct = sizeof(IMAGEHLP_LINE64); - } - }; - - ~SymbolHelper() { - if (cleanup_) - SymCleanup(process_); - } - - private: - ORT_DISALLOW_COPY_ASSIGNMENT_AND_MOVE(SymbolHelper); - - HANDLE process_ = GetCurrentProcess(); - bool cleanup_ = false; -}; std::vector CaptureStackTrace::Trace() const { -#pragma warning(push) -#pragma warning(disable : 26426) - static SymbolHelper sh; -#pragma warning(pop) - std::vector stacktrace; - - PVOID frames[kCallstackLimit]; - const auto f = gsl::make_span(frames); - const auto num_frames = CaptureStackBackTrace(0, kCallstackLimit, f.data(), nullptr); - - stacktrace.reserve(num_frames); - - // hide CaptureStackTrace::Trace and GetStackTrace so the output starts with the 'real' location - constexpr int frames_to_skip = 2; - - // we generally want to skip the first two frames, but if something weird is going on (e.g. code coverage is - // running) and we only have 1 or 2 frames, output them so there's at least something that may be meaningful - const uint16_t start_frame = num_frames > frames_to_skip ? frames_to_skip : 0; - for (uint16_t i = start_frame; i < num_frames; ++i) { - stacktrace.push_back(Lookup(f[i])); + auto st = std::stacktrace::current(2); + for (const auto& stack : st) { + std::ostringstream oss; + oss << stack.source_file() << "(" << stack.source_line() << "): " << stack.description(); + stacktrace.push_back(oss.str()); } return stacktrace; } -std::string CaptureStackTrace::Lookup(void* address_in) const { - SymbolHelper::Symbol symbol; - std::ostringstream result; - - DWORD64 address = 0; - - GSL_SUPPRESS(type .1) { - address = reinterpret_cast(address_in); - } - - if (SymFromAddr(process_, address, 0, &symbol) == false) { - result << "0x" << std::hex << address << " (Unknown symbol)"; - } else - GSL_SUPPRESS(bounds .3) // symbol.Name converts to char* - { - SymbolHelper::Line line; - DWORD displacement; - if (SymGetLineFromAddr64(process_, address, &displacement, &line) == false) { - result << "???: " << symbol.Name; - } else { - result << line.FileName << '(' << line.LineNumber << "): " << symbol.Name; - } - } - - return result.str(); -} - #endif #endif } // namespace detail