This series provides a bunch of new tests related to module management in dbghelp, and a couple of fixes: - correcting some module's name vs filename propagation - better handling in SymLoadModules when module overlaps the already loaded module(s)
From: Eric Pouech eric.pouech@gmail.com
It's supposed to be the base address of the module.
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/tests/dbghelp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index bcae5c1f875..4ddc7e7bc8a 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -241,6 +241,7 @@ static void test_modules(void) BOOL ret; char file_system[MAX_PATH]; char file_wow64[MAX_PATH]; + DWORD64 base; const DWORD64 base1 = 0x00010000; const DWORD64 base2 = 0x08010000; IMAGEHLP_MODULEW64 im; @@ -261,8 +262,8 @@ static void test_modules(void) machine_wow = get_module_machine(file_wow64); if (machine_wow != IMAGE_FILE_MACHINE_UNKNOWN) { - ret = SymLoadModule(GetCurrentProcess(), NULL, file_wow64, NULL, base2, 0); - ok(ret, "SymLoadModule failed: %lu\n", GetLastError()); + base = SymLoadModule(GetCurrentProcess(), NULL, file_wow64, NULL, base2, 0); + ok(base == base2, "SymLoadModule failed: %lu\n", GetLastError()); ret = SymGetModuleInfoW64(GetCurrentProcess(), base2, &im); if (!ret && skip_too_old_dbghelp(GetCurrentProcess(), base2)) return; ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); @@ -273,8 +274,8 @@ static void test_modules(void) GetSystemDirectoryA(file_system, MAX_PATH); strcat(file_system, "\notepad.exe");
- ret = SymLoadModule(GetCurrentProcess(), NULL, file_system, NULL, base1, 0); - ok(ret, "SymLoadModule failed: %lu\n", GetLastError()); + base = SymLoadModule(GetCurrentProcess(), NULL, file_system, NULL, base1, 0); + ok(base == base1, "SymLoadModule failed: %lu\n", GetLastError()); ret = SymGetModuleInfoW64(GetCurrentProcess(), base1, &im); if (!ret && skip_too_old_dbghelp(GetCurrentProcess(), base1)) return; ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError());
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/tests/Makefile.in | 2 +- dlls/dbghelp/tests/dbghelp.c | 138 ++++++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-)
diff --git a/dlls/dbghelp/tests/Makefile.in b/dlls/dbghelp/tests/Makefile.in index 31e5b01e8a8..40ec97eca8c 100644 --- a/dlls/dbghelp/tests/Makefile.in +++ b/dlls/dbghelp/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = dbghelp.dll -IMPORTS = dbghelp +IMPORTS = dbghelp user32
C_SRCS = \ dbghelp.c diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 4ddc7e7bc8a..015f045d213 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -16,12 +16,15 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
-#include "windef.h" +#include "windows.h" +#include "psapi.h" #include "verrsrc.h" #include "dbghelp.h" #include "wine/test.h" #include "winternl.h"
+static const BOOL is_win64 = sizeof(void*) > sizeof(int); + #if defined(__i386__) || defined(__x86_64__)
static DWORD CALLBACK stack_walk_thread(void *arg) @@ -297,6 +300,138 @@ static void test_modules(void) ok(ret, "SymCleanup failed: %lu\n", GetLastError()); }
+struct loaded_module_aggregation +{ + HANDLE proc; + unsigned int count_exe; +}; + +static BOOL CALLBACK aggregate_cb(PCWSTR imagename, DWORD64 base, ULONG sz, PVOID usr) +{ + struct loaded_module_aggregation* aggregation = usr; + IMAGEHLP_MODULEW64 im; + size_t image_len; + BOOL ret; + + memset(&im, 0, sizeof(im)); + im.SizeOfStruct = sizeof(im); + + image_len = wcslen(imagename); + + ret = SymGetModuleInfoW64(aggregation->proc, base, &im); + if (ret) + ok(aggregation->count_exe && image_len >= 4 && !wcscmp(imagename + image_len - 4, L".exe"), + "%ls shouldn't already be loaded\n", imagename); + else + { + ok(!ret, "Module %ls shouldn't be loaded\n", imagename); + ret = SymLoadModuleExW(aggregation->proc, NULL, imagename, NULL, base, sz, NULL, 0); + todo_wine + ok(ret, "SymLoadModuleExW failed on %ls: %lu\n", imagename, GetLastError()); + ret = SymGetModuleInfoW64(aggregation->proc, base, &im); + todo_wine + ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); + } + if (image_len >= 4 && !wcsicmp(imagename + image_len - 4, L".exe")) + aggregation->count_exe++; + + return TRUE; +} + + +static void test_loaded_modules(void) +{ + BOOL ret; + char buffer[200]; + PROCESS_INFORMATION pi = {0}; + STARTUPINFOA si = {0}; + struct loaded_module_aggregation aggregation = {0}; + + ret = GetSystemDirectoryA(buffer, sizeof(buffer)); + ok(ret, "got error %lu\n", GetLastError()); + strcat(buffer, "\notepad.exe"); + + /* testing with child process of different machines */ + ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "CreateProcess failed: %lu\n", GetLastError()); + + ret = WaitForInputIdle(pi.hProcess, 5000); + ok(!ret, "wait timed out\n"); + + ret = SymInitialize(pi.hProcess, NULL, FALSE); + ok(ret, "SymInitialize failed: %lu\n", GetLastError()); + memset(&aggregation, 0, sizeof(aggregation)); + aggregation.proc = pi.hProcess; + + ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation); + ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError()); + + SymCleanup(pi.hProcess); + TerminateProcess(pi.hProcess, 0); + + if (is_win64) + { + ret = GetSystemWow64DirectoryA(buffer, sizeof(buffer)); + ok(ret, "got error %lu\n", GetLastError()); + strcat(buffer, "\notepad.exe"); + + SymSetOptions(SymGetOptions() & ~SYMOPT_INCLUDE_32BIT_MODULES); + + ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + if (ret) + { + ret = WaitForInputIdle(pi.hProcess, 5000); + ok(!ret, "wait timed out\n"); + + ret = SymInitialize(pi.hProcess, NULL, FALSE); + ok(ret, "SymInitialize failed: %lu\n", GetLastError()); + memset(&aggregation, 0, sizeof(aggregation)); + aggregation.proc = pi.hProcess; + + ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation); + ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError()); + + SymCleanup(pi.hProcess); + TerminateProcess(pi.hProcess, 0); + } + else + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + skip("Skip wow64 test on non compatible platform\n"); + else + ok(ret, "CreateProcess failed: %lu\n", GetLastError()); + } + + SymSetOptions(SymGetOptions() | SYMOPT_INCLUDE_32BIT_MODULES); + + ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + if (ret) + { + struct loaded_module_aggregation aggregation2 = {0}; + + ret = WaitForInputIdle(pi.hProcess, 5000); + ok(!ret, "wait timed out\n"); + + ret = SymInitialize(pi.hProcess, NULL, FALSE); + ok(ret, "SymInitialize failed: %lu\n", GetLastError()); + memset(&aggregation2, 0, sizeof(aggregation2)); + aggregation2.proc = pi.hProcess; + ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation2); + ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError()); + + SymCleanup(pi.hProcess); + TerminateProcess(pi.hProcess, 0); + } + else + { + if (GetLastError() == ERROR_FILE_NOT_FOUND) + skip("Skip wow64 test on non compatible platform\n"); + else + ok(ret, "CreateProcess failed: %lu\n", GetLastError()); + } + } +} + START_TEST(dbghelp) { BOOL ret; @@ -315,4 +450,5 @@ START_TEST(dbghelp) ok(ret, "got error %lu\n", GetLastError());
test_modules(); + test_loaded_modules(); }
From: Eric Pouech eric.pouech@gmail.com
Even is MSDN states that it enumerates modules' name, the first parameter to the callback is the fullpath to the image. So - fix EnumerateLoadedModules() to pass the image name for the considered module - fix all callbacks in Wine code to EnumerateLoadedModules to handle the image name instead of module name
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/dbghelp.c | 6 +----- dlls/dbghelp/minidump.c | 8 ++------ dlls/dbghelp/module.c | 7 +++---- dlls/dbghelp/tests/dbghelp.c | 2 -- 4 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/dlls/dbghelp/dbghelp.c b/dlls/dbghelp/dbghelp.c index 67ac7765c6f..81a0d50f25d 100644 --- a/dlls/dbghelp/dbghelp.c +++ b/dlls/dbghelp/dbghelp.c @@ -306,13 +306,9 @@ BOOL WINAPI SymGetSearchPath(HANDLE hProcess, PSTR szSearchPath, */ static BOOL WINAPI process_invade_cb(PCWSTR name, ULONG64 base, ULONG size, PVOID user) { - WCHAR tmp[MAX_PATH]; HANDLE hProcess = user;
- if (!GetModuleFileNameExW(hProcess, (HMODULE)(DWORD_PTR)base, tmp, ARRAY_SIZE(tmp))) - lstrcpynW(tmp, name, ARRAY_SIZE(tmp)); - - SymLoadModuleExW(hProcess, 0, tmp, name, base, size, NULL, 0); + SymLoadModuleExW(hProcess, 0, name, NULL, base, size, NULL, 0); return TRUE; }
diff --git a/dlls/dbghelp/minidump.c b/dlls/dbghelp/minidump.c index 4d3818a8f54..11ce767aa71 100644 --- a/dlls/dbghelp/minidump.c +++ b/dlls/dbghelp/minidump.c @@ -213,12 +213,8 @@ static BOOL add_module(struct dump_context* dc, const WCHAR* name, dc->alloc_modules = dc->num_modules = 0; return FALSE; } - if (is_elf || - !GetModuleFileNameExW(dc->process->handle, (HMODULE)(DWORD_PTR)base, - dc->modules[dc->num_modules].name, - ARRAY_SIZE(dc->modules[dc->num_modules].name))) - lstrcpynW(dc->modules[dc->num_modules].name, name, - ARRAY_SIZE(dc->modules[dc->num_modules].name)); + lstrcpynW(dc->modules[dc->num_modules].name, name, + ARRAY_SIZE(dc->modules[dc->num_modules].name)); dc->modules[dc->num_modules].base = base; dc->modules[dc->num_modules].size = size; dc->modules[dc->num_modules].timestamp = timestamp; diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 5b99b5eea58..4bd7b0e5009 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -1270,7 +1270,7 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE hProcess, PVOID UserContext) { HMODULE* hMods; - WCHAR baseW[256], modW[256]; + WCHAR imagenameW[MAX_PATH]; DWORD i, sz; MODULEINFO mi; BOOL wow64; @@ -1300,10 +1300,9 @@ BOOL WINAPI EnumerateLoadedModulesW64(HANDLE hProcess, for (i = 0; i < sz; i++) { if (!GetModuleInformation(hProcess, hMods[i], &mi, sizeof(mi)) || - !GetModuleBaseNameW(hProcess, hMods[i], baseW, ARRAY_SIZE(baseW))) + !GetModuleFileNameExW(hProcess, hMods[i], imagenameW, ARRAY_SIZE(imagenameW))) continue; - module_fill_module(baseW, modW, ARRAY_SIZE(modW)); - EnumLoadedModulesCallback(modW, (DWORD_PTR)mi.lpBaseOfDll, mi.SizeOfImage, + EnumLoadedModulesCallback(imagenameW, (DWORD_PTR)mi.lpBaseOfDll, mi.SizeOfImage, UserContext); } HeapFree(GetProcessHeap(), 0, hMods); diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 015f045d213..8f2d6ce1991 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -326,10 +326,8 @@ static BOOL CALLBACK aggregate_cb(PCWSTR imagename, DWORD64 base, ULONG sz, PVOI { ok(!ret, "Module %ls shouldn't be loaded\n", imagename); ret = SymLoadModuleExW(aggregation->proc, NULL, imagename, NULL, base, sz, NULL, 0); - todo_wine ok(ret, "SymLoadModuleExW failed on %ls: %lu\n", imagename, GetLastError()); ret = SymGetModuleInfoW64(aggregation->proc, base, &im); - todo_wine ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); } if (image_len >= 4 && !wcsicmp(imagename + image_len - 4, L".exe"))
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/tests/dbghelp.c | 79 +++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-)
diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 8f2d6ce1991..ccabaf5b55f 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -303,7 +303,12 @@ static void test_modules(void) struct loaded_module_aggregation { HANDLE proc; + unsigned int count_32bit; + unsigned int count_64bit; unsigned int count_exe; + unsigned int count_ntdll; + unsigned int count_systemdir; + unsigned int count_wowdir; };
static BOOL CALLBACK aggregate_cb(PCWSTR imagename, DWORD64 base, ULONG sz, PVOID usr) @@ -311,7 +316,8 @@ static BOOL CALLBACK aggregate_cb(PCWSTR imagename, DWORD64 base, ULONG sz, PVOI struct loaded_module_aggregation* aggregation = usr; IMAGEHLP_MODULEW64 im; size_t image_len; - BOOL ret; + BOOL ret, wow64; + WCHAR buffer[MAX_PATH];
memset(&im, 0, sizeof(im)); im.SizeOfStruct = sizeof(im); @@ -330,8 +336,35 @@ static BOOL CALLBACK aggregate_cb(PCWSTR imagename, DWORD64 base, ULONG sz, PVOI ret = SymGetModuleInfoW64(aggregation->proc, base, &im); ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); } + + switch (im.MachineType) + { + case IMAGE_FILE_MACHINE_UNKNOWN: + break; + case IMAGE_FILE_MACHINE_I386: + case IMAGE_FILE_MACHINE_ARM: + case IMAGE_FILE_MACHINE_ARMNT: + aggregation->count_32bit++; + break; + case IMAGE_FILE_MACHINE_AMD64: + case IMAGE_FILE_MACHINE_ARM64: + aggregation->count_64bit++; + break; + default: + ok(0, "Unsupported machine %lx\n", im.MachineType); + break; + } if (image_len >= 4 && !wcsicmp(imagename + image_len - 4, L".exe")) aggregation->count_exe++; + if (!wcsicmp(im.ModuleName, L"ntdll")) + aggregation->count_ntdll++; + if (GetSystemDirectoryW(buffer, ARRAY_SIZE(buffer)) && + !wcsnicmp(imagename, buffer, wcslen(buffer))) + aggregation->count_systemdir++; + if (is_win64 && IsWow64Process(aggregation->proc, &wow64) && wow64 && + GetSystemWow64DirectoryW(buffer, ARRAY_SIZE(buffer)) && + !wcsnicmp(imagename, buffer, wcslen(buffer))) + aggregation->count_wowdir++;
return TRUE; } @@ -364,6 +397,30 @@ static void test_loaded_modules(void) ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation); ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());
+ if (is_win64) + { + ok(!aggregation.count_32bit && aggregation.count_64bit, "Wrong bitness aggregation count %u %u\n", + aggregation.count_32bit, aggregation.count_64bit); + ok(aggregation.count_exe == 1 && aggregation.count_ntdll == 1, "Wrong kind aggregation count %u %u\n", + aggregation.count_exe, aggregation.count_ntdll); + ok(aggregation.count_systemdir > 2 && !aggregation.count_wowdir, "Wrong directory aggregation count %u %u\n", + aggregation.count_systemdir, aggregation.count_wowdir); + } + else + { + BOOL is_wow64; + ret = IsWow64Process(pi.hProcess, &is_wow64); + ok(ret, "IsWow64Process failed: %lu\n", GetLastError()); + + ok(aggregation.count_32bit && !aggregation.count_64bit, "Wrong bitness aggregation count %u %u\n", + aggregation.count_32bit, aggregation.count_64bit); + ok(aggregation.count_exe == 1 && aggregation.count_ntdll == 1, "Wrong kind aggregation count %u %u\n", + aggregation.count_exe, aggregation.count_ntdll); + todo_wine_if(is_wow64) + ok(aggregation.count_systemdir > 2 && !aggregation.count_wowdir, "Wrong directory aggregation count %u %u\n", + aggregation.count_systemdir, aggregation.count_wowdir); + } + SymCleanup(pi.hProcess); TerminateProcess(pi.hProcess, 0);
@@ -389,6 +446,16 @@ static void test_loaded_modules(void) ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation); ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());
+ todo_wine + ok(aggregation.count_32bit == 1 && aggregation.count_64bit, "Wrong bitness aggregation count %u %u\n", + aggregation.count_32bit, aggregation.count_64bit); + ok(aggregation.count_exe == 1 && aggregation.count_ntdll == 1, "Wrong kind aggregation count %u %u\n", + aggregation.count_exe, aggregation.count_ntdll); + todo_wine + ok(aggregation.count_systemdir > 2 && aggregation.count_64bit == aggregation.count_systemdir && aggregation.count_wowdir == 1, + "Wrong directory aggregation count %u %u\n", + aggregation.count_systemdir, aggregation.count_wowdir); + SymCleanup(pi.hProcess); TerminateProcess(pi.hProcess, 0); } @@ -417,6 +484,16 @@ static void test_loaded_modules(void) ret = EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation2); ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());
+ ok(aggregation2.count_32bit && aggregation2.count_64bit, "Wrong bitness aggregation count %u %u\n", + aggregation2.count_32bit, aggregation2.count_64bit); + todo_wine + ok(aggregation2.count_exe == 2 && aggregation2.count_ntdll == 2, "Wrong kind aggregation count %u %u\n", + aggregation2.count_exe, aggregation2.count_ntdll); + todo_wine + ok(aggregation2.count_systemdir > 2 && aggregation2.count_64bit == aggregation2.count_systemdir && aggregation2.count_wowdir > 2, + "Wrong directory aggregation count %u %u\n", + aggregation2.count_systemdir, aggregation2.count_wowdir); + SymCleanup(pi.hProcess); TerminateProcess(pi.hProcess, 0); }
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/tests/dbghelp.c | 190 +++++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+)
diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index ccabaf5b55f..6e66b1d681f 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -239,6 +239,79 @@ static BOOL skip_too_old_dbghelp(HANDLE proc, DWORD64 base) return FALSE; }
+static DWORD get_module_size(const char* path) +{ + BOOL ret; + HANDLE hFile, hMap; + void* mapping; + IMAGE_NT_HEADERS *nthdr; + DWORD size; + + hFile = CreateFileA(path, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL); + ok(hFile != INVALID_HANDLE_VALUE, "Couldn't open file %s (%lu)\n", path, GetLastError()); + hMap = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL); + ok(hMap != NULL, "Couldn't create map (%lu)\n", GetLastError()); + mapping = MapViewOfFile(hMap, FILE_MAP_READ, 0, 0, 0); + ok(mapping != NULL, "Couldn't map (%lu)\n", GetLastError()); + nthdr = RtlImageNtHeader(mapping); + ok(nthdr != NULL, "Cannot get NT headers out of %s\n", path); + size = nthdr ? nthdr->OptionalHeader.SizeOfImage : 0; + ret = UnmapViewOfFile(mapping); + ok(ret, "Couldn't unmap (%lu)\n", GetLastError()); + CloseHandle(hMap); + CloseHandle(hFile); + return size; +} + +static BOOL CALLBACK count_module_cb(const char* name, DWORD64 base, void* usr) +{ + (*(unsigned*)usr)++; + return TRUE; +} + +static unsigned get_module_count(HANDLE proc) +{ + unsigned count = 0; + BOOL ret; + + ret = SymEnumerateModules64(proc, count_module_cb, &count); + ok(ret, "SymEnumerateModules64 failed: %lu\n", GetLastError()); + return count; +} + +struct nth_module +{ + HANDLE proc; + unsigned int index; + BOOL will_fail; + IMAGEHLP_MODULE64 module; + BOOL with_todo_wine; +}; + +static BOOL CALLBACK nth_module_cb(const char* name, DWORD64 base, void* usr) +{ + struct nth_module* nth = usr; + BOOL ret; + + if (nth->index--) return TRUE; + nth->module.SizeOfStruct = sizeof(nth->module); + ret = SymGetModuleInfo64(nth->proc, base, &nth->module); + todo_wine_if(nth->with_todo_wine) + { + if (nth->will_fail) + { + ok(!ret, "SymGetModuleInfo64 should have failed\n"); + nth->module.BaseOfImage = base; + } + else + { + ok(ret, "SymGetModuleInfo64 failed: %lu\n", GetLastError()); + ok(nth->module.BaseOfImage == base, "Wrong base\n"); + } + } + return FALSE; +} + static void test_modules(void) { BOOL ret; @@ -249,6 +322,9 @@ static void test_modules(void) const DWORD64 base2 = 0x08010000; IMAGEHLP_MODULEW64 im; USHORT machine_wow, machine2; + HANDLE dummy = (HANDLE)(ULONG_PTR)0xcafef00d; + const char* target_dll = "c:\windows\system32\kernel32.dll"; + unsigned count;
im.SizeOfStruct = sizeof(im);
@@ -298,6 +374,119 @@ static void test_modules(void)
ret = SymCleanup(GetCurrentProcess()); ok(ret, "SymCleanup failed: %lu\n", GetLastError()); + + ret = SymInitialize(dummy, NULL, FALSE); + ok(ret, "got error %lu\n", GetLastError()); + + count = get_module_count(dummy); + ok(count == 0, "Unexpected count (%u instead of 0)\n", count); + + /* loading with 0 size succeeds */ + base = SymLoadModuleEx(dummy, NULL, target_dll, NULL, base1, 0, NULL, 0); + ok(base == base1, "SymLoadModuleEx failed: %lu\n", GetLastError()); + + ret = SymUnloadModule64(dummy, base1); + ok(ret, "SymUnloadModule64 failed: %lu\n", GetLastError()); + + count = get_module_count(dummy); + ok(count == 0, "Unexpected count (%u instead of 0)\n", count); + + ret = SymCleanup(dummy); + ok(ret, "SymCleanup failed: %lu\n", GetLastError()); + +} + +static void test_modules_overlap(void) +{ + BOOL ret; + DWORD64 base; + const DWORD64 base1 = 0x00010000; + HANDLE dummy = (HANDLE)(ULONG_PTR)0xcafef00d; + const char* target1_dll = "c:\windows\system32\kernel32.dll"; + const char* target2_dll = "c:\windows\system32\winmm.dll"; + DWORD imsize = get_module_size(target1_dll); + int i, j; + struct test_module + { + DWORD64 base; + DWORD size; + const char* name; + }; + const struct test + { + struct test_module input; + DWORD error_code; + struct test_module outputs[2]; + } + tests[] = + { + /* cases where first module is left "untouched" and second not loaded */ + {{base1, 0, target1_dll}, ERROR_SUCCESS, {{base1, imsize, "kernel32"},{0}}}, + {{base1, imsize, target1_dll}, ERROR_SUCCESS, {{base1, imsize, "kernel32"},{0}}}, + {{base1, imsize / 2, target1_dll}, ERROR_SUCCESS, {{base1, imsize, "kernel32"},{0}}}, + {{base1, imsize * 2, target1_dll}, ERROR_SUCCESS, {{base1, imsize, "kernel32"},{0}}}, + + /* cases where first module is unloaded and replaced by second module */ + {{base1 + imsize / 2, imsize, target1_dll}, ~0, {{base1 + imsize / 2, imsize, "kernel32"},{0}}}, + {{base1 + imsize / 3, imsize / 3, target1_dll}, ~0, {{base1 + imsize / 3, imsize / 3, "kernel32"},{0}}}, + {{base1 + imsize / 2, imsize, target2_dll}, ~0, {{base1 + imsize / 2, imsize, "winmm"},{0}}}, + {{base1 + imsize / 3, imsize / 3, target2_dll}, ~0, {{base1 + imsize / 3, imsize / 3, "winmm"},{0}}}, + + /* cases where second module is actually loaded */ + {{base1 + imsize, imsize, target1_dll}, ~0, {{base1, imsize, "kernel32"}, {base1 + imsize, imsize, "kernel32"}}}, + {{base1 - imsize / 2, imsize, target1_dll}, ~0, {{base1, imsize, "kernel32"}, {base1 - imsize / 2, imsize, NULL}}}, + /* we mark with a NULL modulename the cases where the module is loaded, but isn't visible + * (SymGetModuleInfo fails in callback) as it's base address is inside the first loaded module. + */ + {{base1 + imsize, imsize, target2_dll}, ~0, {{base1, imsize, "kernel32"}, {base1 + imsize, imsize, "winmm"}}}, + {{base1 - imsize / 2, imsize, target2_dll}, ~0, {{base1, imsize, "kernel32"}, {base1 - imsize / 2, imsize, NULL}}}, + }; + + ok(imsize, "Cannot get image size\n"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + ret = SymInitialize(dummy, NULL, FALSE); + ok(ret, "SymInitialize failed: %lu\n", GetLastError()); + + base = SymLoadModuleEx(dummy, NULL, target1_dll, NULL, base1, 0, NULL, 0); + ok(base == base1, "SymLoadModuleEx failed: %lu\n", GetLastError()); + base = SymLoadModuleEx(dummy, NULL, tests[i].input.name, NULL, tests[i].input.base, tests[i].input.size, NULL, 0); + if (tests[i].error_code != ~0) + { + ok(base == 0, "SymLoadModuleEx should have failed\n"); + ok(GetLastError() == tests[i].error_code, "Wrong error %lu\n", GetLastError()); + } + else + { + todo_wine_if(i == 6 || i == 7) + ok(base == tests[i].input.base, "SymLoadModuleEx failed: %lu\n", GetLastError()); + } + for (j = 0; j < ARRAY_SIZE(tests[i].outputs); j++) + { + struct nth_module nth = {dummy, j, !tests[i].outputs[j].name, {0}, i == 9 || i == 11}; + + ret = SymEnumerateModules64(dummy, nth_module_cb, &nth); + ok(ret, "SymEnumerateModules64 failed: %lu\n", GetLastError()); + if (!tests[i].outputs[j].base) + { + ok(nth.index != -1, "Got more modules than expected %d, %d\n", nth.index, j); + break; + } + ok(nth.index == -1, "Expecting more modules\n"); + todo_wine_if(i >= 6) + ok(nth.module.BaseOfImage == tests[i].outputs[j].base, "Wrong base\n"); + if (!nth.will_fail) + { + todo_wine_if(i == 7 || i == 9 || i == 11) + ok(nth.module.ImageSize == tests[i].outputs[j].size, "Wrong size\n"); + todo_wine_if(i >= 6 && i != 8) + ok(!strcasecmp(nth.module.ModuleName, tests[i].outputs[j].name), "Wrong name\n"); + } + } + ret = SymCleanup(dummy); + ok(ret, "SymCleanup failed: %lu\n", GetLastError()); + } }
struct loaded_module_aggregation @@ -525,5 +714,6 @@ START_TEST(dbghelp) ok(ret, "got error %lu\n", GetLastError());
test_modules(); + test_modules_overlap(); test_loaded_modules(); }
From: Eric Pouech eric.pouech@gmail.com
This preserves order in which modules are loaded, and enumeration is done according to this order.
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/dbghelp.c | 6 ++---- dlls/dbghelp/module.c | 6 ++++-- dlls/dbghelp/tests/dbghelp.c | 12 ++++-------- 3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/dlls/dbghelp/dbghelp.c b/dlls/dbghelp/dbghelp.c index 81a0d50f25d..3341bea442e 100644 --- a/dlls/dbghelp/dbghelp.c +++ b/dlls/dbghelp/dbghelp.c @@ -333,10 +333,8 @@ const struct cpu* process_get_cpu(const struct process* pcs) { const struct module* m = pcs->lmodules;
- /* main module is the last one in list */ - if (!m) return dbghelp_current_cpu; - while (m->next) m = m->next; - return m->cpu; + /* return cpu of main module, which is the first module in process's modules list */ + return (m) ? m->cpu : dbghelp_current_cpu; }
/****************************************************************** diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index 4bd7b0e5009..b6f36b1f28a 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -177,14 +177,16 @@ struct module* module_new(struct process* pcs, const WCHAR* name, ULONG_PTR stamp, ULONG_PTR checksum, WORD machine) { struct module* module; + struct module** pmodule; unsigned i;
assert(type == DMT_ELF || type == DMT_PE || type == DMT_MACHO); if (!(module = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*module)))) return NULL;
- module->next = pcs->lmodules; - pcs->lmodules = module; + for (pmodule = &pcs->lmodules; *pmodule; pmodule = &(*pmodule)->next); + module->next = NULL; + *pmodule = module;
TRACE("=> %s %I64x-%I64x %s\n", get_module_type(type, virtual), mod_addr, mod_addr + size, debugstr_w(name)); diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 6e66b1d681f..1775459eae8 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -285,7 +285,6 @@ struct nth_module unsigned int index; BOOL will_fail; IMAGEHLP_MODULE64 module; - BOOL with_todo_wine; };
static BOOL CALLBACK nth_module_cb(const char* name, DWORD64 base, void* usr) @@ -296,8 +295,6 @@ static BOOL CALLBACK nth_module_cb(const char* name, DWORD64 base, void* usr) if (nth->index--) return TRUE; nth->module.SizeOfStruct = sizeof(nth->module); ret = SymGetModuleInfo64(nth->proc, base, &nth->module); - todo_wine_if(nth->with_todo_wine) - { if (nth->will_fail) { ok(!ret, "SymGetModuleInfo64 should have failed\n"); @@ -308,7 +305,6 @@ static BOOL CALLBACK nth_module_cb(const char* name, DWORD64 base, void* usr) ok(ret, "SymGetModuleInfo64 failed: %lu\n", GetLastError()); ok(nth->module.BaseOfImage == base, "Wrong base\n"); } - } return FALSE; }
@@ -464,7 +460,7 @@ static void test_modules_overlap(void) } for (j = 0; j < ARRAY_SIZE(tests[i].outputs); j++) { - struct nth_module nth = {dummy, j, !tests[i].outputs[j].name, {0}, i == 9 || i == 11}; + struct nth_module nth = {dummy, j, !tests[i].outputs[j].name, {0}};
ret = SymEnumerateModules64(dummy, nth_module_cb, &nth); ok(ret, "SymEnumerateModules64 failed: %lu\n", GetLastError()); @@ -474,13 +470,13 @@ static void test_modules_overlap(void) break; } ok(nth.index == -1, "Expecting more modules\n"); - todo_wine_if(i >= 6) + todo_wine_if(i == 6 || i == 7) ok(nth.module.BaseOfImage == tests[i].outputs[j].base, "Wrong base\n"); if (!nth.will_fail) { - todo_wine_if(i == 7 || i == 9 || i == 11) + todo_wine_if(i == 7) ok(nth.module.ImageSize == tests[i].outputs[j].size, "Wrong size\n"); - todo_wine_if(i >= 6 && i != 8) + todo_wine_if(i == 6 || i == 7) ok(!strcasecmp(nth.module.ModuleName, tests[i].outputs[j].name), "Wrong name\n"); } }
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- dlls/dbghelp/module.c | 20 +++++++------------- dlls/dbghelp/tests/dbghelp.c | 4 ---- 2 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c index b6f36b1f28a..ad8ec22fe32 100644 --- a/dlls/dbghelp/module.c +++ b/dlls/dbghelp/module.c @@ -992,17 +992,10 @@ DWORD64 WINAPI SymLoadModuleExW(HANDLE hProcess, HANDLE hFile, PCWSTR wImageNam } if (altmodule) { - /* we have a conflict as the new module cannot be found by its base address - * we need to get rid of one on the two modules + /* We have a conflict as the new module cannot be found by its base address + * (it's hidden by altmodule). + * We need to decide which one the two modules we need to get rid of. */ - if (lstrcmpW(module->modulename, altmodule->modulename) != 0) - { - /* module overlaps an existing but different module... unload new module and return error */ - WARN("%ls overlaps %ls\n", module->modulename, altmodule->modulename); - module_remove(pcs, module); - SetLastError(ERROR_INVALID_PARAMETER); - return 0; - } /* loading same module at same address... don't change anything */ if (module->module.BaseOfImage == altmodule->module.BaseOfImage) { @@ -1010,9 +1003,10 @@ DWORD64 WINAPI SymLoadModuleExW(HANDLE hProcess, HANDLE hFile, PCWSTR wImageNam SetLastError(ERROR_SUCCESS); return 0; } - /* replace old module with new one, which will look like a shift of base address */ - WARN("Shift module %ls from %I64x to %I64x\n", - module->modulename, altmodule->module.BaseOfImage, module->module.BaseOfImage); + /* replace old module with new one */ + WARN("Replace module %ls at %I64x by module %ls at %I64x\n", + altmodule->module.ImageName, altmodule->module.BaseOfImage, + module->module.ImageName, module->module.BaseOfImage); module_remove(pcs, altmodule); }
diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c index 1775459eae8..88d23c053ce 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -455,7 +455,6 @@ static void test_modules_overlap(void) } else { - todo_wine_if(i == 6 || i == 7) ok(base == tests[i].input.base, "SymLoadModuleEx failed: %lu\n", GetLastError()); } for (j = 0; j < ARRAY_SIZE(tests[i].outputs); j++) @@ -470,13 +469,10 @@ static void test_modules_overlap(void) break; } ok(nth.index == -1, "Expecting more modules\n"); - todo_wine_if(i == 6 || i == 7) ok(nth.module.BaseOfImage == tests[i].outputs[j].base, "Wrong base\n"); if (!nth.will_fail) { - todo_wine_if(i == 7) ok(nth.module.ImageSize == tests[i].outputs[j].size, "Wrong size\n"); - todo_wine_if(i == 6 || i == 7) ok(!strcasecmp(nth.module.ModuleName, tests[i].outputs[j].name), "Wrong name\n"); } }
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=129484
Your paranoid android.
=== w7u_2qxl (32 bit report) ===
dbghelp: dbghelp.c:596: Test failed: Wrong bitness aggregation count 0 0 dbghelp.c:598: Test failed: Wrong kind aggregation count 1 0
=== w7u_adm (32 bit report) ===
dbghelp: dbghelp.c:596: Test failed: Wrong bitness aggregation count 0 0 dbghelp.c:598: Test failed: Wrong kind aggregation count 1 0
=== w7u_el (32 bit report) ===
dbghelp: dbghelp.c:596: Test failed: Wrong bitness aggregation count 0 0 dbghelp.c:598: Test failed: Wrong kind aggregation count 1 0
-
Eric Pouech -
eric pouech (＠epo)
-
Marvin