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

src,win: informative stack traces #23822

Merged
merged 1 commit into from
Oct 30, 2018
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
139 changes: 110 additions & 29 deletions src/debug_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,35 +100,104 @@ class Win32SymbolDebuggingContext final : public NativeSymbolDebuggingContext {
USE(SymInitialize(current_process_, nullptr, true));
}

~Win32SymbolDebuggingContext() {
~Win32SymbolDebuggingContext() override {
USE(SymCleanup(current_process_));
}

SymbolInfo LookupSymbol(void* address) override {
// Ref: https://msdn.microsoft.com/en-en/library/windows/desktop/ms680578(v=vs.85).aspx
char info_buf[sizeof(SYMBOL_INFO) + MAX_SYM_NAME];
SYMBOL_INFO* info = reinterpret_cast<SYMBOL_INFO*>(info_buf);
char demangled[MAX_SYM_NAME];
using NameAndDisplacement = std::pair<std::string, DWORD64>;
NameAndDisplacement WrappedSymFromAddr(DWORD64 dwAddress) const {
// Refs: https://docs.microsoft.com/en-us/windows/desktop/Debug/retrieving-symbol-information-by-address
// Patches:
// Use `fprintf(stderr, ` instead of `printf`
// `sym.filename = pSymbol->Name` on success
// `current_process_` instead of `hProcess.
DWORD64 dwDisplacement = 0;
// Patch: made into arg - DWORD64 dwAddress = SOME_ADDRESS;

char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)];
const auto pSymbol = reinterpret_cast<PSYMBOL_INFO>(buffer);

pSymbol->SizeOfStruct = sizeof(SYMBOL_INFO);
pSymbol->MaxNameLen = MAX_SYM_NAME;

if (SymFromAddr(current_process_, dwAddress, &dwDisplacement, pSymbol)) {
// SymFromAddr returned success
return NameAndDisplacement(pSymbol->Name, dwDisplacement);
} else {
// SymFromAddr failed
const DWORD error = GetLastError(); // "eat" the error anyway
#ifdef DEBUG
fprintf(stderr, "SymFromAddr returned error : %lu\n", error);
#endif
}
// End MSDN code

info->MaxNameLen = MAX_SYM_NAME;
info->SizeOfStruct = sizeof(SYMBOL_INFO);
return NameAndDisplacement();
}

SymbolInfo ret;
const bool have_info = SymFromAddr(current_process_,
reinterpret_cast<DWORD64>(address),
nullptr,
info);
if (have_info && strlen(info->Name) == 0) {
if (UnDecorateSymbolName(info->Name,
demangled,
sizeof(demangled),
UNDNAME_COMPLETE)) {
ret.name = demangled;
} else {
ret.name = info->Name;
}
SymbolInfo WrappedGetLine(DWORD64 dwAddress) const {
SymbolInfo sym{};

// Refs: https://docs.microsoft.com/en-us/windows/desktop/Debug/retrieving-symbol-information-by-address
// Patches:
// Use `fprintf(stderr, ` instead of `printf`.
// Assign values to `sym` on success.
// `current_process_` instead of `hProcess.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not sure whether this is going to help us at some point, I suspect it won't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment are free.
Personally I have not been in a situation where I said "I wish there was less comments"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to agree, but someone will either change our code or the original, and updating the comment will mostly be a maintenance burden at that point from my perspective.


// Patch: made into arg - DWORD64 dwAddress;
DWORD dwDisplacement;
IMAGEHLP_LINE64 line;

SymSetOptions(SYMOPT_LOAD_LINES);

line.SizeOfStruct = sizeof(IMAGEHLP_LINE64);
// Patch: made into arg - dwAddress = 0x1000000;

if (SymGetLineFromAddr64(current_process_, dwAddress,
&dwDisplacement, &line)) {
// SymGetLineFromAddr64 returned success
sym.filename = line.FileName;
sym.line = line.LineNumber;
} else {
// SymGetLineFromAddr64 failed
const DWORD error = GetLastError(); // "eat" the error anyway
#ifdef DEBUG
fprintf(stderr, "SymGetLineFromAddr64 returned error : %lu\n", error);
#endif
}
// End MSDN code

return sym;
}

// Fills the SymbolInfo::name of the io/out argument `sym`
std::string WrappedUnDecorateSymbolName(const char* name) const {
// Refs: https://docs.microsoft.com/en-us/windows/desktop/Debug/retrieving-undecorated-symbol-names
// Patches:
// Use `fprintf(stderr, ` instead of `printf`.
// return `szUndName` instead of `printf` on success
char szUndName[MAX_SYM_NAME];
if (UnDecorateSymbolName(name, szUndName, sizeof(szUndName),
UNDNAME_COMPLETE)) {
// UnDecorateSymbolName returned success
return szUndName;
} else {
// UnDecorateSymbolName failed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of this type of comment: What happens is obvious in both branches, and the branches themselves are short. (And I think we have a convention of ending comments with a period.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was lifter verbatim from https://docs.microsoft.com/en-us/windows/desktop/Debug/retrieving-symbol-information-by-address
I would rather keep changes to minimon, for future reverse engineering.

const DWORD error = GetLastError(); // "eat" the error anyway
#ifdef DEBUG
fprintf(stderr, "UnDecorateSymbolName returned error %lu\n", error);
#endif
}
return nullptr;
}

SymbolInfo LookupSymbol(void* address) override {
const DWORD64 dw_address = reinterpret_cast<DWORD64>(address);
SymbolInfo ret = WrappedGetLine(dw_address);
std::tie(ret.name, ret.dis) = WrappedSymFromAddr(dw_address);
if (!ret.name.empty()) {
ret.name = WrappedUnDecorateSymbolName(ret.name.c_str());
}
return ret;
}

Expand All @@ -145,6 +214,13 @@ class Win32SymbolDebuggingContext final : public NativeSymbolDebuggingContext {
return CaptureStackBackTrace(0, count, frames, nullptr);
}

Win32SymbolDebuggingContext(const Win32SymbolDebuggingContext&) = delete;
Win32SymbolDebuggingContext(Win32SymbolDebuggingContext&&) = delete;
Win32SymbolDebuggingContext operator=(const Win32SymbolDebuggingContext&)
= delete;
Win32SymbolDebuggingContext operator=(Win32SymbolDebuggingContext&&)
= delete;

private:
HANDLE current_process_;
};
Expand All @@ -158,13 +234,18 @@ NativeSymbolDebuggingContext::New() {
#endif // __POSIX__

std::string NativeSymbolDebuggingContext::SymbolInfo::Display() const {
std::string ret = name;
std::ostringstream oss;
oss << name;
if (dis != 0) {
oss << "+" << dis;
}
if (!filename.empty()) {
ret += " [";
ret += filename;
ret += ']';
oss << " [" << filename << ']';
}
if (line != 0) {
oss << ":L" << line;
}
return ret;
return oss.str();
}

void DumpBacktrace(FILE* fp) {
Expand All @@ -173,8 +254,8 @@ void DumpBacktrace(FILE* fp) {
const int size = sym_ctx->GetStackTrace(frames, arraysize(frames));
for (int i = 1; i < size; i += 1) {
void* frame = frames[i];
fprintf(fp, "%2d: %p %s\n",
i, frame, sym_ctx->LookupSymbol(frame).Display().c_str());
NativeSymbolDebuggingContext::SymbolInfo s = sym_ctx->LookupSymbol(frame);
fprintf(fp, "%2d: %p %s\n", i, frame, s.Display().c_str());
refack marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
14 changes: 13 additions & 1 deletion src/debug_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "async_wrap.h"
#include "env.h"
#include <string>
#include <sstream>

// Use FORCE_INLINE on functions that have a debug-category-enabled check first
// and then ideally only a single function call following it, to maintain
Expand Down Expand Up @@ -93,14 +94,25 @@ class NativeSymbolDebuggingContext {
public:
std::string name;
std::string filename;
size_t line = 0;
size_t dis = 0;

std::string Display() const;
};

NativeSymbolDebuggingContext() = default;
virtual ~NativeSymbolDebuggingContext() {}
virtual SymbolInfo LookupSymbol(void* address) { return { "", "" }; }

virtual SymbolInfo LookupSymbol(void* address) { return {}; }
virtual bool IsMapped(void* address) { return false; }
virtual int GetStackTrace(void** frames, int count) { return 0; }

NativeSymbolDebuggingContext(const NativeSymbolDebuggingContext&) = delete;
NativeSymbolDebuggingContext(NativeSymbolDebuggingContext&&) = delete;
NativeSymbolDebuggingContext operator=(NativeSymbolDebuggingContext&)
= delete;
NativeSymbolDebuggingContext operator=(NativeSymbolDebuggingContext&&)
= delete;
};

// Variant of `uv_loop_close` that tries to be as helpful as possible
Expand Down