Skip to content

Commit

Permalink
Make MS Debug engine SymInitialize() called as needed. (#20036)
Browse files Browse the repository at this point in the history
### Description
<!-- Describe your changes. -->
Initialize Symbol engine as needed with no duplicate calls.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
  Currently absel library may call SymInitialize more than once
  when shared libraries are involved. However, this can only be
  called only once per process. Our debug_alloc also may call it
  when enabled. This change enables intialization to proceed
  only when needed with no duplicate effort.
  • Loading branch information
yuslepukhin authored Mar 22, 2024
1 parent f9cddd2 commit 3076b56
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 31 deletions.
98 changes: 86 additions & 12 deletions cmake/patches/abseil/absl_windows.patch
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,91 @@ index a6efc98e..8c4de8e7 100644
"/wd4800",
]
diff --git a/absl/copts/copts.py b/absl/copts/copts.py
index 0d6c1ec3..75fd935f 100644
index e6e11949..0aa7d868 100644
--- a/absl/copts/copts.py
+++ b/absl/copts/copts.py
@@ -132,10 +132,6 @@ COPT_VARS = {
"/wd4068", # unknown pragma
# qualifier applied to function type has no meaning; ignored
"/wd4180",
- # conversion from 'type1' to 'type2', possible loss of data
- "/wd4244",
- # conversion from 'size_t' to 'type', possible loss of data
- "/wd4267",
# The decorated name was longer than the compiler limit
"/wd4503",
# forcing value to bool 'true' or 'false' (performance warning)
@@ -115,10 +115,6 @@ MSVC_WARNING_FLAGS = [
"/wd4068", # unknown pragma
# qualifier applied to function type has no meaning; ignored
"/wd4180",
- # conversion from 'type1' to 'type2', possible loss of data
- "/wd4244",
- # conversion from 'size_t' to 'type', possible loss of data
- "/wd4267",
# The decorated name was longer than the compiler limit
"/wd4503",
# forcing value to bool 'true' or 'false' (performance warning)
diff --git a/absl/debugging/symbolize_win32.inc b/absl/debugging/symbolize_win32.inc
index 53a099a1..34d210d6 100644
--- a/absl/debugging/symbolize_win32.inc
+++ b/absl/debugging/symbolize_win32.inc
@@ -35,15 +35,15 @@ ABSL_NAMESPACE_BEGIN

static HANDLE process = NULL;

-void InitializeSymbolizer(const char*) {
- if (process != nullptr) {
- return;
- }
+namespace {
+void InitializeSymbolizerImpl() {
+
process = GetCurrentProcess();

// Symbols are not loaded until a reference is made requiring the
// symbols be loaded. This is the fastest, most efficient way to use
// the symbol handler.
+
SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_UNDNAME);
if (!SymInitialize(process, nullptr, true)) {
// GetLastError() returns a Win32 DWORD, but we assign to
@@ -54,6 +54,36 @@ void InitializeSymbolizer(const char*) {
}
}

+bool LookupAndInitialize(const void* pc, SYMBOL_INFO* symbol) {
+ auto hProcess = (process != NULL) ? process : GetCurrentProcess();
+ if (SymFromAddr(hProcess, reinterpret_cast<DWORD64>(pc), nullptr, symbol) != TRUE) {
+ if (GetLastError() == ERROR_INVALID_HANDLE && process == NULL) {
+ InitializeSymbolizerImpl();
+ if (SymFromAddr(process, reinterpret_cast<DWORD64>(pc), nullptr, symbol) != TRUE) {
+ return false;
+ }
+ } else {
+ return false;
+ }
+ return false;
+ }
+ return true;
+}
+}
+
+void InitializeSymbolizer(const char*) {
+ if (process != nullptr) {
+ return;
+ }
+
+ alignas(SYMBOL_INFO) char buf[sizeof(SYMBOL_INFO) + MAX_SYM_NAME];
+ SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buf);
+ symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
+ symbol->MaxNameLen = MAX_SYM_NAME;
+
+ static_cast<void>(LookupAndInitialize(reinterpret_cast<const void*>(&InitializeSymbolizer), symbol));
+}
+
bool Symbolize(const void* pc, char* out, int out_size) {
if (out_size <= 0) {
return false;
@@ -62,9 +92,11 @@ bool Symbolize(const void* pc, char* out, int out_size) {
SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buf);
symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
symbol->MaxNameLen = MAX_SYM_NAME;
- if (!SymFromAddr(process, reinterpret_cast<DWORD64>(pc), nullptr, symbol)) {
+
+ if(!LookupAndInitialize(pc, symbol)) {
return false;
}
+
const size_t out_size_t = static_cast<size_t>(out_size);
strncpy(out, symbol->Name, out_size_t);
if (out[out_size_t - 1] != '\0') {
64 changes: 45 additions & 19 deletions onnxruntime/core/platform/windows/debug_alloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,41 +55,67 @@ struct MemoryBlock {
};

struct SymbolHelper {
SymbolHelper() noexcept {
SymSetOptions(SymGetOptions() | SYMOPT_DEFERRED_LOADS);
SymInitialize(GetCurrentProcess(), nullptr, true);
HANDLE process_handle_ = GetCurrentProcess();
bool initialized_ = false;

bool InitializeWhenNeeded() {
// We try only once
if (!initialized_) {
SymSetOptions(SymGetOptions() | SYMOPT_DEFERRED_LOADS);
// We use GetCurrentProcess() because other libs are likely to use it
if (!SymInitialize(process_handle_, nullptr, true)) {
const unsigned long long error{GetLastError()};
std::cerr << "SymInitialize() failed: " << error << std::endl;
return false;
}
initialized_ = true;
}
return true;
}

SymbolHelper() = default;

static constexpr size_t kInitialBufferSize = 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) {
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);
return false;
}
} else {
_snprintf_s(buffer, buffer_size, _TRUNCATE, "0x%08IX (Unknown symbol)", address);
return false;
}
}
return true;
}

void Lookup(std::string& string, const ULONG_PTR address) {
char buffer[2048] = {0};
Symbol symbol;
if (SymFromAddr(GetCurrentProcess(), address, 0, &symbol) == false) {
_snprintf_s(buffer, _TRUNCATE, "0x%08IX (Unknown symbol)", address);
alignas(SYMBOL_INFO) char buffer[kInitialBufferSize] = {0};
SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buffer);
symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
symbol->MaxNameLen = MAX_SYM_NAME;

if (!LoookupSymAndInitialize(address, buffer, kInitialBufferSize, symbol)) {
string.append(buffer);
return;
}

Line line;
DWORD displacement;
if (SymGetLineFromAddr(GetCurrentProcess(), address, &displacement, &line) == false) {
_snprintf_s(buffer, _TRUNCATE, "(unknown file & line number): %s", symbol.Name);
if (SymGetLineFromAddr(process_handle_, address, &displacement, &line) == false) {
_snprintf_s(buffer, _TRUNCATE, "(unknown file & line number): %s", symbol->Name);
string.append(buffer);
return;
}

_snprintf_s(buffer, _TRUNCATE, "%s(%d): %s", line.FileName, static_cast<int>(line.LineNumber), symbol.Name);
_snprintf_s(buffer, _TRUNCATE, "%s(%d): %s", line.FileName, static_cast<int>(line.LineNumber), symbol->Name);
string.append(buffer);
}

struct Symbol : SYMBOL_INFO {
Symbol() noexcept {
SizeOfStruct = sizeof(SYMBOL_INFO);
MaxNameLen = _countof(buffer);
}

char buffer[1024] = {0};
};

struct Line : IMAGEHLP_LINE {
Line() noexcept {
SizeOfStruct = sizeof(IMAGEHLP_LINE);
Expand Down

0 comments on commit 3076b56

Please sign in to comment.