-- v3: ntdll: Use HashLinks when searching for a dll using the basename. kernel32/tests: Add tests for module hash links. kernel32/tests: Factor out is_old_loader_struct().
From: Michael Müller michael@fds-team.de
Changed by Paul Gofman in Wine-Staging patch: - remove older hash version support and use RtlHashUnicodeString(); - split off the test in a separate patch; - remove unrelated InInitializationOrderLinks nullification; - remove a comment. --- dlls/ntdll/loader.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 2f2a7fe5427..9e2a32f69d3 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -128,6 +128,9 @@ struct file_id BYTE ObjectId[16]; };
+#define HASH_MAP_SIZE 32 +static LIST_ENTRY hash_table[HASH_MAP_SIZE]; + /* internal representation of loaded modules */ typedef struct _wine_modref { @@ -600,6 +603,15 @@ static int base_address_compare( const void *key, const RTL_BALANCED_NODE *entry return 0; }
+/* compute basename hash */ +static ULONG hash_basename( const UNICODE_STRING *basename ) +{ + ULONG hash = 0; + + RtlHashUnicodeString( basename, TRUE, HASH_STRING_ALGORITHM_DEFAULT, &hash ); + return hash & (HASH_MAP_SIZE - 1); +} + /************************************************************************* * get_modref * @@ -1596,6 +1608,7 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name &wm->ldr.InLoadOrderLinks); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList, &wm->ldr.InMemoryOrderLinks); + InsertTailList(&hash_table[hash_basename( &wm->ldr.BaseDllName )], &wm->ldr.HashLinks); if (rtl_rb_tree_put( &base_address_index_tree, wm->ldr.DllBase, &wm->ldr.BaseAddressIndexNode, base_address_compare )) ERR( "rtl_rb_tree_put failed.\n" ); /* wait until init is called for inserting into InInitializationOrderModuleList */ @@ -2296,6 +2309,7 @@ static NTSTATUS build_module( LPCWSTR load_path, const UNICODE_STRING *nt_name, /* the module has only be inserted in the load & memory order lists */ RemoveEntryList(&wm->ldr.InLoadOrderLinks); RemoveEntryList(&wm->ldr.InMemoryOrderLinks); + RemoveEntryList(&wm->ldr.HashLinks); RtlRbRemoveNode( &base_address_index_tree, &wm->ldr.BaseAddressIndexNode );
/* FIXME: there are several more dangling references @@ -3966,6 +3980,7 @@ static void free_modref( WINE_MODREF *wm )
RemoveEntryList(&wm->ldr.InLoadOrderLinks); RemoveEntryList(&wm->ldr.InMemoryOrderLinks); + RemoveEntryList(&wm->ldr.HashLinks); RtlRbRemoveNode( &base_address_index_tree, &wm->ldr.BaseAddressIndexNode ); if (wm->ldr.InInitializationOrderLinks.Flink) RemoveEntryList(&wm->ldr.InInitializationOrderLinks); @@ -4387,6 +4402,7 @@ void loader_init( CONTEXT *context, void **entry ) ANSI_STRING ctrl_routine = RTL_CONSTANT_STRING( "CtrlRoutine" ); WINE_MODREF *kernel32; PEB *peb = NtCurrentTeb()->Peb; + unsigned int i;
peb->LdrData = &ldr; peb->FastPebLock = &peb_lock; @@ -4405,6 +4421,9 @@ void loader_init( CONTEXT *context, void **entry ) if (!(tls_dirs = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, tls_module_count * sizeof(*tls_dirs) ))) NtTerminateProcess( GetCurrentProcess(), STATUS_NO_MEMORY );
+ for (i = 0; i < HASH_MAP_SIZE; i++) + InitializeListHead( &hash_table[i] ); + init_user_process_params(); load_global_options(); version_init();
From: Paul Gofman pgofman@codeweavers.com
--- dlls/kernel32/tests/module.c | 50 +++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c index 8f3ed5872e4..72c2ce97127 100644 --- a/dlls/kernel32/tests/module.c +++ b/dlls/kernel32/tests/module.c @@ -141,6 +141,34 @@ static void create_test_dll( const char *name ) CloseHandle( handle ); }
+static BOOL is_old_loader_struct(void) +{ + LDR_DATA_TABLE_ENTRY *mod, *mod2; + LDR_DDAG_NODE *ddag_node; + NTSTATUS status; + HMODULE hexe; + + /* Check for old LDR data strcuture. */ + hexe = GetModuleHandleW( NULL ); + ok( !!hexe, "Got NULL exe handle.\n" ); + status = LdrFindEntryForAddress( hexe, &mod ); + ok( !status, "got %#lx.\n", status ); + if (!(ddag_node = mod->DdagNode)) + { + win_skip( "DdagNode is NULL, skipping tests.\n" ); + return TRUE; + } + ok( !!ddag_node->Modules.Flink, "Got NULL module link.\n" ); + mod2 = CONTAINING_RECORD(ddag_node->Modules.Flink, LDR_DATA_TABLE_ENTRY, NodeModuleLink); + ok( mod2 == mod || broken( (void **)mod2 == (void **)mod - 1 ), "got %p, expected %p.\n", mod2, mod ); + if (mod2 != mod) + { + win_skip( "Old LDR_DATA_TABLE_ENTRY structure, skipping tests.\n" ); + return TRUE; + } + return FALSE; +} + static void testGetModuleFileName(const char* name) { HMODULE hMod; @@ -1750,29 +1778,9 @@ static void test_base_address_index_tree(void) unsigned int tree_count, list_count = 0; LDR_DATA_TABLE_ENTRY *mod, *mod2; RTL_BALANCED_NODE *root, *node; - LDR_DDAG_NODE *ddag_node; - NTSTATUS status; - HMODULE hexe; char *base;
- /* Check for old LDR data strcuture. */ - hexe = GetModuleHandleW( NULL ); - ok( !!hexe, "Got NULL exe handle.\n" ); - status = LdrFindEntryForAddress( hexe, &mod ); - ok( !status, "got %#lx.\n", status ); - if (!(ddag_node = mod->DdagNode)) - { - win_skip( "DdagNode is NULL, skipping tests.\n" ); - return; - } - ok( !!ddag_node->Modules.Flink, "Got NULL module link.\n" ); - mod2 = CONTAINING_RECORD(ddag_node->Modules.Flink, LDR_DATA_TABLE_ENTRY, NodeModuleLink); - ok( mod2 == mod || broken( (void **)mod2 == (void **)mod - 1 ), "got %p, expected %p.\n", mod2, mod ); - if (mod2 != mod) - { - win_skip( "Old LDR_DATA_TABLE_ENTRY structure, skipping tests.\n" ); - return; - } + if (is_old_loader_struct()) return;
mod = CONTAINING_RECORD(first->Flink, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks); ok( mod->BaseAddressIndexNode.ParentValue || mod->BaseAddressIndexNode.Left || mod->BaseAddressIndexNode.Right,
From: Paul Gofman pgofman@codeweavers.com
Based on a patch by Michael Müller michael@fds-team.de. --- dlls/kernel32/tests/module.c | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c index 72c2ce97127..0619ce5f747 100644 --- a/dlls/kernel32/tests/module.c +++ b/dlls/kernel32/tests/module.c @@ -48,6 +48,8 @@ static NTSTATUS (WINAPI *pLdrGetDllFullName)( HMODULE module, UNICODE_STRING *na
static BOOL (WINAPI *pIsApiSetImplemented)(LPCSTR);
+static NTSTATUS (WINAPI *pRtlHashUnicodeString)( const UNICODE_STRING *, BOOLEAN, ULONG, ULONG * ); + static BOOL is_unicode_enabled = TRUE;
static BOOL cmpStrAW(const char* a, const WCHAR* b, DWORD lenA, DWORD lenB) @@ -967,6 +969,7 @@ static void init_pointers(void) MAKEFUNC(LdrGetDllHandle); MAKEFUNC(LdrGetDllHandleEx); MAKEFUNC(LdrGetDllFullName); + MAKEFUNC(RtlHashUnicodeString); mod = GetModuleHandleA( "kernelbase.dll" ); MAKEFUNC(IsApiSetImplemented); #undef MAKEFUNC @@ -1810,6 +1813,45 @@ static void test_base_address_index_tree(void) ok( tree_count == list_count, "count mismatch %u, %u.\n", tree_count, list_count ); }
+static ULONG hash_basename( const UNICODE_STRING *basename ) +{ + NTSTATUS status; + ULONG hash; + + status = pRtlHashUnicodeString( basename, TRUE, HASH_STRING_ALGORITHM_DEFAULT, &hash ); + ok( !status, "got %#lx.\n", status ); + return hash & 31; +} + +static void test_hash_links(void) +{ + LIST_ENTRY *hash_map, *entry, *entry2, *mark, *root; + LDR_DATA_TABLE_ENTRY *module; + const WCHAR *modname; + BOOL found; + + /* Hash links structure is the same on older Windows loader but hashing algorithm is different. */ + if (is_old_loader_struct()) return; + + root = &NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList; + module = CONTAINING_RECORD(root->Flink, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks); + hash_map = module->HashLinks.Blink - hash_basename( &module->BaseDllName ); + + for (entry = root->Flink; entry != root; entry = entry->Flink) + { + module = CONTAINING_RECORD(entry, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks); + modname = module->BaseDllName.Buffer; + mark = &hash_map[hash_basename( &module->BaseDllName )]; + found = FALSE; + for (entry2 = mark->Flink; entry2 != mark; entry2 = entry2->Flink) + { + module = CONTAINING_RECORD(entry2, LDR_DATA_TABLE_ENTRY, HashLinks); + if ((found = !lstrcmpiW( module->BaseDllName.Buffer, modname ))) break; + } + ok( found, "Could not find %s.\n", debugstr_w(modname) ); + } +} + START_TEST(module) { WCHAR filenameW[MAX_PATH]; @@ -1848,4 +1890,5 @@ START_TEST(module) test_ddag_node(); test_tls_links(); test_base_address_index_tree(); + test_hash_links(); }
From: Michael Müller michael@fds-team.de
--- dlls/ntdll/loader.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 9e2a32f69d3..c3d3896c8ea 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -647,10 +647,10 @@ static WINE_MODREF *find_basename_module( LPCWSTR name ) if (cached_modref && RtlEqualUnicodeString( &name_str, &cached_modref->ldr.BaseDllName, TRUE )) return cached_modref;
- mark = &NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList; + mark = &hash_table[hash_basename( &name_str )]; for (entry = mark->Flink; entry != mark; entry = entry->Flink) { - WINE_MODREF *mod = CONTAINING_RECORD(entry, WINE_MODREF, ldr.InLoadOrderLinks); + WINE_MODREF *mod = CONTAINING_RECORD(entry, WINE_MODREF, ldr.HashLinks); if (RtlEqualUnicodeString( &name_str, &mod->ldr.BaseDllName, TRUE ) && !mod->system) { cached_modref = CONTAINING_RECORD(mod, WINE_MODREF, ldr);
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149601
Your paranoid android.
=== build (build log) ===
error: patch failed: dlls/ntdll/loader.c:128 error: patch failed: dlls/kernel32/tests/module.c:48 error: patch failed: dlls/ntdll/loader.c:647 Task: Patch failed to apply
=== debian11 (build log) ===
error: patch failed: dlls/ntdll/loader.c:128 error: patch failed: dlls/kernel32/tests/module.c:48 error: patch failed: dlls/ntdll/loader.c:647 Task: Patch failed to apply
=== debian11b (build log) ===
error: patch failed: dlls/ntdll/loader.c:128 error: patch failed: dlls/kernel32/tests/module.c:48 error: patch failed: dlls/ntdll/loader.c:647 Task: Patch failed to apply
v3: - also remove version checks in test (and skip thus crashing tests on Win7 based on loader structure layout).
-
Marvin -
Michael Müller -
Paul Gofman
-
Paul Gofman (＠gofman)