[PATCH 0/1] MR10016: dbghelp: Rewrite EnumerateLoadedModulesW64 in terms of CreateToolhelp32Snapshot.
Follow up to !9371. Did some quick, unscientific performance tests, EnumerateLoadedModulesW64 is massively faster after this change, so I think everything is working. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10016
From: Yuxuan Shui <yshui@codeweavers.com> --- dlls/dbghelp/module.c | 125 ++++++++---------------------------------- 1 file changed, 24 insertions(+), 101 deletions(-) diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 6428c539420..d0e27a44d28 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -29,6 +29,7 @@ #include "dbghelp_private.h" #include "image_private.h" #include "psapi.h" +#include "tlhelp32.h" #include "winternl.h" #include "wine/debug.h" @@ -1254,23 +1255,6 @@ BOOL WINAPI EnumerateLoadedModules(HANDLE hProcess, } #endif -static unsigned int load_and_grow_modules(HANDLE process, HMODULE** hmods, unsigned start, unsigned* alloc, DWORD filter) -{ - DWORD needed; - BOOL ret; - - while ((ret = EnumProcessModulesEx(process, *hmods + start, (*alloc - start) * sizeof(HMODULE), - &needed, filter)) && - needed > (*alloc - start) * sizeof(HMODULE)) - { - HMODULE* new = HeapReAlloc(GetProcessHeap(), 0, *hmods, (*alloc) * 2 * sizeof(HMODULE)); - if (!new) return 0; - *hmods = new; - *alloc *= 2; - } - return ret ? needed / sizeof(HMODULE) : 0; -} - /****************************************************************** * EnumerateLoadedModulesW64 (DBGHELP.@) * @@ -1280,15 +1264,16 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE process, PVOID user) { OBJECT_BASIC_INFORMATION obi; - HMODULE* hmods; - unsigned alloc = 256, count, count32, i; - USHORT pcs_machine, native_machine; - BOOL with_32bit_modules; - WCHAR imagenameW[MAX_PATH]; - MODULEINFO mi; - WCHAR* sysdir = NULL; - WCHAR* wowdir = NULL; - size_t sysdir_len = 0, wowdir_len = 0; + DWORD snapshot_flags; + DWORD pid = GetProcessId( process ); + HANDLE snapshot; + MODULEENTRY32W me; + + if (!pid) + { + SetLastError(STATUS_INVALID_CID); + return FALSE; + } if (process != GetCurrentProcess() && RtlIsCurrentProcess( process ) && @@ -1296,90 +1281,28 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE process, obi.GrantedAccess & PROCESS_VM_READ) { TRACE("same process.\n"); - process = GetCurrentProcess(); + pid = 0; } - /* process might not be a handle to a live process */ - if (!IsWow64Process2(process, &pcs_machine, &native_machine)) - { - SetLastError(STATUS_INVALID_CID); - return FALSE; - } - with_32bit_modules = sizeof(void*) > sizeof(int) && - pcs_machine != IMAGE_FILE_MACHINE_UNKNOWN && - (dbghelp_options & SYMOPT_INCLUDE_32BIT_MODULES); + snapshot_flags = TH32CS_SNAPMODULE; + if (dbghelp_options & SYMOPT_INCLUDE_32BIT_MODULES) + snapshot_flags |= TH32CS_SNAPMODULE32; - if (!(hmods = HeapAlloc(GetProcessHeap(), 0, alloc * sizeof(hmods[0])))) + snapshot = CreateToolhelp32Snapshot( snapshot_flags, pid ); + if (snapshot == INVALID_HANDLE_VALUE) return FALSE; - /* Note: - * - we report modules returned from kernelbase.EnumProcessModulesEx - * - appending 32bit modules when possible and requested - * - * When considering 32bit modules in a wow64 child process, required from - * a 64bit process: - * - native returns from kernelbase.EnumProcessModulesEx - * redirected paths (that is in system32 directory), while - * dbghelp.EnumerateLoadedModulesWine returns the effective path - * (eg. syswow64 for x86_64). - * - (Except for the main module, if gotten from syswow64, where kernelbase - * will return the effective path) - * - Wine kernelbase (and ntdll) incorrectly return these modules from - * syswow64 (except for ntdll which is returned from system32). - * => for these modules, always perform a system32 => syswow64 path - * conversion (it'll work even if ntdll/kernelbase is fixed). - */ - if ((count = load_and_grow_modules(process, &hmods, 0, &alloc, LIST_MODULES_DEFAULT)) && with_32bit_modules) + me.dwSize = sizeof(me); + if (Module32FirstW( snapshot, &me )) { - /* append 32bit modules when required */ - if ((count32 = load_and_grow_modules(process, &hmods, count, &alloc, LIST_MODULES_32BIT))) + do { - sysdir_len = GetSystemDirectoryW(NULL, 0); - wowdir_len = GetSystemWow64Directory2W(NULL, 0, pcs_machine); - - if (!sysdir_len || !wowdir_len || - !(sysdir = HeapAlloc(GetProcessHeap(), 0, (sysdir_len + 1 + wowdir_len + 1) * sizeof(WCHAR)))) - { - HeapFree(GetProcessHeap(), 0, hmods); - return FALSE; - } - wowdir = sysdir + sysdir_len + 1; - if (GetSystemDirectoryW(sysdir, sysdir_len) >= sysdir_len) - FIXME("shouldn't happen\n"); - if (GetSystemWow64Directory2W(wowdir, wowdir_len, pcs_machine) >= wowdir_len) - FIXME("shouldn't happen\n"); - wcscat(sysdir, L"\\"); - wcscat(wowdir, L"\\"); - } - } - else count32 = 0; - - for (i = 0; i < count + count32; i++) - { - if (GetModuleInformation(process, hmods[i], &mi, sizeof(mi)) && - GetModuleFileNameExW(process, hmods[i], imagenameW, ARRAY_SIZE(imagenameW))) - { - /* rewrite path in system32 into syswow64 for 32bit modules */ - if (i >= count) - { - size_t len = wcslen(imagenameW); - - if (!wcsnicmp(imagenameW, sysdir, sysdir_len) && - (len - sysdir_len + wowdir_len) + 1 <= ARRAY_SIZE(imagenameW)) - { - memmove(&imagenameW[wowdir_len], &imagenameW[sysdir_len], (len - sysdir_len) * sizeof(WCHAR)); - memcpy(imagenameW, wowdir, wowdir_len * sizeof(WCHAR)); - } - } - if (!enum_cb(imagenameW, (DWORD_PTR)mi.lpBaseOfDll, mi.SizeOfImage, user)) - break; - } + if (!enum_cb( me.szExePath, (DWORD_PTR)me.modBaseAddr, me.modBaseSize, user )) break; + } while (Module32NextW( snapshot, &me )); } - HeapFree(GetProcessHeap(), 0, hmods); - HeapFree(GetProcessHeap(), 0, sysdir); - - return count != 0; + CloseHandle( snapshot ); + return TRUE; } static void dbghelp_str_WtoA(const WCHAR *src, char *dst, int dst_len) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10016
I get this error on old wow configuration when testing dbghelp:dbghelp dbghelp.c:1475: Test failed: \[32/32 enum:default wine_old_wow64\]: Unexpected event.count_wowdir 5 but that's generated from the debug LOAD_DLL event:s, not your patch; so we can leave it aside for now -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10016#note_128636
This merge request was approved by eric pouech. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10016
participants (3)
-
eric pouech (@epo) -
Yuxuan Shui -
Yuxuan Shui (@yshui)