[PATCH v6 0/1] MR364: kernel32/tests: Test module refcounting with forwarded exports.
Test module references created by resolving forwarded exports. The following diagram explains what happens in bug #52094 under the hood: ```mermaid graph TB subgraph state_00["Initial"] direction LR s00_ida[ida64.dll] ~~~ s00_idapy[idapython64.dll] -. "(import)" .-> s00_py3[python3.dll] -. "(forwards to)" .-> s00_py311[python311.dll] end state_00 -. "1. <strong><code><span style='color:#28f'>Load</span>Library("</code></strong>configured Python version (e.g., python311.dll)<strong><code>")</code></strong>" .-> state_01 subgraph state_01["python311.dll loaded"] direction LR s01_ida[ida64.dll] ~~~ s01_idapy[idapython64.dll] -. "(import)" .-> s01_py3[python3.dll] -. "(forwards to)" .-> s01_py311[python311.dll] s01_ida -- "<em>dynamic</em> ref" --> s01_py311 end state_01 -. "2. <strong><code><span style='color:#28f'>Load</span>Library("</code></strong>idapython64.dll<strong><code>")</code></strong>" .-> state_02 subgraph state_02["idapython64.dll loaded"] direction LR s02_ida[ida64.dll] -- "<em>dynamic</em> ref" --> s02_idapy[idapython64.dll] -- "<em><strong>static</strong></em> ref" --> s02_py3[python3.dll] -. "(forwards to)" .-> s02_py311[python311.dll] s02_ida -- "<em>dynamic</em> ref" --> s02_py311 s02_idapy -- "<span style='color:red'><em><strong>static forwarded</strong></em> ref</span>" --> s02_py311 end state_02 -. "3. <strong><code><span style='color:#d70'>Free</span>Library("</code></strong>configured Python version (e.g., python311.dll)<strong><code>")</code></strong>" .-> state_03 subgraph state_03["python311.dll reference removed"] direction LR style s03_py311 color:#f00,stroke:#f00 s03_ida[ida64.dll] -- "<em>dynamic</em> ref" --> s03_idapy[idapython64.dll] -- "<em><strong>static</strong></em> ref" --> s03_py3[python3.dll] -. "(forwards to)" .-> s03_py311[python311.dll] s03_ida -. "<del><em>dynamic</em> ref (deleted)</del>" .-> s03_py311 s03_idapy -- "<span style='color:red'><em><strong>static forwarded</strong></em> ref</span>" --> s03_py311 end ``` On Windows, the <em><strong>static forwarded</strong></em> ref keeps `python311.dll` from being unloaded; however, Wine is missing this behavior, causing it to be unloaded and segfault immediately (since further initialization happens just after step 3 FreeLibrary). The tests aim to reproduce this condition. --- ### Alternative options 1. Find existing DLLs satisfying this scenario: I haven't found any in wine tree. Let me know if I missed anything. 2. Construct PE for the DLLs on-the-fly: This sounds more complicated than a handful of test DLLs. -- v6: kernel32/tests: Test module refcounting with forwarded exports. https://gitlab.winehq.org/wine/wine/-/merge_requests/364
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/icmp/Makefile.in | 1 + dlls/kernel32/tests/Makefile.in | 12 +- dlls/kernel32/tests/forward1.c | 37 ++++++ dlls/kernel32/tests/forward1.spec | 2 + dlls/kernel32/tests/forward2.c | 23 ++++ dlls/kernel32/tests/forward2.spec | 2 + dlls/kernel32/tests/forward3.c | 23 ++++ dlls/kernel32/tests/forward3.spec | 2 + dlls/kernel32/tests/loader.c | 184 ++++++++++++++++++++++++++++++ dlls/kernel32/tests/sforward.c | 36 ++++++ dlls/kernel32/tests/sforward.spec | 1 + 11 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 dlls/kernel32/tests/forward1.c create mode 100644 dlls/kernel32/tests/forward1.spec create mode 100644 dlls/kernel32/tests/forward2.c create mode 100644 dlls/kernel32/tests/forward2.spec create mode 100644 dlls/kernel32/tests/forward3.c create mode 100644 dlls/kernel32/tests/forward3.spec create mode 100644 dlls/kernel32/tests/sforward.c create mode 100644 dlls/kernel32/tests/sforward.spec diff --git a/dlls/icmp/Makefile.in b/dlls/icmp/Makefile.in index 42d30b63d8d..1eb9f8a4662 100644 --- a/dlls/icmp/Makefile.in +++ b/dlls/icmp/Makefile.in @@ -1,4 +1,5 @@ MODULE = icmp.dll +IMPORTLIB = icmp EXTRADLLFLAGS = -Wb,--data-only diff --git a/dlls/kernel32/tests/Makefile.in b/dlls/kernel32/tests/Makefile.in index e9516603ce9..bd5c5d135be 100644 --- a/dlls/kernel32/tests/Makefile.in +++ b/dlls/kernel32/tests/Makefile.in @@ -1,6 +1,8 @@ TESTDLL = kernel32.dll IMPORTS = user32 advapi32 +sforward_IMPORTS = icmp + SOURCES = \ actctx.c \ atom.c \ @@ -37,4 +39,12 @@ SOURCES = \ toolhelp.c \ version.c \ virtual.c \ - volume.c + volume.c \ + forward1.c \ + forward1.spec \ + forward2.c \ + forward2.spec \ + forward3.c \ + forward3.spec \ + sforward.c \ + sforward.spec diff --git a/dlls/kernel32/tests/forward1.c b/dlls/kernel32/tests/forward1.c new file mode 100644 index 00000000000..22651c96b47 --- /dev/null +++ b/dlls/kernel32/tests/forward1.c @@ -0,0 +1,37 @@ +/* + * Dummy forwarder test DLL (source) + * + * Copyright 2024 Jinoh Kang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep testdll +#endif + +#include <stdarg.h> +#include "windef.h" +#include "winbase.h" + +unsigned long forward_test_func(void) +{ + return 0x00005678UL; +} + +unsigned long forward_test_func2(void) +{ + return 0x12340000UL; +} diff --git a/dlls/kernel32/tests/forward1.spec b/dlls/kernel32/tests/forward1.spec new file mode 100644 index 00000000000..bf19fa7e011 --- /dev/null +++ b/dlls/kernel32/tests/forward1.spec @@ -0,0 +1,2 @@ +1 cdecl forward_test_func() +2 cdecl -noname forward_test_func2() diff --git a/dlls/kernel32/tests/forward2.c b/dlls/kernel32/tests/forward2.c new file mode 100644 index 00000000000..c08d218f82a --- /dev/null +++ b/dlls/kernel32/tests/forward2.c @@ -0,0 +1,23 @@ +/* + * Dummy forwarder test DLL (forward) + * + * Copyright 2024 Jinoh Kang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep testdll +#endif diff --git a/dlls/kernel32/tests/forward2.spec b/dlls/kernel32/tests/forward2.spec new file mode 100644 index 00000000000..374156d8d06 --- /dev/null +++ b/dlls/kernel32/tests/forward2.spec @@ -0,0 +1,2 @@ +1 cdecl forward_test_func() forward1.forward_test_func +2 cdecl -noname forward_test_func2() forward1.#2 diff --git a/dlls/kernel32/tests/forward3.c b/dlls/kernel32/tests/forward3.c new file mode 100644 index 00000000000..c08d218f82a --- /dev/null +++ b/dlls/kernel32/tests/forward3.c @@ -0,0 +1,23 @@ +/* + * Dummy forwarder test DLL (forward) + * + * Copyright 2024 Jinoh Kang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep testdll +#endif diff --git a/dlls/kernel32/tests/forward3.spec b/dlls/kernel32/tests/forward3.spec new file mode 100644 index 00000000000..31d019aa071 --- /dev/null +++ b/dlls/kernel32/tests/forward3.spec @@ -0,0 +1,2 @@ +1 cdecl forward_test_func() forward2.forward_test_func +2 cdecl -noname forward_test_func2() forward2.#2 diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 8f418ef09a9..edf78f0c08f 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -1613,6 +1613,187 @@ static void test_ImportDescriptors(void) } } +static void extract_resource(const char *name, const char *type, const char *path) +{ + HMODULE module; + DWORD written; + HANDLE file; + HRSRC res; + void *ptr; + + file = CreateFileA(path, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); + ok( file != INVALID_HANDLE_VALUE, "file creation failed, at %s, error %lu\n", path, GetLastError() ); + + module = GetModuleHandleA( NULL ); + res = FindResourceA( module, name, type ); + ok( res != 0, "couldn't find resource\n" ); + ptr = LockResource( LoadResource( module, res )); + WriteFile( file, ptr, SizeofResource( module, res ), &written, NULL ); + ok( written == SizeofResource( module, res ), "couldn't write resource\n" ); + CloseHandle( file ); +} + +static void test_static_forwarded_import_refs(void) +{ + CHAR temp_path[MAX_PATH], dir_path[MAX_PATH], sforward_path[MAX_PATH]; + HMODULE iphlpapi, icmp, sforward; + FARPROC test_func_stub; + + if (GetModuleHandleA( "iphlpapi.dll" ) /* win1064v1809 32bit */) + { + win_skip( "iphlpapi.dll already loaded, skipping\n" ); + return; + } + + ok( !GetModuleHandleA( "icmp.dll" ), "icmp.dll should not have already been loaded\n" ); + + GetTempPathA( ARRAY_SIZE(temp_path), temp_path ); + GetTempFileNameA( temp_path, "ldr", GetTickCount() | 1UL, dir_path ); + ok( CreateDirectoryA( dir_path, NULL ), "failed to create dir %s, error %lu\n", + dir_path, GetLastError() ); + + snprintf( sforward_path, MAX_PATH, "%s\\sforward.dll", dir_path ); + extract_resource( "sforward.dll", "TESTDLL", sforward_path ); + + iphlpapi = LoadLibraryA( "iphlpapi.dll" ); + ok( !!iphlpapi, "couldn't find iphlpapi.dll: %lu\n", GetLastError() ); + icmp = LoadLibraryA( "icmp.dll" ); + ok( !!icmp, "couldn't find icmp.dll: %lu\n", GetLastError() ); + sforward = LoadLibraryA( sforward_path ); + ok( !!sforward, "couldn't find %s: %lu\n", sforward_path, GetLastError() ); + + test_func_stub = GetProcAddress( sforward, "test_func_stub" ); + ok( !!test_func_stub, "sforward!test_func_stub not found\n" ); + + /* When a DLL imports a forwarded export, the loader introduces a module + * dependency from the importer DLL to the target DLL of the forwarded + * export. This keeps iphlpapi and icmp from being unloaded. + */ + FreeLibrary( iphlpapi ); + FreeLibrary( icmp ); + todo_wine + ok( !!GetModuleHandleA( "iphlpapi.dll" ), "iphlpapi.dll unexpectedly unloaded\n" ); + ok( !!GetModuleHandleA( "icmp.dll" ), "icmp.dll unexpectedly unloaded\n" ); + + FreeLibrary( sforward ); + ok( !GetModuleHandleA( "iphlpapi.dll" ), "iphlpapi.dll unexpectedly kept open\n" ); + ok( !GetModuleHandleA( "icmp.dll" ), "icmp.dll unexpectedly kept open\n" ); + ok( !GetModuleHandleA( "sforward.dll" ), "sforward.dll unexpectedly kept open\n" ); + + DeleteFileA( sforward_path ); + RemoveDirectoryA( dir_path ); +} + +static void test_dynamic_forwarded_import_refs(void) +{ + CHAR temp_path[MAX_PATH], dir_path[MAX_PATH]; + CHAR forward1_path[MAX_PATH]; + CHAR forward2_path[MAX_PATH]; + CHAR forward3_path[MAX_PATH]; + HMODULE forward1, forward2, forward3; + FARPROC proc1, proc2, proc3, oproc1, oproc2, oproc3; + + GetTempPathA( ARRAY_SIZE(temp_path), temp_path ); + GetTempFileNameA( temp_path, "ldr", GetTickCount() | 1UL, dir_path ); + ok( CreateDirectoryA( dir_path, NULL ), "failed to create dir %s, error %lu\n", + dir_path, GetLastError() ); + + snprintf( forward1_path, MAX_PATH, "%s\\forward1.dll", dir_path ); + snprintf( forward2_path, MAX_PATH, "%s\\forward2.dll", dir_path ); + snprintf( forward3_path, MAX_PATH, "%s\\forward3.dll", dir_path ); + extract_resource( "forward1.dll", "TESTDLL", forward1_path ); + extract_resource( "forward2.dll", "TESTDLL", forward2_path ); + extract_resource( "forward3.dll", "TESTDLL", forward3_path ); + + forward1 = LoadLibraryA( forward1_path ); + ok( !!forward1, "couldn't find %s: %lu\n", forward1_path, GetLastError() ); + forward2 = LoadLibraryA( forward2_path ); + ok( !!forward2, "couldn't find %s: %lu\n", forward2_path, GetLastError() ); + forward3 = LoadLibraryA( forward3_path ); + ok( !!forward3, "couldn't find %s: %lu\n", forward3_path, GetLastError() ); + + proc1 = GetProcAddress(forward1, "forward_test_func"); + ok( !!proc1, "cannot resolve forward1!forward_test_func\n"); + proc2 = GetProcAddress(forward2, "forward_test_func"); + ok( !!proc2, "cannot resolve forward2!forward_test_func\n"); + proc3 = GetProcAddress(forward3, "forward_test_func"); + ok( !!proc3, "cannot resolve forward3!forward_test_func\n"); + ok( proc1 == proc3, "forward1!forward_test_func is not equal to forward3!forward_test_func\n"); + ok( proc2 == proc3, "forward2!forward_test_func is not equal to forward3!forward_test_func\n"); + + oproc1 = GetProcAddress(forward1, (LPSTR)2); + ok( !!oproc1, "cannot resolve forward1!#2 (forward_test_func2)\n"); + oproc2 = GetProcAddress(forward2, (LPSTR)2); + ok( !!oproc2, "cannot resolve forward2!#2 (forward_test_func2)\n"); + oproc3 = GetProcAddress(forward3, (LPSTR)2); + ok( !!oproc3, "cannot resolve forward3!#2 (forward_test_func2)\n"); + ok( oproc1 == oproc3, "forward1!forward_test_func2 is not equal to forward3!forward_test_func2\n"); + ok( oproc2 == oproc3, "forward2!forward_test_func2 is not equal to forward3!forward_test_func2\n"); + + /* GetProcAddress, when called on a forwarded export, has a side effect of + * introducing a module dependency from the source forwarder DLL to the + * target DLL. This keeps forward1 and forward2 from being unloaded. + */ + FreeLibrary( forward1 ); + FreeLibrary( forward2 ); + todo_wine + ok( !!GetModuleHandleA( "forward1.dll" ), "forward1.dll unexpectedly unloaded\n" ); + todo_wine + ok( !!GetModuleHandleA( "forward2.dll" ), "forward2.dll unexpectedly unloaded\n" ); + + FreeLibrary( forward3 ); + ok( !GetModuleHandleA( "forward1.dll" ), "forward1.dll unexpectedly kept open\n" ); + ok( !GetModuleHandleA( "forward2.dll" ), "forward2.dll unexpectedly kept open\n" ); + ok( !GetModuleHandleA( "forward3.dll" ), "forward3.dll unexpectedly kept open\n" ); + + DeleteFileA( forward1_path ); + DeleteFileA( forward2_path ); + DeleteFileA( forward3_path ); + RemoveDirectoryA( dir_path ); +} + +static void test_dynamic_forward_export_norefs(void) +{ + CHAR temp_path[MAX_PATH], dir_path[MAX_PATH]; + CHAR forward1_path[MAX_PATH]; + CHAR forward2_path[MAX_PATH]; + CHAR forward3_path[MAX_PATH]; + HMODULE forward1, forward2, forward3; + + GetTempPathA( ARRAY_SIZE(temp_path), temp_path ); + GetTempFileNameA( temp_path, "ldr", GetTickCount() | 1UL, dir_path ); + ok( CreateDirectoryA( dir_path, NULL ), "failed to create dir %s, error %lu\n", + dir_path, GetLastError() ); + + snprintf( forward1_path, MAX_PATH, "%s\\forward1.dll", dir_path ); + snprintf( forward2_path, MAX_PATH, "%s\\forward2.dll", dir_path ); + snprintf( forward3_path, MAX_PATH, "%s\\forward3.dll", dir_path ); + extract_resource( "forward1.dll", "TESTDLL", forward1_path ); + extract_resource( "forward2.dll", "TESTDLL", forward2_path ); + extract_resource( "forward3.dll", "TESTDLL", forward3_path ); + + forward1 = LoadLibraryA( forward1_path ); + ok( !!forward1, "couldn't find %s: %lu\n", forward1_path, GetLastError() ); + forward2 = LoadLibraryA( forward2_path ); + ok( !!forward2, "couldn't find %s: %lu\n", forward2_path, GetLastError() ); + forward3 = LoadLibraryA( forward3_path ); + ok( !!forward3, "couldn't find %s: %lu\n", forward3_path, GetLastError() ); + + /* The mere existence of a forwarded export should not count as a reference by itself. */ + FreeLibrary( forward1 ); + FreeLibrary( forward3 ); + ok( !GetModuleHandleA( "forward1.dll" ), "forward1.dll unexpectedly kept open\n" ); + ok( !GetModuleHandleA( "forward3.dll" ), "forward3.dll unexpectedly kept open\n" ); + + FreeLibrary( forward2 ); + ok( !GetModuleHandleA( "forward2.dll" ), "forward2.dll unexpectedly kept open\n" ); + + DeleteFileA( forward1_path ); + DeleteFileA( forward2_path ); + DeleteFileA( forward3_path ); + RemoveDirectoryA( dir_path ); +} + static void test_image_mapping(const char *dll_name, DWORD scn_page_access, BOOL is_dll) { HANDLE hfile, hmap; @@ -4277,9 +4458,12 @@ START_TEST(loader) ok(len && len < ARRAY_SIZE(syswow_dir), "Couldn't get wow directory: %lu\n", GetLastError()); } + test_static_forwarded_import_refs(); /* Must be first; other tests may load iphlpapi.dll */ test_filenames(); test_ResolveDelayLoadedAPI(); test_ImportDescriptors(); + test_dynamic_forwarded_import_refs(); + test_dynamic_forward_export_norefs(); test_section_access(); test_import_resolution(); test_ExitProcess(); diff --git a/dlls/kernel32/tests/sforward.c b/dlls/kernel32/tests/sforward.c new file mode 100644 index 00000000000..c4e7305e467 --- /dev/null +++ b/dlls/kernel32/tests/sforward.c @@ -0,0 +1,36 @@ +/* + * Dummy static forwarder test DLL + * + * Copyright 2024 Jinoh Kang + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep testdll +#endif + +#include <stdarg.h> +#include "windef.h" +#include "winbase.h" +#include "ws2tcpip.h" +#include "iphlpapi.h" +#include "icmpapi.h" + +void test_func_stub(void) +{ + HANDLE file = IcmpCreateFile(); + if (file != INVALID_HANDLE_VALUE) IcmpCloseHandle( file ); +} diff --git a/dlls/kernel32/tests/sforward.spec b/dlls/kernel32/tests/sforward.spec new file mode 100644 index 00000000000..cb6d4add796 --- /dev/null +++ b/dlls/kernel32/tests/sforward.spec @@ -0,0 +1 @@ +@ cdecl test_func_stub() -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/364
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=143265 Your paranoid android. === w10pro64_en_AE_u8 (32 bit report) === kernel32: heap.c:3314: Test failed: HeapValidate failed heap.c:3323: Test failed: HeapValidate failed
On Tue Feb 20 12:14:28 2024 +0000, Zebediah Figura wrote:
You don't need those DllMain(); winecrt0 provides a default that does the same thing. Done in v6.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62033
Rémi Bernon (@rbernon) commented about dlls/kernel32/tests/forward3.spec:
+1 cdecl forward_test_func() forward2.forward_test_func +2 cdecl -noname forward_test_func2() forward2.#2 What about forwarding to icmp here, and removing the need for forward2 (would be icmp) and forward1 (would be iphlpapi)?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62602
Rémi Bernon (@rbernon) commented about dlls/kernel32/tests/Makefile.in:
toolhelp.c \ version.c \ virtual.c \ - volume.c + volume.c \ + forward1.c \ + forward1.spec \ + forward2.c \ + forward2.spec \ + forward3.c \ + forward3.spec \ + sforward.c \ + sforward.spec I think these are supposed to be sorted, you can run tools/make_makefiles to do that for you.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62603
Alexandre Julliard (@julliard) commented about dlls/icmp/Makefile.in:
MODULE = icmp.dll +IMPORTLIB = icmp
There's no such import lib on Windows, it would be better to use some other existing library. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62709
On Mon Feb 26 19:58:49 2024 +0000, Alexandre Julliard wrote:
There's no such import lib on Windows, it would be better to use some other existing library. We need a DLL, not already loaded, that forwards to another DLL, also not already loaded[^1].
Most forwarder DLLs forward to known DLLs (e.g., combase.dll, setupapi.dll), which are already transitively loaded at process startup, in some Windows versions or another. Therefore, *all* of the following DLLs are ruled out (assuming we want to test on Windows 7 as well): ```mermaid graph LR classDef default text-decoration:line-through kernel32_test.exe --> kernel32.dll --> kernelbase.dll --> ntdll.dll kernel32_test.exe --> user32.dll --> gdi32.dll --> win32u.dll --> ntdll.dll user32.dll --> sechost.dll user32.dll --> advapi32.dll user32.dll --> kernelbase.dll user32.dll --> win32u.dll kernel32_test.exe --> advapi32.dll --> kernelbase.dll advapi32.dll --> sechost.dll advapi32.dll --> msvcrt.dll --> ntdll.dll kernel32_test.exe -. "(w7)" .-> ole32.dll kernel32_test.exe -. "(w7)" .-> shlwapi.dll --> userenv.dll kernel32_test.exe -. "(w7)" .-> sspicli.dll --> secur32.dll ``` Others are relatively obscure, or are not guaranteed to forward to specific DLLs[^2] (or contain forwarders at all) or even exist at all[^3] across Windows versions. Some others (e.g., ncrypt.dll) are seemingly pinned by some unrelated mechanism once loaded by some other forwarder DLL. There might be something I haven't discovered yet and I can continue scavenging applicable DLLs, but then I wonder if I would end up doing a whack-a-mole here and writing tests that aren't guaranteed to be run on future Windows updates anyway. Alternatively, shall I just skip it on Windows 7? [^1]: We can't test refcounts dropping to zero if someone else is holding the DLL. [^2]: E.g., ole32.dll [^3]: E.g., olepro32.dll, olecli32.dll, combase.dll -- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62834
On Tue Feb 27 18:21:26 2024 +0000, Jinoh Kang wrote:
We need a DLL, not already loaded, that forwards to another DLL, also not already loaded[^1]. Most forwarder DLLs forward to known DLLs (e.g., combase.dll, setupapi.dll), which are already transitively loaded at process startup, in some Windows versions or another. Therefore, *all* of the following DLLs are ruled out (assuming we want to test on Windows 7 as well): ```mermaid graph LR classDef default text-decoration:line-through kernel32_test.exe --> kernel32.dll --> kernelbase.dll --> ntdll.dll kernel32_test.exe --> user32.dll --> gdi32.dll --> win32u.dll --> ntdll.dll user32.dll --> sechost.dll user32.dll --> advapi32.dll user32.dll --> kernelbase.dll user32.dll --> win32u.dll kernel32_test.exe --> advapi32.dll --> kernelbase.dll advapi32.dll --> sechost.dll advapi32.dll --> msvcrt.dll --> ntdll.dll kernel32_test.exe -. "(w7)" .-> ole32.dll kernel32_test.exe .-> shlwapi.dll --> userenv.dll kernel32_test.exe -. "(w7)" .-> sspicli.dll --> secur32.dll ``` Others are relatively obscure, or are not guaranteed to forward to specific DLLs[^2] (or contain forwarders at all) or even exist at all[^3] across Windows versions. Some others (e.g., ncrypt.dll) are seemingly pinned by some unrelated mechanism once loaded by some other forwarder DLL. There might be something I haven't discovered yet and I can continue scavenging applicable DLLs, but then I wonder if I would end up doing a whack-a-mole here and writing tests that aren't guaranteed to be run on future Windows updates anyway. Alternatively, shall I just skip it on Windows 7? [^1]: We can't test refcounts dropping to zero if someone else is holding the DLL. [^2]: E.g., ole32.dll [^3]: E.g., olepro32.dll, olecli32.dll, combase.dll Turns out win7 is not the only problem here; for example, shlwapi.dll is already loaded on w10 or later.
How about adding importlib generation support for makedep? Would that be an overkill? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_62843
How about adding importlib generation support for makedep? Would that be an overkill?
(Quoting comment on IRC)
this may be a case where a standalone test is better than trying to shoehorn it into the wine test framework
Thanks! My gut feeling was that refcounting machinery is fragile and easily regress-able in nature. If you think this is not worth staying in the tree, feel free to close this MR at any time. In the meantime, I'll try to switch to runtime PE generation (which doesn't touch the framework or other parts of wine at all). If it looks good enough, it can land into trunk I guess. If it doesn't, it can always remain out-of-tree. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/364#note_63182
participants (5)
-
Alexandre Julliard (＠julliard) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)
-
Marvin -
Rémi Bernon