dbghelp: Implement SymRefreshModuleList().

Signed-off-by: Eric Pouech <epouech@codeweavers.com>

dlls/dbghelp/dbghelp.c

@@ -300,20 +300,6 @@ BOOL WINAPI SymGetSearchPath(HANDLE hProcess, PSTR szSearchPath,
     return ret;
 }
 
-/******************************************************************
- *		invade_process
- *
- * SymInitialize helper: loads in dbghelp all known (and loaded modules)
- * this assumes that hProcess is a handle on a valid process
- */
-static BOOL WINAPI process_invade_cb(PCWSTR name, ULONG64 base, ULONG size, PVOID user)
-{
-    HANDLE      hProcess = user;
-
-    SymLoadModuleExW(hProcess, 0, name, NULL, base, size, NULL, 0);
-    return TRUE;
-}
-
 const WCHAR *process_getenv(const struct process *process, const WCHAR *name)
 {
     size_t name_len;
@@ -432,7 +418,10 @@ static BOOL check_live_target(struct process* pcs, BOOL wow64, BOOL child_wow64)
 
     TRACE("got debug info address %#I64x from PEB %p\n", base, pbi.PebBaseAddress);
     if (!elf_read_wine_loader_dbg_info(pcs, base) && !macho_read_wine_loader_dbg_info(pcs, base))
+    {
         WARN("couldn't load process debug info at %#I64x\n", base);
+        pcs->loader = &empty_loader_ops;
+    }
     return TRUE;
 }
 
@@ -510,8 +499,9 @@ BOOL WINAPI SymInitializeW(HANDLE hProcess, PCWSTR UserSearchPath, BOOL fInvadeP
     if (check_live_target(pcs, wow64, child_wow64))
     {
         if (fInvadeProcess)
-            EnumerateLoadedModulesW64(hProcess, process_invade_cb, hProcess);
-        if (pcs->loader) pcs->loader->synchronize_module_list(pcs);
+            module_refresh_list(pcs);
+        else
+            pcs->loader->synchronize_module_list(pcs);
     }
     else if (fInvadeProcess)
     {

dlls/dbghelp/dbghelp_private.h

@@ -743,6 +743,7 @@ extern BOOL         module_remove(struct process* pcs,
 extern void         module_set_module(struct module* module, const WCHAR* name);
 extern WCHAR*       get_wine_loader_name(struct process *pcs) __WINE_DEALLOC(HeapFree, 3) __WINE_MALLOC;
 extern BOOL         module_is_wine_host(const WCHAR* module_name, const WCHAR* ext);
+extern BOOL         module_refresh_list(struct process *pcs);
 
 /* msc.c */
 extern BOOL         pe_load_debug_directory(const struct process* pcs,

dlls/dbghelp/elf_module.c

@@ -1785,7 +1785,6 @@ BOOL elf_read_wine_loader_dbg_info(struct process* pcs, ULONG_PTR addr)
     {
         ERR("Unable to access ELF libraries (outside 32bit limit)\n");
         module_remove(pcs, elf_info.module);
-        pcs->loader = &empty_loader_ops;
         return FALSE;
     }
     TRACE("Found ELF debug header %#I64x\n", elf_info.dbg_hdr_addr);

dlls/dbghelp/module.c

@@ -24,6 +24,8 @@
 #include <string.h>
 #include <assert.h>
 
+#include "ntstatus.h"
+#define WIN32_NO_STATUS
 #include "dbghelp_private.h"
 #include "image_private.h"
 #include "psapi.h"
@@ -1317,7 +1319,11 @@ BOOL  WINAPI EnumerateLoadedModulesW64(HANDLE process,
     size_t              sysdir_len = 0, wowdir_len = 0;
 
     /* process might not be a handle to a live process */
-    if (!IsWow64Process2(process, &pcs_machine, &native_machine)) return FALSE;
+    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);
@@ -1600,6 +1606,30 @@ void module_reset_debug_info(struct module* module)
     module->sources = NULL;
 }
 
+static BOOL WINAPI process_invade_cb(PCWSTR name, ULONG64 base, ULONG size, PVOID user)
+{
+    HANDLE      hProcess = user;
+
+    /* Note: this follows native behavior:
+     * If a PE module has been unloaded from debuggee, it's not immediately removed
+     * from module list in dbghelp.
+     * Removal may eventually happen when loading a another module with SymLoadModule:
+     * if the module to be loaded overlaps an existing one, SymLoadModule will
+     * automatically unload the eldest one.
+     */
+    SymLoadModuleExW(hProcess, 0, name, NULL, base, size, NULL, 0);
+    return TRUE;
+}
+
+BOOL module_refresh_list(struct process *pcs)
+{
+    BOOL ret;
+
+    ret = pcs->loader->synchronize_module_list(pcs);
+    ret = EnumerateLoadedModulesW64(pcs->handle, process_invade_cb, pcs->handle) && ret;
+    return ret;
+}
+
 /******************************************************************
  *              SymRefreshModuleList (DBGHELP.@)
  */
@@ -1610,8 +1640,7 @@ BOOL WINAPI SymRefreshModuleList(HANDLE hProcess)
     TRACE("(%p)\n", hProcess);
 
     if (!(pcs = process_find_by_handle(hProcess))) return FALSE;
-
-    return pcs->loader->synchronize_module_list(pcs);
+    return module_refresh_list(pcs);
 }
 
 /***********************************************************************

dlls/dbghelp/tests/dbghelp.c

@@ -372,6 +372,29 @@ static unsigned get_native_module_count(HANDLE proc)
     return count;
 }
 
+struct module_present
+{
+    const WCHAR* module_name;
+    BOOL found;
+};
+
+static BOOL CALLBACK is_module_present_cb(const WCHAR* name, DWORD64 base, void* usr)
+{
+    struct module_present* present = usr;
+    if (!wcsicmp(name, present->module_name))
+    {
+        present->found = TRUE;
+        return FALSE;
+    }
+    return TRUE;
+}
+
+static BOOL is_module_present(HANDLE proc, const WCHAR* module_name)
+{
+    struct module_present present = { .module_name = module_name };
+    return SymEnumerateModulesW64(proc, is_module_present_cb, &present) && present.found;
+}
+
 struct nth_module
 {
     HANDLE              proc;
@@ -424,6 +447,28 @@ static BOOL wrapper_EnumerateLoadedModulesW64(HANDLE proc, PENUMLOADED_MODULES_C
     return ret;
 }
 
+/* wrapper around SymRefreshModuleList which sometimes fails (it's very likely implemented on top
+ * of EnumerateLoadedModulesW64 on native too)
+ */
+static BOOL wrapper_SymRefreshModuleList(HANDLE proc)
+{
+    BOOL ret;
+    int retry;
+    int retry_count = !strcmp(winetest_platform, "wine") ? 1 : 5;
+
+    for (retry = retry_count - 1; retry >= 0; retry--)
+    {
+        ret = SymRefreshModuleList(proc);
+        if (ret || (GetLastError() != STATUS_INFO_LENGTH_MISMATCH && GetLastError() == STATUS_PARTIAL_COPY))
+            break;
+        Sleep(10);
+    }
+    if (retry + 1 < retry_count)
+        trace("used wrapper retry: ret=%d retry=%d top=%d\n", ret, retry, retry_count);
+
+    return ret;
+}
+
 static BOOL test_modules(void)
 {
     BOOL ret;
@@ -492,6 +537,7 @@ static BOOL test_modules(void)
 
     ret = SymRefreshModuleList(dummy);
     ok(!ret, "SymRefreshModuleList should have failed\n");
+    ok(GetLastError() == STATUS_INVALID_CID, "Unexpected last error %lx\n", GetLastError());
 
     count = get_module_count(dummy);
     ok(count == 0, "Unexpected count (%u instead of 0)\n", count);
@@ -825,6 +871,11 @@ static void test_loaded_modules(void)
     ok(ret, "got error %lu\n", GetLastError());
     strcat(buffer, "\\msinfo32.exe");
 
+    /* testing invalid process handle */
+    ret = wrapper_EnumerateLoadedModulesW64((HANDLE)(ULONG_PTR)0xffffffc0, NULL, FALSE);
+    ok(!ret, "EnumerateLoadedModulesW64 should have failed\n");
+    ok(GetLastError() == STATUS_INVALID_CID, "Unexpected last error %lx\n", GetLastError());
+
     /* 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());
@@ -885,9 +936,10 @@ static void test_loaded_modules(void)
 
     pcskind = get_process_kind(pi.hProcess);
 
-    ret = SymRefreshModuleList(pi.hProcess);
-    todo_wine_if(pcskind == PCSKIND_WOW64)
-    ok(ret || broken(GetLastError() == STATUS_PARTIAL_COPY /* Win11 in some cases */), "SymRefreshModuleList failed: %lu\n", GetLastError());
+    ret = wrapper_SymRefreshModuleList(pi.hProcess);
+    ok(ret || broken(GetLastError() == STATUS_PARTIAL_COPY /* Win11 in some cases */ ||
+                             GetLastError() == STATUS_INFO_LENGTH_MISMATCH /* Win11 in some cases */),
+       "SymRefreshModuleList failed: %lx\n", GetLastError());
 
     if (!strcmp(winetest_platform, "wine"))
     {
@@ -945,8 +997,8 @@ static void test_loaded_modules(void)
                    "Wrong directory aggregation count %u %u\n",
                    aggregation.count_systemdir, aggregation.count_wowdir);
             }
-            ret = SymRefreshModuleList(pi.hProcess);
-            ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());
+            ret = wrapper_SymRefreshModuleList(pi.hProcess);
+            ok(ret, "SymRefreshModuleList failed: %lx\n", GetLastError());
 
             if (!strcmp(winetest_platform, "wine"))
             {
@@ -1007,8 +1059,8 @@ static void test_loaded_modules(void)
                 break;
             }
 
-            ret = SymRefreshModuleList(pi.hProcess);
-            ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());
+            ret = wrapper_SymRefreshModuleList(pi.hProcess);
+            ok(ret, "SymRefreshModuleList failed: %lx\n", GetLastError());
 
             if (!strcmp(winetest_platform, "wine"))
             {
@@ -1529,6 +1581,75 @@ static void test_function_tables(void)
     SymCleanup(GetCurrentProcess());
 }
 
+static void test_refresh_modules(void)
+{
+    BOOL ret;
+    unsigned count, count_current;
+    HMODULE hmod;
+    IMAGEHLP_MODULEW64 module_info = { .SizeOfStruct = sizeof(module_info) };
+
+    /* pick a DLL: which isn't already loaded by test, and that will not load other DLLs for deps */
+    static const WCHAR* unused_dll = L"psapi";
+
+    ret = SymInitialize(GetCurrentProcess(), 0, TRUE);
+    ok(ret, "SymInitialize failed: %lu\n", GetLastError());
+
+    count = get_module_count(GetCurrentProcess());
+    ok(count, "Unexpected module count %u\n", count);
+
+    ret = SymCleanup(GetCurrentProcess());
+    ok(ret, "SymCleanup failed: %lu\n", GetLastError());
+
+    ret = SymInitialize(GetCurrentProcess(), 0, FALSE);
+    ok(ret, "SymInitialize failed: %lu\n", GetLastError());
+
+    count_current = get_module_count(GetCurrentProcess());
+    ok(!count_current, "Unexpected module count %u\n", count_current);
+
+    ret = wrapper_SymRefreshModuleList(GetCurrentProcess());
+    ok(ret, "SymRefreshModuleList failed: %lx\n", GetLastError());
+
+    count_current = get_module_count(GetCurrentProcess());
+    ok(count == count_current, "Unexpected module count %u, %u\n", count, count_current);
+
+    hmod = GetModuleHandleW(unused_dll);
+    ok(hmod == NULL, "Expecting DLL %ls not be loaded\n", unused_dll);
+
+    hmod = LoadLibraryW(unused_dll);
+    ok(hmod != NULL, "LoadLibraryW failed: %lu\n", GetLastError());
+
+    count_current = get_module_count(GetCurrentProcess());
+    ok(count == count_current, "Unexpected module count %u, %u\n", count, count_current);
+    ret = is_module_present(GetCurrentProcess(), unused_dll);
+    ok(!ret, "Couldn't find module %ls\n", unused_dll);
+
+    ret = wrapper_SymRefreshModuleList(GetCurrentProcess());
+    ok(ret, "SymRefreshModuleList failed: %lx\n", GetLastError());
+
+    count_current = get_module_count(GetCurrentProcess());
+    ok(count + 1 == count_current, "Unexpected module count %u, %u\n", count, count_current);
+    ret = is_module_present(GetCurrentProcess(), unused_dll);
+    ok(ret, "Couldn't find module %ls\n", unused_dll);
+
+    ret = FreeLibrary(hmod);
+    ok(ret, "LoadLibraryW failed: %lu\n", GetLastError());
+
+    count_current = get_module_count(GetCurrentProcess());
+    ok(count + 1 == count_current, "Unexpected module count %u, %u\n", count, count_current);
+
+    ret = wrapper_SymRefreshModuleList(GetCurrentProcess());
+    ok(ret, "SymRefreshModuleList failed: %lx\n", GetLastError());
+
+    /* SymRefreshModuleList() doesn't remove the unloaded modules... */
+    count_current = get_module_count(GetCurrentProcess());
+    ok(count + 1 == count_current, "Unexpected module count %u != %u\n", count, count_current);
+    ret = is_module_present(GetCurrentProcess(), unused_dll);
+    ok(ret, "Couldn't find module %ls\n", unused_dll);
+
+    ret = SymCleanup(GetCurrentProcess());
+    ok(ret, "SymCleanup failed: %lu\n", GetLastError());
+}
+
 START_TEST(dbghelp)
 {
     BOOL ret;
@@ -1559,6 +1680,7 @@ START_TEST(dbghelp)
         test_modules_overlap();
         test_loaded_modules();
         test_live_modules();
+        test_refresh_modules();
     }
     test_function_tables();
 }