[PATCH v2 0/2] MR3129: dbghelp/test: Add explicit tests about loading ELF modules.

[eric pouech (＠epo)]

Perhaps next time ELF loader is modified, it'll be noticed if it breaks
things.

See:
- https://bugs.winehq.org/show_bug.cgi?id=55058
- https://bugs.winehq.org/show_bug.cgi?id=55096

Signed-off-by: Eric Pouech

--
  v2: dbghelp: Fix reading Wine loader's base address.
      dbghelp/test: Add explicit tests about loading ELF modules.

https://gitlab.winehq.org/wine/wine/-/merge_requests/3129

[PATCH v2 1/2] dbghelp/test: Add explicit tests about loading ELF modules.

[Eric Pouech]

From: Eric Pouech

Perhaps next time ELF loader is modified, it'll be noticed if it breaks
things.

See:
- https://bugs.winehq.org/show_bug.cgi?id=55058
- https://bugs.winehq.org/show_bug.cgi?id=55096

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/tests/dbghelp.c | 53 ++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c
index 6e3edc7c952..1bd1122c642 100644
--- a/dlls/dbghelp/tests/dbghelp.c
+++ b/dlls/dbghelp/tests/dbghelp.c
@@ -300,6 +300,12 @@ static BOOL CALLBACK count_module_cb(const char* name, DWORD64 base, void* usr)
     return TRUE;
 }

+static BOOL CALLBACK count_native_module_cb(const char* name, DWORD64 base, void* usr)
+{
+    if (strstr(name, ".so") || strchr(name, '<')) (*(unsigned*)usr)++;
+    return TRUE;
+}
+
 static unsigned get_module_count(HANDLE proc)
 {
     unsigned count = 0;
@@ -310,6 +316,19 @@ static unsigned get_module_count(HANDLE proc)
     return count;
 }

+static unsigned get_native_module_count(HANDLE proc)
+{
+    unsigned count = 0;
+    BOOL old, ret;
+
+    old = SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, TRUE);
+    ret = SymEnumerateModules64(proc, count_native_module_cb, &count);
+    ok(ret, "SymEnumerateModules64 failed: %lu\n", GetLastError());
+    SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, old);
+
+    return count;
+}
+
 struct nth_module
 {
     HANDLE              proc;
@@ -681,6 +700,7 @@ static void test_loaded_modules(void)
     PROCESS_INFORMATION pi = {0};
     STARTUPINFOA si = {0};
     struct loaded_module_aggregation aggregation = {0};
+    enum process_kind pcskind;

ret = GetSystemDirectoryA(buffer, sizeof(buffer));
     ok(ret, "got error %lu\n", GetLastError());
@@ -701,6 +721,7 @@ static void test_loaded_modules(void)
     ret = wrapper_EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation);
     ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());

+    pcskind = get_process_kind(pi.hProcess);
     if (is_win64)
     {
         ok(!aggregation.count_32bit && aggregation.count_64bit, "Wrong bitness aggregation count %u %u\n",
@@ -720,7 +741,7 @@ static void test_loaded_modules(void)
            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);
-        switch (get_process_kind(pi.hProcess))
+        switch (pcskind)
         {
         case PCSKIND_ERROR:
             ok(0, "Unknown process kind\n");
@@ -743,8 +764,16 @@ static void test_loaded_modules(void)
     }

ret = SymRefreshModuleList(pi.hProcess);
+    todo_wine_if(pcskind != PCSKIND_32BIT && pcskind != PCSKIND_WINE_OLD_WOW64)
     ok(ret || broken(GetLastError() == STATUS_PARTIAL_COPY /* Win11 in some cases */), "SymRefreshModuleList failed: %lu\n", GetLastError());

+    if (!strcmp(winetest_platform, "wine"))
+    {
+        unsigned count = get_native_module_count(pi.hProcess);
+        todo_wine_if(pcskind != PCSKIND_32BIT && pcskind != PCSKIND_WINE_OLD_WOW64)
+        ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
+    }
+
     SymCleanup(pi.hProcess);
     TerminateProcess(pi.hProcess, 0);

@@ -770,7 +799,8 @@ static void test_loaded_modules(void)
             ret = wrapper_EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation);
             ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());

-            switch (get_process_kind(pi.hProcess))
+            pcskind = get_process_kind(pi.hProcess);
+            switch (pcskind)
             {
             default:
                 ok(0, "Unknown process kind\n");
@@ -793,9 +823,17 @@ static void test_loaded_modules(void)
                    "Wrong directory aggregation count %u %u\n",
                    aggregation.count_systemdir, aggregation.count_wowdir);
             }
+            todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
             ret = SymRefreshModuleList(pi.hProcess);
             ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());

+            if (!strcmp(winetest_platform, "wine"))
+            {
+                unsigned count = get_native_module_count(pi.hProcess);
+                todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
+                ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
+            }
+
             SymCleanup(pi.hProcess);
             TerminateProcess(pi.hProcess, 0);
         }
@@ -824,7 +862,8 @@ static void test_loaded_modules(void)
             ret = wrapper_EnumerateLoadedModulesW64(pi.hProcess, aggregate_cb, &aggregation2);
             ok(ret, "EnumerateLoadedModulesW64 failed: %lu\n", GetLastError());

-            switch (get_process_kind(pi.hProcess))
+            pcskind = get_process_kind(pi.hProcess);
+            switch (pcskind)
             {
             case PCSKIND_ERROR:
                 break;
@@ -848,9 +887,17 @@ static void test_loaded_modules(void)
                 break;
             }

+            todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
             ret = SymRefreshModuleList(pi.hProcess);
             ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());

+            if (!strcmp(winetest_platform, "wine"))
+            {
+                unsigned count = get_native_module_count(pi.hProcess);
+                todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
+                ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
+            }
+
             SymCleanup(pi.hProcess);
             TerminateProcess(pi.hProcess, 0);
         }

[PATCH v2 2/2] dbghelp: Fix reading Wine loader's base address.

[Eric Pouech]

From: Eric Pouech

Note: the 32bit tests in new wow mode are still marked
todo as the ELF debug header is located above the 4G limit
and "default" module management in dbghelp compiled in 32bit
is (mostly) done with 32 bit addresses.
The correct fix should be to always consider ELF (and macho-O)
modules with 64bit addresses (which means likely to move all
modules handling to 64bit even in 32bit compilations).
But that goes far beyond this patch.

This limitation will only impact debuggers compiled in 32bit,
using 32bit builtin dbghelp on a 32bit debuggee.

https://bugs.winehq.org/show_bug.cgi?id=55058
https://bugs.winehq.org/show_bug.cgi?id=55096

Signed-off-by: Eric Pouech 
---
 dlls/dbghelp/dbghelp.c       |  2 +-
 dlls/dbghelp/tests/dbghelp.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/dlls/dbghelp/dbghelp.c b/dlls/dbghelp/dbghelp.c
index d3003fc44d2..3ce3ef6f707 100644
--- a/dlls/dbghelp/dbghelp.c
+++ b/dlls/dbghelp/dbghelp.c
@@ -387,7 +387,7 @@ static BOOL check_live_target(struct process* pcs, BOOL wow64, BOOL child_wow64)

if (!pcs->is_64bit) peb_addr -= 0x1000; /* PEB32 => PEB64 */
         if (!ReadProcessMemory(pcs->handle, peb_addr, &peb, sizeof(peb), NULL)) return FALSE;
-        base = peb.CloudFileFlags;
+        base = *(const DWORD64*)&peb.CloudFileFlags;
         pcs->is_system_64bit = TRUE;
         if (pcs->is_64bit)
             ReadProcessMemory(pcs->handle,
diff --git a/dlls/dbghelp/tests/dbghelp.c b/dlls/dbghelp/tests/dbghelp.c
index 1bd1122c642..575b9f76d20 100644
--- a/dlls/dbghelp/tests/dbghelp.c
+++ b/dlls/dbghelp/tests/dbghelp.c
@@ -763,14 +763,16 @@ static void test_loaded_modules(void)
         }
     }

+    pcskind = get_process_kind(pi.hProcess);
+
     ret = SymRefreshModuleList(pi.hProcess);
-    todo_wine_if(pcskind != PCSKIND_32BIT && pcskind != PCSKIND_WINE_OLD_WOW64)
+    todo_wine_if(pcskind == PCSKIND_WOW64)
     ok(ret || broken(GetLastError() == STATUS_PARTIAL_COPY /* Win11 in some cases */), "SymRefreshModuleList failed: %lu\n", GetLastError());

if (!strcmp(winetest_platform, "wine"))
     {
         unsigned count = get_native_module_count(pi.hProcess);
-        todo_wine_if(pcskind != PCSKIND_32BIT && pcskind != PCSKIND_WINE_OLD_WOW64)
+        todo_wine_if(pcskind == PCSKIND_WOW64)
         ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
     }

@@ -823,14 +825,12 @@ static void test_loaded_modules(void)
                    "Wrong directory aggregation count %u %u\n",
                    aggregation.count_systemdir, aggregation.count_wowdir);
             }
-            todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
             ret = SymRefreshModuleList(pi.hProcess);
             ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());

if (!strcmp(winetest_platform, "wine"))
             {
                 unsigned count = get_native_module_count(pi.hProcess);
-                todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
                 ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
             }

@@ -887,14 +887,12 @@ static void test_loaded_modules(void)
                 break;
             }

-            todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
             ret = SymRefreshModuleList(pi.hProcess);
             ok(ret, "SymRefreshModuleList failed: %lu\n", GetLastError());

if (!strcmp(winetest_platform, "wine"))
             {
                 unsigned count = get_native_module_count(pi.hProcess);
-                todo_wine_if(pcskind != PCSKIND_WINE_OLD_WOW64)
                 ok(count > 0, "Didn't find any native (ELF/Macho) modules\n");
             }

[eric pouech (＠epo)]

pushed V2:
- added todo on failing tests for SymRefreshList()
- added fix (except for new wow for 32bit debugger, where it'll require quite more work)