From ed184da3508b21c2e3b66de5708eced69ab4102e Mon Sep 17 00:00:00 2001 From: Jinoh Kang Date: Thu, 18 Apr 2024 23:58:21 +0900 Subject: [PATCH 1/5] ntdll: Wrap current_modref variable in a new structure. Prepare for adding context information (e.g. static or dynamic) about the current importer, which is needed for correct RelayFromExclude handling. --- dlls/ntdll/loader.c | 70 ++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 2f2a7fe5427..0ce704da2b5 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -181,9 +181,16 @@ static RTL_BITMAP tls_bitmap; static RTL_BITMAP tls_expansion_bitmap; static WINE_MODREF *cached_modref; -static WINE_MODREF *current_modref; static WINE_MODREF *last_failed_modref; +struct importer +{ + struct importer *prev; + WINE_MODREF *modref; +}; + +static struct importer *current_importer; + static LDR_DDAG_NODE *node_ntdll, *node_kernel32; static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, DWORD flags, WINE_MODREF** pwm, BOOL system ); @@ -563,6 +570,20 @@ static ULONG_PTR allocate_stub( const char *dll, const char *name ) static inline ULONG_PTR allocate_stub( const char *dll, const char *name ) { return 0xdeadbeef; } #endif /* __i386__ */ +/* The loader_section must be locked while calling this function. */ +static void push_importer( struct importer *importer, WINE_MODREF *modref ) +{ + importer->modref = modref; + importer->prev = current_importer; + current_importer = importer; +} + +/* The loader_section must be locked while calling this function. */ +static void pop_importer( struct importer *importer ) +{ + current_importer = importer->prev; +} + /* call ldr notifications */ static void call_ldr_notifications( ULONG reason, LDR_DATA_TABLE_ENTRY *module ) { @@ -788,7 +809,7 @@ static NTSTATUS build_import_name( WCHAR buffer[256], const char *import, int le { const API_SET_NAMESPACE *map = NtCurrentTeb()->Peb->ApiSetMap; const API_SET_NAMESPACE_ENTRY *entry; - const WCHAR *host = current_modref ? current_modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *host = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; UNICODE_STRING str; while (len && import[len-1] == ' ') len--; /* remove trailing spaces */ @@ -972,9 +993,9 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS if (load_dll( load_path, mod_name, 0, &wm, imp->system ) == STATUS_SUCCESS && !(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) { - if (!imports_fixup_done && current_modref) + if (!imports_fixup_done && current_importer) { - add_module_dependency( current_modref->ldr.DdagNode, wm->ldr.DdagNode ); + add_module_dependency( current_importer->modref->ldr.DdagNode, wm->ldr.DdagNode ); } else if (process_attach( wm->ldr.DdagNode, NULL ) != STATUS_SUCCESS) { @@ -1042,12 +1063,12 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY if (TRACE_ON(snoop)) { - const WCHAR *user = current_modref ? current_modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *user = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; proc = SNOOP_GetProcAddress( module, exports, exp_size, proc, ordinal, user ); } if (TRACE_ON(relay)) { - const WCHAR *user = current_modref ? current_modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *user = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; proc = RELAY_GetProcAddress( module, exports, exp_size, proc, ordinal, user ); } return proc; @@ -1140,7 +1161,8 @@ void * WINAPI RtlFindExportedRoutineByName( HMODULE module, const char *name ) */ static BOOL import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *descr, LPCWSTR load_path, WINE_MODREF **pwm ) { - BOOL system = current_modref->system || (current_modref->ldr.Flags & LDR_WINE_INTERNAL); + struct importer *importer = current_importer; + BOOL system = importer->modref->system || (importer->modref->ldr.Flags & LDR_WINE_INTERNAL); NTSTATUS status; WINE_MODREF *wmImp; HMODULE imp_mod; @@ -1175,10 +1197,10 @@ static BOOL import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *descr, LP { if (status == STATUS_DLL_NOT_FOUND) ERR("Library %s (which is needed by %s) not found\n", - name, debugstr_w(current_modref->ldr.FullDllName.Buffer)); + name, debugstr_w(importer->modref->ldr.FullDllName.Buffer)); else ERR("Loading library %s (which is needed by %s) failed (error %lx).\n", - name, debugstr_w(current_modref->ldr.FullDllName.Buffer), status); + name, debugstr_w(importer->modref->ldr.FullDllName.Buffer), status); return FALSE; } @@ -1211,7 +1233,7 @@ static BOOL import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *descr, LP thunk_list->u1.Function = allocate_stub( name, (const char*)pe_name->Name ); } WARN(" imported from %s, allocating stub %p\n", - debugstr_w(current_modref->ldr.FullDllName.Buffer), + debugstr_w(importer->modref->ldr.FullDllName.Buffer), (void *)thunk_list->u1.Function ); import_list++; thunk_list++; @@ -1231,7 +1253,7 @@ static BOOL import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *descr, LP { thunk_list->u1.Function = allocate_stub( name, IntToPtr(ordinal) ); WARN("No implementation for %s.%d imported from %s, setting to %p\n", - name, ordinal, debugstr_w(current_modref->ldr.FullDllName.Buffer), + name, ordinal, debugstr_w(importer->modref->ldr.FullDllName.Buffer), (void *)thunk_list->u1.Function ); } TRACE_(imports)("--- Ordinal %s.%d = %p\n", name, ordinal, (void *)thunk_list->u1.Function ); @@ -1247,7 +1269,7 @@ static BOOL import_dll( HMODULE module, const IMAGE_IMPORT_DESCRIPTOR *descr, LP { thunk_list->u1.Function = allocate_stub( name, (const char*)pe_name->Name ); WARN("No implementation for %s.%s imported from %s, setting to %p\n", - name, pe_name->Name, debugstr_w(current_modref->ldr.FullDllName.Buffer), + name, pe_name->Name, debugstr_w(importer->modref->ldr.FullDllName.Buffer), (void *)thunk_list->u1.Function ); } TRACE_(imports)("--- %s %s.%d = %p\n", @@ -1446,21 +1468,21 @@ static void free_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) */ static NTSTATUS fixup_imports_ilonly( WINE_MODREF *wm, LPCWSTR load_path, void **entry ) { + struct importer importer; NTSTATUS status; void *proc; const char *name; - WINE_MODREF *prev, *imp; + WINE_MODREF *imp; if (!(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) return STATUS_SUCCESS; /* already done */ wm->ldr.Flags &= ~LDR_DONT_RESOLVE_REFS; - prev = current_modref; - current_modref = wm; + push_importer( &importer, wm ); assert( !wm->ldr.DdagNode->Dependencies.Tail ); if (!(status = load_dll( load_path, L"mscoree.dll", 0, &imp, FALSE )) && !add_module_dependency_after( wm->ldr.DdagNode, imp->ldr.DdagNode, NULL )) status = STATUS_NO_MEMORY; - current_modref = prev; + pop_importer( &importer ); if (status) { ERR( "mscoree.dll not found, IL-only binary %s cannot be loaded\n", @@ -1487,7 +1509,8 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) { const IMAGE_IMPORT_DESCRIPTOR *imports; SINGLE_LIST_ENTRY *dep_after; - WINE_MODREF *prev, *imp; + struct importer importer; + WINE_MODREF *imp; int i, nb_imports; DWORD size; NTSTATUS status; @@ -1513,8 +1536,7 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) /* load the imported modules. They are automatically * added to the modref list of the process. */ - prev = current_modref; - current_modref = wm; + push_importer( &importer, wm ); status = STATUS_SUCCESS; for (i = 0; i < nb_imports; i++) { @@ -1524,7 +1546,7 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) else if (imp && imp->ldr.DdagNode != node_ntdll && imp->ldr.DdagNode != node_kernel32) add_module_dependency_after( wm->ldr.DdagNode, imp->ldr.DdagNode, dep_after ); } - current_modref = prev; + pop_importer( &importer ); if (wm->ldr.ActivationContext) RtlDeactivateActivationContext( 0, cookie ); return status; } @@ -1804,8 +1826,9 @@ static NTSTATUS process_attach( LDR_DDAG_NODE *node, LPVOID lpReserved ) /* Call DLL entry point */ if (status == STATUS_SUCCESS) { - WINE_MODREF *prev = current_modref; - current_modref = wm; + struct importer importer; + + push_importer( &importer, wm ); call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_LOADED, &wm->ldr ); status = MODULE_InitDLL( wm, DLL_PROCESS_ATTACH, lpReserved ); @@ -1822,7 +1845,8 @@ static NTSTATUS process_attach( LDR_DDAG_NODE *node, LPVOID lpReserved ) last_failed_modref = wm; WARN("Initialization of %s failed\n", debugstr_w(wm->ldr.BaseDllName.Buffer)); } - current_modref = prev; + + pop_importer( &importer ); } if (wm->ldr.ActivationContext) RtlDeactivateActivationContext( 0, cookie ); From c17600b062b4435626a8a13a79ac45083aaf5b2e Mon Sep 17 00:00:00 2001 From: Jinoh Kang Date: Fri, 19 Apr 2024 00:02:07 +0900 Subject: [PATCH 2/5] ntdll: Set export forwarder DLL as the dynamic importer in LdrGetProcedureAddress(). This matches the Windows GetProcAddress() behavior when handling dynamic module dependencies. To avoid breaking WINEDEBUG=+relay, flag dynamic importers (`is_dynamic = TRUE`) and explicitly ignore them when testing for RelayFromExclude modules. Otherwise, GetProcAddress() on kernel32 export forwarders (which comprise most of the exports) will be recognized as relaying *from* kernel32 itself (instead of the actual importer) and will be subsequently excluded from WINEDEBUG=+relay due to the default `RelayFromExclude` which includes `kernel32`. The current behavior is to treat it as relaying from the actual importer, not kernel32, so this doen't become a problem. This bit is true for all export forwarder DLLs in general, and also affects RelayFromInclude as well as SnoopFrom{Exclude,Include} (+snoop). --- dlls/ntdll/loader.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 0ce704da2b5..ea01263d023 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -187,6 +187,7 @@ struct importer { struct importer *prev; WINE_MODREF *modref; + BOOL is_dynamic; }; static struct importer *current_importer; @@ -571,9 +572,10 @@ static inline ULONG_PTR allocate_stub( const char *dll, const char *name ) { ret #endif /* __i386__ */ /* The loader_section must be locked while calling this function. */ -static void push_importer( struct importer *importer, WINE_MODREF *modref ) +static void push_importer( struct importer *importer, WINE_MODREF *modref, BOOL is_dynamic ) { importer->modref = modref; + importer->is_dynamic = is_dynamic; importer->prev = current_importer; current_importer = importer; } @@ -584,6 +586,20 @@ static void pop_importer( struct importer *importer ) current_importer = importer->prev; } +/* The loader_section must be locked while calling this function. */ +static const WCHAR *get_last_static_importer_name(void) +{ + struct importer *importer; + for (importer = current_importer; importer != NULL; importer = importer->prev) + { + if (!importer->is_dynamic) + { + return importer->modref->ldr.BaseDllName.Buffer; + } + } + return NULL; +} + /* call ldr notifications */ static void call_ldr_notifications( ULONG reason, LDR_DATA_TABLE_ENTRY *module ) { @@ -1063,12 +1079,12 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY if (TRACE_ON(snoop)) { - const WCHAR *user = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *user = get_last_static_importer_name(); proc = SNOOP_GetProcAddress( module, exports, exp_size, proc, ordinal, user ); } if (TRACE_ON(relay)) { - const WCHAR *user = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *user = get_last_static_importer_name(); proc = RELAY_GetProcAddress( module, exports, exp_size, proc, ordinal, user ); } return proc; @@ -1477,7 +1493,7 @@ static NTSTATUS fixup_imports_ilonly( WINE_MODREF *wm, LPCWSTR load_path, void * if (!(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) return STATUS_SUCCESS; /* already done */ wm->ldr.Flags &= ~LDR_DONT_RESOLVE_REFS; - push_importer( &importer, wm ); + push_importer( &importer, wm, FALSE ); assert( !wm->ldr.DdagNode->Dependencies.Tail ); if (!(status = load_dll( load_path, L"mscoree.dll", 0, &imp, FALSE )) && !add_module_dependency_after( wm->ldr.DdagNode, imp->ldr.DdagNode, NULL )) @@ -1536,7 +1552,7 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) /* load the imported modules. They are automatically * added to the modref list of the process. */ - push_importer( &importer, wm ); + push_importer( &importer, wm, FALSE ); status = STATUS_SUCCESS; for (i = 0; i < nb_imports; i++) { @@ -1828,7 +1844,7 @@ static NTSTATUS process_attach( LDR_DDAG_NODE *node, LPVOID lpReserved ) { struct importer importer; - push_importer( &importer, wm ); + push_importer( &importer, wm, FALSE ); call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_LOADED, &wm->ldr ); status = MODULE_InitDLL( wm, DLL_PROCESS_ATTACH, lpReserved ); @@ -2112,8 +2128,14 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name, else if ((exports = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_EXPORT, &exp_size ))) { - void *proc = name ? find_named_export( module, exports, exp_size, name->Buffer, -1, NULL ) - : find_ordinal_export( module, exports, exp_size, ord - exports->Base, NULL ); + struct importer importer; + void *proc; + + push_importer( &importer, wm, TRUE ); + proc = name ? find_named_export( module, exports, exp_size, name->Buffer, -1, NULL ) + : find_ordinal_export( module, exports, exp_size, ord - exports->Base, NULL ); + pop_importer( &importer ); + if (proc) { *address = proc; From 70c7ad699e99be555301c6666739cad295ac3872 Mon Sep 17 00:00:00 2001 From: Jinoh Kang Date: Thu, 18 Apr 2024 23:23:32 +0900 Subject: [PATCH 3/5] ntdll: Remove some unnecessary NULL checks for current_importer. current_importer is now set by all callers of build_import_name, find_forwarded_export, and find_ordinal_export. --- 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 ea01263d023..5f7ce313113 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -825,7 +825,7 @@ static NTSTATUS build_import_name( WCHAR buffer[256], const char *import, int le { const API_SET_NAMESPACE *map = NtCurrentTeb()->Peb->ApiSetMap; const API_SET_NAMESPACE_ENTRY *entry; - const WCHAR *host = current_importer ? current_importer->modref->ldr.BaseDllName.Buffer : NULL; + const WCHAR *host = current_importer->modref->ldr.BaseDllName.Buffer; UNICODE_STRING str; while (len && import[len-1] == ' ') len--; /* remove trailing spaces */ @@ -1009,7 +1009,7 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS if (load_dll( load_path, mod_name, 0, &wm, imp->system ) == STATUS_SUCCESS && !(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) { - if (!imports_fixup_done && current_importer) + if (!imports_fixup_done) { add_module_dependency( current_importer->modref->ldr.DdagNode, wm->ldr.DdagNode ); } From 7686456a4e8e427dde11b958c480eda7cc26e1d5 Mon Sep 17 00:00:00 2001 From: Jinoh Kang Date: Thu, 18 Apr 2024 23:23:43 +0900 Subject: [PATCH 4/5] ntdll: Don't re-add a module dependency if it already exists. Today, calling add_module_dependency() multiple times with the same arguments results in duplicate edges. Duplicate edges are harmless, but bloats memory usage. The number of duplicate edges does not affect the dependency graph; the graph is determined by the set of unique edges. Consciously avoid duplicates by checking for them in add_module_dependency_after(). This allows us to generate a unique dependency edge for all imports of export forwarders that belong to the same DLL. --- dlls/ntdll/loader.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 5f7ce313113..29108c401cc 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -925,6 +925,21 @@ static void remove_single_list_entry( LDRP_CSLIST *list, SINGLE_LIST_ENTRY *entr entry->Next = NULL; } +static LDR_DEPENDENCY *find_module_dependency( LDR_DDAG_NODE *from, LDR_DDAG_NODE *to ) +{ + SINGLE_LIST_ENTRY *entry, *mark = from->Dependencies.Tail; + + if (!mark) return NULL; + + for (entry = mark->Next; entry != mark; entry = entry->Next) + { + LDR_DEPENDENCY *dep = CONTAINING_RECORD( entry, LDR_DEPENDENCY, dependency_to_entry ); + if (dep->dependency_from == from && dep->dependency_to == to) return dep; + } + + return NULL; +} + /********************************************************************** * add_module_dependency_after */ @@ -933,6 +948,15 @@ static BOOL add_module_dependency_after( LDR_DDAG_NODE *from, LDR_DDAG_NODE *to, { LDR_DEPENDENCY *dep; + if ((dep = find_module_dependency( from, to ))) + { + /* Dependency already exists; consume the module reference stolen from the caller */ + LDR_DATA_TABLE_ENTRY *mod = CONTAINING_RECORD( to->Modules.Flink, LDR_DATA_TABLE_ENTRY, NodeModuleLink ); + WINE_MODREF *wm = CONTAINING_RECORD( mod, WINE_MODREF, ldr ); + LdrUnloadDll( wm->ldr.DllBase ); + return TRUE; + } + if (!(dep = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*dep) ))) return FALSE; dep->dependency_from = from; From 5ffce73492a388ebc5588c8b2154b0047420905a Mon Sep 17 00:00:00 2001 From: Jinoh Kang Date: Fri, 19 Apr 2024 00:42:57 +0900 Subject: [PATCH 5/5] ntdll: Properly track refcount with forwarded exports. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52094 --- dlls/kernel32/tests/loader.c | 2 -- dlls/ntdll/loader.c | 17 ++++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 9b0f8f6bff2..36f04afbf3d 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -2775,7 +2775,6 @@ static void subtest_export_forwarder_dep_chain( size_t num_chained_export_module /* FreeLibrary() should *not* unload the DLL immediately */ module = GetModuleHandleA( temp_paths[i] ); - todo_wine_if(i < ultimate_depender_index && i + 1 != importer_index) ok( module == modules[i], "modules[%Iu] expected %p, got %p (unloaded?) err=%lu\n", i, modules[i], module, GetLastError() ); } @@ -2787,7 +2786,6 @@ static void subtest_export_forwarder_dep_chain( size_t num_chained_export_module { HMODULE module = GetModuleHandleA( temp_paths[i] ); - todo_wine_if(i < ultimate_depender_index && i + 1 != importer_index) ok( module == modules[i], "modules[%Iu] expected %p, got %p (unloaded?) err=%lu\n", i, modules[i], module, GetLastError() ); } diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 29108c401cc..eadda01ac14 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1033,11 +1033,7 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS if (load_dll( load_path, mod_name, 0, &wm, imp->system ) == STATUS_SUCCESS && !(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) { - if (!imports_fixup_done) - { - add_module_dependency( current_importer->modref->ldr.DdagNode, wm->ldr.DdagNode ); - } - else if (process_attach( wm->ldr.DdagNode, NULL ) != STATUS_SUCCESS) + if (imports_fixup_done && process_attach( wm->ldr.DdagNode, NULL ) != STATUS_SUCCESS) { LdrUnloadDll( wm->ldr.DllBase ); wm = NULL; @@ -1051,6 +1047,11 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS return NULL; } } + else + { + if (wm->ldr.LoadCount != -1) wm->ldr.LoadCount++; + } + if ((exports = RtlImageDirectoryEntryToData( wm->ldr.DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_EXPORT, &exp_size ))) { @@ -1070,6 +1071,12 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS forward, debugstr_w(get_modref(module)->ldr.FullDllName.Buffer), debugstr_w(get_modref(module)->ldr.BaseDllName.Buffer) ); } + else if (wm->ldr.DdagNode != node_ntdll && wm->ldr.DdagNode != node_kernel32) + { + add_module_dependency( current_importer->modref->ldr.DdagNode, wm->ldr.DdagNode ); + wm = NULL; + } + if (wm) LdrUnloadDll( wm->ldr.DllBase ); return proc; }