[PATCH v2 0/1] MR5236: Draft: ntdll: Implement NtdllDefWindowProc_A().
I've tried implementing this before but importing user32 from ntdll isn't a reliable thing (so I recently rewrote it using the ntdll equivalents of LoadLibrary() and GetProcAddress()) Note: This is a weird way to implement this function (Windows implements this in ntdll instead) but that would probably require a user32/ntdll rewrite (so this is the next best tning to get the application working) -- v2: ntdll: Implement NtdllDefWindowProc_A(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5236
From: Aida Jonikienė <aidas957(a)gmail.com> The Crinkler linker redirects the DefWindowProcA calls to the NtdllDefWindowProc_A function (which doesn't exist on Wine) and so it picks the first export in ntdll (which is A_SHAFinal right now) which obviously causes a segfault (this is problematic for the application listed in the bug report). Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53321 --- dlls/ntdll/misc.c | 32 ++++++++++++++++++++++++++++++++ dlls/ntdll/ntdll.spec | 1 + 2 files changed, 33 insertions(+) diff --git a/dlls/ntdll/misc.c b/dlls/ntdll/misc.c index ab01c77531c..b5286fefd13 100644 --- a/dlls/ntdll/misc.c +++ b/dlls/ntdll/misc.c @@ -501,3 +501,35 @@ ULONG WINAPIV EtwTraceMessage( TRACEHANDLE handle, ULONG flags, LPGUID guid, /*U va_end( valist ); return ret; } + +/****************************************************************************** + * NtdllDefWindowProc_A (NTDLL.@) + */ +LRESULT WINAPI NtdllDefWindowProc_A( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) +{ + static LPARAM (WINAPI *pDefWindowProcA)(HWND,UINT,WPARAM,LPARAM); /* DefWindowProcA */ + + UNICODE_STRING name; + NTSTATUS status; + HMODULE module; + + if (pDefWindowProcA) return pDefWindowProcA( hwnd, msg, wParam, lParam ); + + RtlCreateUnicodeString( &name, L"user32.dll" ); + if (( status = LdrLoadDll( NULL, 0, &name, &module ))) + { + ERR("Failed to load user32.dll, status %ld\n", status); + if (name.Buffer) RtlFreeUnicodeString( &name ); + return 1; /* FIXME: Is this a good way to indicate failure? */ + } + + if (name.Buffer) RtlFreeUnicodeString( &name ); + pDefWindowProcA = RtlFindExportedRoutineByName( module, "DefWindowProcA" ); + if (!pDefWindowProcA) + { + ERR("Failed to load the function\n"); + return 1; + } + + return pDefWindowProcA( hwnd, msg, wParam, lParam ); +} diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 66995346e57..27efbf3ad3f 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -455,6 +455,7 @@ # @ stub NtWriteRequestData @ stdcall -syscall NtWriteVirtualMemory(long ptr ptr long ptr) @ stdcall -syscall NtYieldExecution() +@ stdcall NtdllDefWindowProc_A(long long long long) @ stub PfxFindPrefix @ stub PfxInitialize @ stub PfxInsertPrefix -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5236
On Thu Mar 7 13:05:34 2024 +0000, Aida Jonikienė wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5236/diffs?diff_id=103683&start_sha=2ee242a6fb89ff34924c11d61377b504a79fbe95#791681ff2a6e63ef1234735237aa6da70dcbcf32_212_212) @iamahuman Any updates on this? I saw that you pinged me on IRC telling me this patch is good enough (except for some nits) :frog:
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5236#note_65240
On Tue Mar 19 00:20:53 2024 +0000, Aida Jonikienė wrote:
@iamahuman Any updates on this? I saw that you pinged me on IRC telling me this patch is good enough (except for some nits) :frog: About forwarded export bug: no, there is no progress.
(I'm assuming you are talking about export forwarders since you commented specifically on this thread.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5236#note_65279
Jinoh Kang (@iamahuman) commented about dlls/ntdll/misc.c:
} + +/****************************************************************************** + * NtdllDefWindowProc_A (NTDLL.@) + */ +LRESULT WINAPI NtdllDefWindowProc_A( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) +{ + static LPARAM (WINAPI *pDefWindowProcA)(HWND,UINT,WPARAM,LPARAM); /* DefWindowProcA */ + + UNICODE_STRING name; + NTSTATUS status; + HMODULE module; + + if (pDefWindowProcA) return pDefWindowProcA( hwnd, msg, wParam, lParam ); + + RtlCreateUnicodeString( &name, L"user32.dll" ); Use `RTL_CONSTANT_STRING` instead. Constant strings don't need to be freed.
```c UNICODE_STRING name = RTL_CONSTANT_STRING( L"user32.dll" ); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5236#note_65563
Jinoh Kang (@iamahuman) commented about dlls/ntdll/misc.c:
+ +/****************************************************************************** + * NtdllDefWindowProc_A (NTDLL.@) + */ +LRESULT WINAPI NtdllDefWindowProc_A( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) +{ + static LPARAM (WINAPI *pDefWindowProcA)(HWND,UINT,WPARAM,LPARAM); /* DefWindowProcA */ + + UNICODE_STRING name; + NTSTATUS status; + HMODULE module; + + if (pDefWindowProcA) return pDefWindowProcA( hwnd, msg, wParam, lParam ); + + RtlCreateUnicodeString( &name, L"user32.dll" ); + if (( status = LdrLoadDll( NULL, 0, &name, &module ))) Don't keep reference to implicitly loaded DLL, which is effectively a memory leak.
Use `LdrGetDllHandle` instead, assuming user32.dll is already loaded (this should be guaranteed by the application). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5236#note_65564
Jinoh Kang (@iamahuman) commented about dlls/ntdll/misc.c:
+ UNICODE_STRING name; + NTSTATUS status; + HMODULE module; + + if (pDefWindowProcA) return pDefWindowProcA( hwnd, msg, wParam, lParam ); + + RtlCreateUnicodeString( &name, L"user32.dll" ); + if (( status = LdrLoadDll( NULL, 0, &name, &module ))) + { + ERR("Failed to load user32.dll, status %ld\n", status); + if (name.Buffer) RtlFreeUnicodeString( &name ); + return 1; /* FIXME: Is this a good way to indicate failure? */ + } + + if (name.Buffer) RtlFreeUnicodeString( &name ); + pDefWindowProcA = RtlFindExportedRoutineByName( module, "DefWindowProcA" ); This leads to data race, since nothing prevents `NtdllDefWindowProc_A` from being called by multiple threads. Since we don't want to keep user32.dll loaded (this should be the application's responsibility), `pDefWindowProcA` should be a local variable instead (i.e., no caching, no `static`).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5236#note_65565
participants (2)
-
Aida Jonikienė -
Jinoh Kang (＠iamahuman)