Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make MS Debug engine SymInitialize() called as needed. #20036

Merged
merged 1 commit into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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()};

Check warning on line 67 in onnxruntime/core/platform/windows/debug_alloc.cc

View workflow job for this annotation

GitHub Actions / Lint C++

[cpplint] reported by reviewdog 🐶 Use int16/int64/etc, rather than the C type long [runtime/int] [4] Raw Output: onnxruntime/core/platform/windows/debug_alloc.cc:67: Use int16/int64/etc, rather than the C type long [runtime/int] [4]
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
Loading