Refactoring debug symbol resolving process on Windows#1346
Conversation
slomp
force-pushed
the
slomp/win-dbghelp-overhaul
branch
from
bd3aa9a to
08d9a6b
Compare
slomp
force-pushed
the
slomp/win-dbghelp-overhaul
branch
from
08d9a6b to
207a7ab
Compare
slomp
force-pushed
the
slomp/win-dbghelp-overhaul
branch
from
bdb1508 to
59bf76e
Compare
| if( code != ERROR_SUCCESS ) | ||
| { | ||
| char msg [512] = {}; | ||
| const char* modName = ModuleName ? ModuleName : (ImageName ? ImageName : "[NULL]"); |
There was a problem hiding this comment.
I would use ImageName first for logging. Even though ModuleName is the "Friendly name", it is sometimes necessary to differentiate two dlls named the same way by their path (yes, it's possible!).
| return 0; | ||
| #endif | ||
| using SymAddrIncludeInlineTraceProc = decltype(TracySymAddrIncludeInlineTrace)(__stdcall*); | ||
| static auto _SymAddrIncludeInlineTrace = (SymAddrIncludeInlineTraceProc)GetProcAddress(GetModuleHandleA("dbghelp.dll"), "SymAddrIncludeInlineTrace"); |
There was a problem hiding this comment.
I'm not sure I like having deferred loading of dll procs each in their own function. I'd rather keep this in the init function if only so that we can log all loading failures at the same place, and know the list of things we depend on in one place.
| { | ||
| static constexpr size_t MaxNameSize = 8*1024; | ||
| SYMBOL_INFO info; | ||
| char buf [MaxNameSize]; // must follow SYMBOL_INFO |
There was a problem hiding this comment.
Should we pack to avoid padding here?
| char* m_name = nullptr; | ||
| char* m_path = nullptr; | ||
|
|
||
| #if TRACY_HAS_CALLSTACK == 1 |
There was a problem hiding this comment.
Note: I'll probably turn this into a BuildInfo (buildid/pdb info, etc) member later
| { | ||
| const char* name; | ||
| uint64_t baseAddr; | ||
| #if TRACY_HAS_CALLSTACK == 1 |
There was a problem hiding this comment.
Not sure about this one (I'm on mobile right now), but isn't this Windows only already?
|
|
||
| return { cb_data, 1, moduleNameAndBaseAddress.name }; | ||
| DbgHelpInline inln; | ||
| int inlineNum = DbgHelpTraceInline( inln, ptr ); |
There was a problem hiding this comment.
It bothers me a bit that the lock is now inside the wrappers, it means we're going to lock/unlock it for each call where before we would just lock unlock once.
| namespace tracy | ||
| { | ||
|
|
||
| struct ImageEntry |
There was a problem hiding this comment.
Note: I'll probably move it back here when adding build info, cause I'll need it to be part of the interface if we want offline resolution.
2 participants
Present:
DBGHELP_LOCK/UNLOCKTRACY_SYMBOL_PATHto_NT_SYMBOL_PATHforSymSrv.dllPlanned for a future PR:
SymGetDiaSession()) whenever possiblecs_disasm()loop