[PATCH 0/2] MR6591: ntdll: Avoid searching DLL file for LdrGetModuleHandleEx() called with short dll name.
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/module.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c index 4b4269c821b..88c3afc16c9 100644 --- a/dlls/kernel32/tests/module.c +++ b/dlls/kernel32/tests/module.c @@ -1317,8 +1317,10 @@ static void test_LdrGetDllHandleEx(void) { HMODULE mod, loaded_mod; UNICODE_STRING name; + char path[MAX_PATH]; NTSTATUS status; unsigned int i; + BOOL bret; if (!pLdrGetDllHandleEx) { @@ -1392,6 +1394,39 @@ static void test_LdrGetDllHandleEx(void) winetest_push_context( "LDR_GET_DLL_HANDLE_EX_FLAG_PIN" ); check_refcount( loaded_mod, ~0u ); winetest_pop_context(); + + GetCurrentDirectoryA( ARRAY_SIZE(path), path ); + create_test_dll( "d01.dll" ); + mod = LoadLibraryA( "d01.dll" ); + ok( !!mod, "got error %lu.\n", GetLastError() ); + RtlInitUnicodeString( &name, L"d01.dll" ); + status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); + ok( !status, "got %#lx.\n", status ); + + RtlInitUnicodeString( &name, L"d02.dll" ); + status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); + ok( status == STATUS_DLL_NOT_FOUND, "got %#lx.\n", status ); + + /* Same (moved) file, different name: not found in loaded modules with short name but found with path. */ + DeleteFileA( "d02.dll" ); + bret = MoveFileA( "d01.dll", "d02.dll" ); + ok( bret, "got error %lu.\n", GetLastError() ); + status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); + todo_wine ok( status == STATUS_DLL_NOT_FOUND, "got %#lx.\n", status ); + CreateDirectoryA( "testdir", NULL ); + DeleteFileA( "testdir\\d02.dll" ); + bret = MoveFileA( "d02.dll", "testdir\\d02.dll" ); + ok( bret, "got error %lu.\n", GetLastError() ); + RtlInitUnicodeString( &name, L"testdir\\d02.dll" ); + status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); + ok( !status, "got %#lx.\n", status ); + ok( loaded_mod == mod, "got %p, %p.\n", loaded_mod, mod ); + FreeLibrary( mod ); + status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); + ok( status == STATUS_DLL_NOT_FOUND, "got %#lx.\n", status ); + + DeleteFileA( "testdir\\d02.dll" ); + RemoveDirectoryA( "testdir" ); } static void test_LdrGetDllFullName(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6591
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/module.c | 2 +- dlls/ntdll/loader.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c index 88c3afc16c9..df48605c376 100644 --- a/dlls/kernel32/tests/module.c +++ b/dlls/kernel32/tests/module.c @@ -1412,7 +1412,7 @@ static void test_LdrGetDllHandleEx(void) bret = MoveFileA( "d01.dll", "d02.dll" ); ok( bret, "got error %lu.\n", GetLastError() ); status = pLdrGetDllHandleEx( LDR_GET_DLL_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, NULL, NULL, &name, &loaded_mod ); - todo_wine ok( status == STATUS_DLL_NOT_FOUND, "got %#lx.\n", status ); + ok( status == STATUS_DLL_NOT_FOUND, "got %#lx.\n", status ); CreateDirectoryA( "testdir", NULL ); DeleteFileA( "testdir\\d02.dll" ); bret = MoveFileA( "d02.dll", "testdir\\d02.dll" ); diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 0ed37ad390c..2982b1e049b 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -3233,7 +3233,7 @@ done: */ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, UNICODE_STRING *nt_name, WINE_MODREF **pwm, HANDLE *mapping, SECTION_IMAGE_INFORMATION *image_info, - struct file_id *id ) + struct file_id *id, BOOL find_loaded ) { WCHAR *fullname = NULL; NTSTATUS status; @@ -3266,6 +3266,12 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, UNI status = STATUS_SUCCESS; goto done; } + if (find_loaded) + { + TRACE( "Skipping file search for %s.\n", debugstr_w(libname) ); + status = STATUS_DLL_NOT_FOUND; + goto done; + } } } @@ -3309,7 +3315,7 @@ static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, DWORD fl if (nts) { - nts = find_dll_file( load_path, libname, &nt_name, pwm, &mapping, &image_info, &id ); + nts = find_dll_file( load_path, libname, &nt_name, pwm, &mapping, &image_info, &id, FALSE ); system = FALSE; } @@ -3499,7 +3505,7 @@ NTSTATUS WINAPI LdrGetDllHandleEx( ULONG flags, LPCWSTR load_path, ULONG *dll_ch RtlEnterCriticalSection( &loader_section ); status = find_dll_file( load_path, dllname ? dllname : name->Buffer, - &nt_name, &wm, &mapping, &image_info, &id ); + &nt_name, &wm, &mapping, &image_info, &id, TRUE ); if (wm) *base = wm->ldr.DllBase; else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6591
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=148783 Your paranoid android. === debian11b (64 bit WoW report) === d3d9: d3d9ex.c:3230: Test failed: Expected message 0x18 for window 0, but didn't receive it, i=0.
The primary motivation for this is optimization. E. g., Final Fantasy XVI calls GetModuleHandle() very often for the libraries (with short name) which are absent (and not supposed to be present) and that severely affects performance. The functional difference which the patch introduces relates to the case when there is a differently named file which is actually the same as one of the loaded module. The included test shows however that for specific case of using short dll name for GetModuleHandle() the module is not found on Windows in this case (i. e., the patch does a functionally correct change). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6591#note_83732
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)