[PATCH 0/4] MR5321: Minidump slowdowns (Part III)
This is the last part for handling the minidump slow downs: - it removes the integration of RUNTIME_INFO chunks for x86_64 - adds fallback mechanism to read this unwind info from winedbg side. Some remarks: - it looks a bit awkward that SymFunctionTableAccess reads PE information from PE image on dbghelp side, while the unwind information reading is read (in this MR) from debugger side - some testings with native show that: + unwind info (inside StackWalk calls) is read through the read memory handler (so delegated to debugger) + there's no fallback from the read memory handler - so this looks like the minimal things to do - however, there could be additional fallback in dbghelp (like in dbghelp internal read memory handler); didn't spend time on these as they would only be helpful for non live targets, using the default read memory handler. - fixed winedbg to reload properly minidumps. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5321
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/cpu_x86_64.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/dbghelp/cpu_x86_64.c b/dlls/dbghelp/cpu_x86_64.c index 5193faee825..a2be9fabbb7 100644 --- a/dlls/dbghelp/cpu_x86_64.c +++ b/dlls/dbghelp/cpu_x86_64.c @@ -279,6 +279,7 @@ static int get_opcode_size(UNWIND_CODE op) return 2 + (op.OpInfo != 0); case UWOP_SAVE_NONVOL: case UWOP_SAVE_XMM128: + case UWOP_EPILOG: return 2; case UWOP_SAVE_NONVOL_FAR: case UWOP_SAVE_XMM128_FAR: @@ -586,6 +587,9 @@ static BOOL interpret_function_table_entry(struct cpu_stack_walk* csw, if (!sw_read_mem(csw, context->Rsp + 24, &context->Rsp, sizeof(DWORD64))) return FALSE; mach_frame = TRUE; break; + case UWOP_EPILOG: + if (info->Version == 2) + break; /* nothing to do */ default: FIXME("unknown code %u\n", info->UnwindCode[i].UnwindOp); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5321
From: Eric Pouech <epouech(a)codeweavers.com> Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/winedbg/tgt_minidump.c | 65 +++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/programs/winedbg/tgt_minidump.c b/programs/winedbg/tgt_minidump.c index 8ca2bd7e5b6..d7cfb73a088 100644 --- a/programs/winedbg/tgt_minidump.c +++ b/programs/winedbg/tgt_minidump.c @@ -25,6 +25,7 @@ #include "debugger.h" #include "wingdi.h" +#include "winnt.h" #include "winuser.h" #include "tlhelp32.h" #include "wine/debug.h" @@ -143,6 +144,70 @@ static BOOL tgt_process_minidump_read(HANDLE hProcess, const void* addr, return TRUE; } } + /* The memory isn't present in minidump. Try to fetch read-only area from PE image. */ + { + IMAGEHLP_MODULEW64 im = {.SizeOfStruct = sizeof(im)}; + + if (SymGetModuleInfoW64(dbg_curr_process->handle, (DWORD_PTR)addr, &im)) + { + WCHAR *image_name; + HANDLE file, map = 0; + void *pe_mapping = NULL; + BOOL found = FALSE; + const IMAGE_NT_HEADERS *nthdr = NULL; + + image_name = im.LoadedImageName[0] ? im.LoadedImageName : im.ImageName; + if ((file = CreateFileW(image_name, GENERIC_READ, FILE_SHARE_READ, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)) != INVALID_HANDLE_VALUE && + ((map = CreateFileMappingW(file, NULL, PAGE_READONLY, 0, 0, NULL)) != 0) && + ((pe_mapping = MapViewOfFile(map, FILE_MAP_READ, 0, 0, 0)) != NULL) && + (nthdr = RtlImageNtHeader(pe_mapping)) != NULL) + { + DWORD_PTR rva = (DWORD_PTR)addr - im.BaseOfImage; + ptrdiff_t size_hdr = (const BYTE*)(IMAGE_FIRST_SECTION(nthdr) + nthdr->FileHeader.NumberOfSections) - (const BYTE*)pe_mapping; + + /* in the PE header ? */ + if (rva < size_hdr) + { + if (rva + len > size_hdr) + len = size_hdr - rva; + memcpy(buffer, (const BYTE*)pe_mapping + rva, len); + if (rlen) *rlen = len; + found = TRUE; + } + else /* in read only section ? */ + { + /* Note: RtlImageRvaToSection checks RVA against raw size, so we won't + * get section when rva falls into the (raw size, virtual size( interval. + */ + const IMAGE_SECTION_HEADER *section = RtlImageRvaToSection(nthdr, NULL, rva); + if (section && !(section->Characteristics & IMAGE_SCN_MEM_WRITE)) + { + DWORD_PTR offset = rva - section->VirtualAddress; + DWORD nw = len; + + if (offset + nw > section->SizeOfRawData) + nw = section->SizeOfRawData - offset; + memcpy(buffer, (const char*)pe_mapping + section->PointerToRawData + offset, nw); + if (nw < len) /* fill with O? */ + { + if (offset + len > section->Misc.VirtualSize) + len = section->Misc.VirtualSize - offset; + memset((char*)buffer + nw, 0, len - nw); + nw = len; + } + if (rlen) *rlen = nw; + found = TRUE; + } + } + } + if (pe_mapping) UnmapViewOfFile(pe_mapping); + if (map) CloseHandle(map); + if (file != INVALID_HANDLE_VALUE) CloseHandle(file); + if (found) return TRUE; + } + } + /* FIXME: this is a dirty hack to let the last frame in a bt to work * However, we need to check who's to blame, this code or the current * dbghelp!StackWalk implementation -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5321
From: Eric Pouech <epouech(a)codeweavers.com> So that we can get to debug info. Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- programs/winedbg/tgt_minidump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/winedbg/tgt_minidump.c b/programs/winedbg/tgt_minidump.c index d7cfb73a088..ac8d9be71b2 100644 --- a/programs/winedbg/tgt_minidump.c +++ b/programs/winedbg/tgt_minidump.c @@ -496,7 +496,7 @@ static enum dbg_start minidump_do_reload(struct tgt_process_minidump_data* data) mm->SizeOfImage); else SymLoadModuleExW(hProc, NULL, nameW, NULL, get_addr64(mm->BaseOfImage), - mm->SizeOfImage, NULL, SLMFLAG_VIRTUAL); + mm->SizeOfImage, NULL, 0); } } if (MiniDumpReadDumpStream(data->mapping, ModuleListStream, NULL, &stream, NULL)) @@ -518,7 +518,7 @@ static enum dbg_start minidump_do_reload(struct tgt_process_minidump_data* data) mm->SizeOfImage); else SymLoadModuleExW(hProc, NULL, nameW, NULL, get_addr64(mm->BaseOfImage), - mm->SizeOfImage, NULL, SLMFLAG_VIRTUAL); + mm->SizeOfImage, NULL, 0); } } if (MiniDumpReadDumpStream(data->mapping, ExceptionStream, NULL, &stream, NULL)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5321
From: Eric Pouech <epouech(a)codeweavers.com> This can generate very long time when saving the minidump, but also when reloading it. Unwind information is expected to be gotten from an image matching the modules listed in the minidump for Normal mode. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55798 Signed-off-by: Eric Pouech <epouech(a)codeweavers.com> --- dlls/dbghelp/cpu_x86_64.c | 39 +---------------------------------- dlls/dbghelp/tests/minidump.c | 1 - 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/dlls/dbghelp/cpu_x86_64.c b/dlls/dbghelp/cpu_x86_64.c index a2be9fabbb7..a328d592d43 100644 --- a/dlls/dbghelp/cpu_x86_64.c +++ b/dlls/dbghelp/cpu_x86_64.c @@ -970,44 +970,7 @@ static BOOL x86_64_fetch_minidump_thread(struct dump_context* dc, unsigned index static BOOL x86_64_fetch_minidump_module(struct dump_context* dc, unsigned index, unsigned flags) { - /* FIXME: not sure about the flags... */ - if (1) - { - /* FIXME: crop values across module boundaries, */ -#ifdef __x86_64__ - struct process* pcs; - struct module* module; - const RUNTIME_FUNCTION* rtf; - ULONG size; - - if (!(pcs = process_find_by_handle(dc->process->handle)) || - !(module = module_find_by_addr(pcs, dc->modules[index].base))) - return FALSE; - rtf = (const RUNTIME_FUNCTION*)pe_map_directory(module, IMAGE_DIRECTORY_ENTRY_EXCEPTION, &size); - if (rtf) - { - const RUNTIME_FUNCTION* end = (const RUNTIME_FUNCTION*)((const char*)rtf + size); - UNWIND_INFO ui; - - while (rtf + 1 < end) - { - while (rtf->UnwindData & 1) /* follow chained entry */ - { - FIXME("RunTime_Function outside IMAGE_DIRECTORY_ENTRY_EXCEPTION unimplemented yet!\n"); - return FALSE; - /* we need to read into the other process */ - /* rtf = (RUNTIME_FUNCTION*)(module->module.BaseOfImage + (rtf->UnwindData & ~1)); */ - } - if (read_process_memory(dc->process, dc->modules[index].base + rtf->UnwindData, &ui, sizeof(ui))) - minidump_add_memory_block(dc, dc->modules[index].base + rtf->UnwindData, - FIELD_OFFSET(UNWIND_INFO, UnwindCode) + ui.CountOfCodes * sizeof(UNWIND_CODE), 0); - rtf++; - } - } -#endif - } - - return TRUE; + return FALSE; } struct cpu cpu_x86_64 = { diff --git a/dlls/dbghelp/tests/minidump.c b/dlls/dbghelp/tests/minidump.c index 3e07911bae6..c6f73ca2662 100644 --- a/dlls/dbghelp/tests/minidump.c +++ b/dlls/dbghelp/tests/minidump.c @@ -496,7 +496,6 @@ static void test_current_process(void) /* native embeds some elements in code segment from ntdll */ ok(walker.num_text < 5, "unexpected code segments %u %u\n", walker.num_text, num_available_modules); - todo_wine_if(RtlImageNtHeader(GetModuleHandleW(NULL))->FileHeader.Machine == IMAGE_FILE_MACHINE_AMD64) ok(walker.num_unwind_info == 0, "unexpected unwind info %u\n", walker.num_unwind_info); minidump_check_nostream(data, ExceptionStream); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5321
participants (2)
-
Eric Pouech -
eric pouech (＠epo)