[PATCH v2 0/3] MR9078: More debugging related fixes & enhancements.
This serie: - drastically reduce winedbg attachement time when lots of modulesi or exported symbols are present (current use case here reduces from 12 seconds to less than a second) - fixes a crash in dbghelp. -- v2: dbghelp: Don't crash on stripped image without DEBUG directories. dbghelp: Add public symbols out of export table when no debug info is present. winedbg: Simplify fetching module name. https://gitlab.winehq.org/wine/wine/-/merge_requests/9078
From: Eric Pouech <epouech(a)codeweavers.com> Always using GetMappedFileNameW for active targets: - as it doesn't depend on information from debug event that are not always present (pointer to module name, or file handle), - it only uses a single server call (pointer to module name requires two). - and stop using GetModuleFileNameExW which implementation walks the module list in debuggee process, hence request time grows linearly with number of loaded modules. Keep fallback to indirection in debuggee address space for non active targets. This reduces significantly attachment time in auto mode. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/winedbg/tgt_active.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/programs/winedbg/tgt_active.c b/programs/winedbg/tgt_active.c index a919f70a486..85fcc430014 100644 --- a/programs/winedbg/tgt_active.c +++ b/programs/winedbg/tgt_active.c @@ -315,20 +315,21 @@ static DWORD dbg_handle_exception(const EXCEPTION_RECORD* rec, BOOL first_chance static BOOL tgt_process_active_close_process(struct dbg_process* pcs, BOOL kill); -void fetch_module_name(void* name_addr, void* mod_addr, WCHAR* buffer, size_t bufsz) +void fetch_module_name(void *name_addr, void* mod_addr, WCHAR* buffer, size_t bufsz) { - memory_get_string_indirect(dbg_curr_process, name_addr, TRUE, buffer, bufsz); - if (!buffer[0] && !GetModuleFileNameExW(dbg_curr_process->handle, mod_addr, buffer, bufsz)) + DWORD len; + if (dbg_curr_process->active_debuggee && (len = GetMappedFileNameW( dbg_curr_process->handle, mod_addr, buffer, bufsz ))) { - if (GetMappedFileNameW( dbg_curr_process->handle, mod_addr, buffer, bufsz )) - { - /* FIXME: proper NT->Dos conversion */ - static const WCHAR nt_prefixW[] = {'\\','?','?','\\'}; + /* FIXME: proper NT->Dos conversion */ + static const WCHAR nt_prefixW[] = {'\\','?','?','\\'}; - if (!wcsncmp( buffer, nt_prefixW, 4 )) - memmove( buffer, buffer + 4, (lstrlenW(buffer + 4) + 1) * sizeof(WCHAR) ); - } - else + if (!wcsncmp( buffer, nt_prefixW, 4 )) + memmove( buffer, buffer + 4, (len - 4 + 1) * sizeof(WCHAR) ); + } + else + { + memory_get_string_indirect(dbg_curr_process, name_addr, TRUE, buffer, bufsz); + if (!buffer[0]) swprintf(buffer, bufsz, L"DLL_%08lx", (ULONG_PTR)mod_addr); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9078
From: Eric Pouech <epouech(a)codeweavers.com> Always creating public symbols from export table can be slow. If we already have some debug information, then exported symbols are likely already present. In extreme cases, this can reduce significantly loading time of module. Note: in some (rare) cases, this will change current behavior: - if symbol has no debug inforamtion attached to it (assembly...), then no name will be available for it, - if exported symbol name is different from internal one, then only the internal one will be reported. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/pe_module.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dlls/dbghelp/pe_module.c b/dlls/dbghelp/pe_module.c index 6b5a9a4b425..cdb8775f70d 100644 --- a/dlls/dbghelp/pe_module.c +++ b/dlls/dbghelp/pe_module.c @@ -779,10 +779,16 @@ BOOL pe_load_debug_info(const struct process* pcs, struct module* module) * in which case we'll rely on the export's on the ELF side */ } - /* FIXME shouldn't we check that? if (!module_get_debug(pcs, module)) */ - if (pe_load_export_debug_info(pcs, module) && !ret) - ret = TRUE; - if (!ret) module->module.SymType = SymNone; + /* FIXME: + * - only loading export debug info in last resort when none of the available formats succeeded + * (assuming export debug info is a subset of actual debug infomation). + */ + if (module->module.SymType == SymDeferred) + { + ret = pe_load_export_debug_info(pcs, module) || ret; + if (module->module.SymType == SymDeferred) + module->module.SymType = SymNone; + } return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9078
From: Eric Pouech <epouech(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58742 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/pe_module.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/dlls/dbghelp/pe_module.c b/dlls/dbghelp/pe_module.c index cdb8775f70d..cbfa2b902c1 100644 --- a/dlls/dbghelp/pe_module.c +++ b/dlls/dbghelp/pe_module.c @@ -650,19 +650,18 @@ static BOOL pe_load_msc_debug_info(const struct process* pcs, struct module* mod if (nth->FileHeader.Characteristics & IMAGE_FILE_DEBUG_STRIPPED) { /* Debug info is stripped to .DBG file */ - const IMAGE_DEBUG_MISC* misc = (const IMAGE_DEBUG_MISC*) - ((const char*)mapping + dbg->PointerToRawData); - - if (nDbg != 1 || dbg->Type != IMAGE_DEBUG_TYPE_MISC || - misc->DataType != IMAGE_DEBUG_MISC_EXENAME) + const IMAGE_DEBUG_MISC *misc = NULL; + if (nDbg == 1 && dbg->Type == IMAGE_DEBUG_TYPE_MISC) { + misc = (const IMAGE_DEBUG_MISC *)((const char *)mapping + dbg->PointerToRawData); + if (misc->DataType == IMAGE_DEBUG_MISC_EXENAME) + ret = pe_load_dbg_file(pcs, module, (const char*)misc->Data, nth->FileHeader.TimeDateStamp); + else + misc = NULL; + } + if (!misc) WARN("-Debug info stripped, but no .DBG file in module %s\n", debugstr_w(module->modulename)); - } - else - { - ret = pe_load_dbg_file(pcs, module, (const char*)misc->Data, nth->FileHeader.TimeDateStamp); - } } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9078
V2 pushed: - should take care of failing CI tests - removing Rémi from reviewers (as it no longer touches gdbproxy.c) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9078#note_117280
participants (2)
-
Eric Pouech -
eric pouech (＠epo)