This series: + fixes all the bugs I'm aware of in all the recent dbghelp changes (dwarf4, but others too). + and let dwarf4 replace dwarf2 as default debug format (it's been almost 15 years <g>) full wine recompilation required to get all debug info upgraded in Wine libraries, DLLs...
A+
---
Eric Pouech (8): dbghelp: fix potential crash when loading a builtin a PE module embedded in an ELF image dbghelp: protect against missing compiland when adding global function... dbghelp/dwarf: don't unmap twice the fmap of a DWZ module dbghelp: remove incorrect FIXME in SymEnumerateLoadedModules dbghelp: in SymEnumerateLoadedModules, don't limit the number of modules dbghelp: fix allocation error in image_load_debugaltlink dbghelp: simplify code for searching alternate debug info files configure.ac: let dwarf4 be Wine's default debug format
configure.ac | 6 +-- dlls/dbghelp/dwarf.c | 1 - dlls/dbghelp/module.c | 87 +++++++++++++++++++++---------------------- dlls/dbghelp/symbol.c | 7 +++- 4 files changed, 49 insertions(+), 52 deletions(-)
(regression introduced by ced12a1a3aca214c64887e1dc65df67015015394)
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index fe65683c6e1..1b64bb57eb7 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -990,7 +990,7 @@ DWORD64 WINAPI SymLoadModuleExW(HANDLE hProcess, HANDLE hFile, PCWSTR wImageNam module_remove(pcs, altmodule); }
- if ((dbghelp_options & SYMOPT_DEFERRED_LOADS) == 0) + if ((dbghelp_options & SYMOPT_DEFERRED_LOADS) == 0 && !module_get_container(pcs, module)) module_load_debug(module); return module->module.BaseOfImage; }
reported by: - Coverity: CID 1493458, CID 1493456 - https://bugs.winehq.org/show_bug.cgi?id=52045
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/symbol.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c index 435d94bc8eb..2443dbf7e73 100644 --- a/dlls/dbghelp/symbol.c +++ b/dlls/dbghelp/symbol.c @@ -353,8 +353,11 @@ struct symt_function* symt_new_function(struct module* module, init_function_or_inlinesite(sym, module, SymTagFunction, &compiland->symt, name, addr, size, sig_type); sym->next_inlinesite = NULL; /* first of list */ symt_add_module_ht(module, (struct symt_ht*)sym); - p = vector_add(&compiland->vchildren, &module->pool); - *p = &sym->symt; + if (compiland) + { + p = vector_add(&compiland->vchildren, &module->pool); + *p = &sym->symt; + } } return sym; }
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/dwarf.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/dbghelp/dwarf.c b/dlls/dbghelp/dwarf.c index 9d60aeb5ad4..67440e97fe8 100644 --- a/dlls/dbghelp/dwarf.c +++ b/dlls/dbghelp/dwarf.c @@ -4131,7 +4131,6 @@ static void dwarf2_unload_dwz(dwarf2_dwz_alternate_t* dwz) image_unmap_section(&dwz->sectmap[section_line]); image_unmap_section(&dwz->sectmap[section_ranges]);
- image_unmap_file(dwz->fmap); HeapFree(GetProcessHeap(), 0, dwz); }
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/module.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 1b64bb57eb7..8119f941e2a 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -1246,7 +1246,6 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE hProcess, if (!EnumProcessModules(hProcess, hMods, 256 * sizeof(hMods[0]), &sz)) { /* hProcess should also be a valid process handle !! */ - FIXME("If this happens, bump the number in mod\n"); HeapFree(GetProcessHeap(), 0, hMods); return FALSE; }
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/module.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 8119f941e2a..363f6314b22 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -1249,6 +1249,12 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE hProcess, HeapFree(GetProcessHeap(), 0, hMods); return FALSE; } + if (sz > 256 * sizeof(hMods[0])) + { + hMods = HeapReAlloc(GetProcessHeap(), 0, hMods, sz); + if (!hMods || !EnumProcessModules(hProcess, hMods, sz, &sz)) + return FALSE; + } sz /= sizeof(HMODULE); for (i = 0; i < sz; i++) {
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/module.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 363f6314b22..73243d80da5 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -671,9 +671,9 @@ struct image_file_map* image_load_debugaltlink(struct image_file_map* fmap, stru */ sect_len = image_get_map_size(&debugaltlink_sect); id = memchr(data, '\0', sect_len); - if (id) + if (id++) { - id++; + unsigned idlen = (const BYTE*)data + sect_len - id; fmap_link = HeapAlloc(GetProcessHeap(), 0, sizeof(*fmap_link)); if (fmap_link) { @@ -691,7 +691,8 @@ struct image_file_map* image_load_debugaltlink(struct image_file_map* fmap, stru { static const WCHAR globalDebugDirW[] = {'/','u','s','r','/','l','i','b','/','d','e','b','u','g','/','.','b','u','i','l','d','-','i','d','/'}; - dst = HeapAlloc(GetProcessHeap(), 0, sizeof(globalDebugDirW) + (3 + filename_len) * sizeof(WCHAR)); + dst = HeapAlloc(GetProcessHeap(), 0, + sizeof(globalDebugDirW) + (3 + filename_len + idlen * 2) * sizeof(WCHAR)); if (dst) { WCHAR* p;
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- dlls/dbghelp/module.c | 73 +++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 41 deletions(-)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 73243d80da5..5614c575cd0 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -634,6 +634,17 @@ found: return TRUE; }
+static WCHAR* append_hex(WCHAR* dst, const BYTE* id, const BYTE* end) +{ + while (id < end) + { + *dst++ = "0123456789abcdef"[*id >> 4 ]; + *dst++ = "0123456789abcdef"[*id & 0x0F]; + id++; + } + return dst; +} + /****************************************************************** * image_load_debugaltlink * @@ -683,16 +694,14 @@ struct image_file_map* image_load_debugaltlink(struct image_file_map* fmap, stru if (dst) { MultiByteToWideChar(CP_UNIXCP, 0, data, -1, dst, filename_len); - ret = image_check_debug_link_gnu_id(dst, fmap_link, id, data + sect_len - (const char*)id); + ret = image_check_debug_link_gnu_id(dst, fmap_link, id, idlen); HeapFree(GetProcessHeap(), 0, dst); } /* Trying relative path to build-id directory */ if (!ret) { - static const WCHAR globalDebugDirW[] = - {'/','u','s','r','/','l','i','b','/','d','e','b','u','g','/','.','b','u','i','l','d','-','i','d','/'}; dst = HeapAlloc(GetProcessHeap(), 0, - sizeof(globalDebugDirW) + (3 + filename_len + idlen * 2) * sizeof(WCHAR)); + sizeof(L"/usr/lib/debug/.build-id/") + (3 + filename_len + idlen * 2) * sizeof(WCHAR)); if (dst) { WCHAR* p; @@ -702,21 +711,16 @@ struct image_file_map* image_load_debugaltlink(struct image_file_map* fmap, stru * where the alternate file is... * so try both */ - p = memcpy(dst, globalDebugDirW, sizeof(globalDebugDirW)); - p += ARRAY_SIZE(globalDebugDirW); + p = memcpy(dst, L"/usr/lib/debug/.build-id/", sizeof(L"/usr/lib/debug/.build-id/")); + p += wcslen(dst); MultiByteToWideChar(CP_UNIXCP, 0, data, -1, p, filename_len); - ret = image_check_debug_link_gnu_id(dst, fmap_link, id, data + sect_len - (const char*)id); + ret = image_check_debug_link_gnu_id(dst, fmap_link, id, idlen); if (!ret) { - p = dst + ARRAY_SIZE(globalDebugDirW); - if ((const char*)id < data + sect_len) - { - *p++ = "0123456789abcdef"[*id >> 4 ]; - *p++ = "0123456789abcdef"[*id & 0x0F]; - } + p = append_hex(p, id, id + idlen); *p++ = '/'; MultiByteToWideChar(CP_UNIXCP, 0, data, -1, p, filename_len); - ret = image_check_debug_link_gnu_id(dst, fmap_link, id, data + sect_len - (const char*)id); + ret = image_check_debug_link_gnu_id(dst, fmap_link, id, idlen); } HeapFree(GetProcessHeap(), 0, dst); } @@ -742,51 +746,38 @@ struct image_file_map* image_load_debugaltlink(struct image_file_map* fmap, stru */ static BOOL image_locate_build_id_target(struct image_file_map* fmap, const BYTE* id, unsigned idlen) { - static const WCHAR globalDebugDirW[] = {'/','u','s','r','/','l','i','b','/','d','e','b','u','g','/'}; - static const WCHAR buildidW[] = {'.','b','u','i','l','d','-','i','d','/'}; struct image_file_map* fmap_link = NULL; WCHAR* p; WCHAR* z; - const BYTE* idend = id + idlen;
fmap_link = HeapAlloc(GetProcessHeap(), 0, sizeof(*fmap_link)); if (!fmap_link) return FALSE;
- p = HeapAlloc(GetProcessHeap(), 0, - sizeof(globalDebugDirW) + sizeof(buildidW) + - (idlen * 2 + 1) * sizeof(WCHAR) + sizeof(L".debug")); - z = p; - memcpy(z, globalDebugDirW, sizeof(globalDebugDirW)); - z += ARRAY_SIZE(globalDebugDirW); - memcpy(z, buildidW, sizeof(buildidW)); - z += ARRAY_SIZE(buildidW); - - if (id < idend) + p = malloc(sizeof(L"/usr/lib/debug/.build-id/") + + (idlen * 2 + 1) * sizeof(WCHAR) + sizeof(L".debug")); + wcscpy(p, L"/usr/lib/debug/.build-id/"); + z = p + wcslen(p); + if (idlen) { - *z++ = "0123456789abcdef"[*id >> 4 ]; - *z++ = "0123456789abcdef"[*id & 0x0F]; - id++; - } - if (id < idend) - *z++ = '/'; - while (id < idend) - { - *z++ = "0123456789abcdef"[*id >> 4 ]; - *z++ = "0123456789abcdef"[*id & 0x0F]; - id++; + z = append_hex(z, id, id + 1); + if (idlen > 1) + { + *z++ = L'/'; + z = append_hex(z, id + 1, id + idlen); + } } memcpy(z, L".debug", sizeof(L".debug")); TRACE("checking %s\n", wine_dbgstr_w(p));
- if (image_check_debug_link_gnu_id(p, fmap_link, idend - idlen, idlen)) + if (image_check_debug_link_gnu_id(p, fmap_link, id, idlen)) { - HeapFree(GetProcessHeap(), 0, p); + free(p); fmap->alternate = fmap_link; return TRUE; }
TRACE("not found\n"); - HeapFree(GetProcessHeap(), 0, p); + free(p); HeapFree(GetProcessHeap(), 0, fmap_link); return FALSE; }
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- configure.ac | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac index 5074ee14833..0509c52d2e9 100644 --- a/configure.ac +++ b/configure.ac @@ -1029,8 +1029,7 @@ then if test "x$ac_debug_format_seen" = x then case $CROSSDEBUG in - *dwarf) WINE_TRY_CROSSCFLAGS([-gdwarf-2]) - WINE_TRY_CROSSCFLAGS([-gstrict-dwarf]) ;; + *dwarf) WINE_TRY_CROSSCFLAGS([-gdwarf-4]) ;; pdb) WINE_TRY_CROSSCFLAGS([-gcodeview]) ;; esac fi @@ -1843,8 +1842,7 @@ char*f(const char *h,char n) {return strchr(h,n);}]])],[ac_cv_c_logicalop_noisy= done if test "x$ac_debug_format_seen" = xdefault then - WINE_TRY_CFLAGS([-gdwarf-2]) - WINE_TRY_CFLAGS([-gstrict-dwarf]) + WINE_TRY_CFLAGS([-gdwarf-4]) fi
dnl Disable gcc builtins except for Mingw
On Mon, 22 Nov 2021, Eric Pouech wrote:
This series:
- fixes all the bugs I'm aware of in all the recent dbghelp
changes (dwarf4, but others too).
- and let dwarf4 replace dwarf2 as default debug format (it's been
almost 15 years <g>)
FWIW, I ran into a surprise due to this (just mentioning for reference, it's probably not worth changing anything about); this broke the recently contributed stack unwinding on ARM, when built with Clang.
When building with -gdwarf-4 with Clang, the CIEs in .debug_frame end up as version 4, but libunwind (which is needed for unwinding the ELF modules) breaks when there's such data present - https://github.com/libunwind/libunwind/issues/256 tracks that issue upstream. (It's easy to work around and patch locally though.)
This is only an issue on ARM, as -fasynchronous-unwind-tables ensures producing .eh_frame (which doesn't have this issue with CIE version) on other architectures (including ARM64). (On ARM on ELF, a different unwinding mechanism, ARMEH, is used primarily, but libunwind doesn't parse that, but can make do with .debug_frame instead.)
So far I'm working around it by explicitly passing in CFLAGS="-g -O2 -gdwarf-2" when building with Clang for ARM, and I guess will be fixed eventually in upstream libunwind too.
// Martin
-
Eric Pouech -
Martin Storsjö