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)
-- v2: dbghelp: Unload overlapping modules in SymLoadModule*(). dbghelp: Add new module at end of the process' modules list. dbghelp: Add some more tests about module handling. dbghelp: Add test for loaded modules enumeration. dbghelp: Let EnumerateLoadedModules() expose image names. dbghelp: Add tests for 'module' name in EnumLoadedModules() callback. dbghelp/tests: Test return value of SymLoadModule.
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 | 149 +++++++++++++++++++++++++++++++-- 2 files changed, 145 insertions(+), 6 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..141c797abfc 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) @@ -236,7 +239,7 @@ static BOOL skip_too_old_dbghelp(HANDLE proc, DWORD64 base) return FALSE; }
-static void test_modules(void) +static BOOL test_modules(void) { BOOL ret; char file_system[MAX_PATH]; @@ -265,7 +268,7 @@ static void test_modules(void) 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; + if (!ret && skip_too_old_dbghelp(GetCurrentProcess(), base2)) return FALSE; ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); ok(im.BaseOfImage == base2, "Wrong base address\n"); ok(im.MachineType == machine_wow, "Wrong machine %lx (expecting %u)\n", im.MachineType, machine_wow); @@ -277,7 +280,7 @@ static void test_modules(void) 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; + if (!ret && skip_too_old_dbghelp(GetCurrentProcess(), base1)) return FALSE; ok(ret, "SymGetModuleInfoW64 failed: %lu\n", GetLastError()); ok(im.BaseOfImage == base1, "Wrong base address\n"); machine2 = get_module_machine(file_system); @@ -295,6 +298,139 @@ static void test_modules(void)
ret = SymCleanup(GetCurrentProcess()); ok(ret, "SymCleanup failed: %lu\n", GetLastError()); + return TRUE; +} + +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) @@ -314,5 +450,8 @@ START_TEST(dbghelp) ret = SymCleanup(GetCurrentProcess()); ok(ret, "got error %lu\n", GetLastError());
- test_modules(); + if (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 141c797abfc..bed4096cac2 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -327,10 +327,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 bed4096cac2..030fdf4bc0e 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -304,7 +304,12 @@ static BOOL 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) @@ -312,7 +317,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); @@ -331,8 +337,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; } @@ -365,6 +398,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);
@@ -390,6 +447,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); } @@ -418,6 +485,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 030fdf4bc0e..b7ecb790441 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 BOOL test_modules(void) { BOOL ret; @@ -249,6 +322,9 @@ static BOOL 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,9 +374,122 @@ static BOOL 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()); + return TRUE; }
+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 { HANDLE proc; @@ -527,6 +716,7 @@ START_TEST(dbghelp)
if (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 b7ecb790441..6b7c6b8d5b8 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; }
@@ -465,7 +461,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()); @@ -475,13 +471,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 6b7c6b8d5b8..83805f3ed10 100644 --- a/dlls/dbghelp/tests/dbghelp.c +++ b/dlls/dbghelp/tests/dbghelp.c @@ -456,7 +456,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++) @@ -471,13 +470,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"); } }
V2: - fix for failing Win7 tests (actually skip as done for previous tests)
-
Eric Pouech -
eric pouech (＠epo)