[PATCH 0/1] MR4999: comctl32: Make functions that DxWnd can hot-patch hot-patchable.
[DxWnd](https://sourceforge.net/p/dxwnd/home/Home/) is a remarkable tool for working with badly behaved old games. In [the DxWnd source code](https://sourceforge.net/projects/dxwnd/files/Sources/v2_06_01_src.rar/downlo...), see dll/comctl32.cpp for the list of hot-patched functions and dll/dxhook.cpp for the actual hot-patching. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999
From: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/comctl32/flatsb.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/dlls/comctl32/flatsb.c b/dlls/comctl32/flatsb.c index 1cedca98a6c..715b6b9b87a 100644 --- a/dlls/comctl32/flatsb.c +++ b/dlls/comctl32/flatsb.c @@ -60,7 +60,7 @@ typedef struct * Subclasses specified window so that flat scroll bars may be drawn * and used. */ -BOOL WINAPI InitializeFlatSB(HWND hwnd) +BOOL WINAPI DECLSPEC_HOTPATCH InitializeFlatSB(HWND hwnd) { TRACE("[%p]\n", hwnd); return TRUE; @@ -80,7 +80,7 @@ BOOL WINAPI InitializeFlatSB(HWND hwnd) * Removes any subclassing on the specified window so that regular * scroll bars are drawn and used. */ -HRESULT WINAPI UninitializeFlatSB(HWND hwnd) +HRESULT WINAPI DECLSPEC_HOTPATCH UninitializeFlatSB(HWND hwnd) { TRACE("[%p]\n", hwnd); return S_FALSE; @@ -96,7 +96,7 @@ HRESULT WINAPI UninitializeFlatSB(HWND hwnd) * the return is nonzero if InitializeFlatSB has been called for this window, or * zero otherwise. */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_GetScrollProp(HWND hwnd, INT propIndex, LPINT prop) { TRACE("[%p] propIndex=%d\n", hwnd, propIndex); @@ -112,7 +112,7 @@ FlatSB_GetScrollProp(HWND hwnd, INT propIndex, LPINT prop) * Success: Non-zero * Failure: Zero */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_SetScrollProp(HWND hwnd, UINT index, INT newValue, BOOL flag) { TRACE("[%p] index=%u newValue=%d flag=%d\n", hwnd, index, newValue, flag); @@ -140,7 +140,7 @@ FlatSB_SetScrollProp(HWND hwnd, UINT index, INT newValue, BOOL flag) * * See EnableScrollBar. */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_EnableScrollBar(HWND hwnd, int nBar, UINT flags) { return EnableScrollBar(hwnd, nBar, flags); @@ -151,7 +151,7 @@ FlatSB_EnableScrollBar(HWND hwnd, int nBar, UINT flags) * * See ShowScrollBar. */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_ShowScrollBar(HWND hwnd, int nBar, BOOL fShow) { return ShowScrollBar(hwnd, nBar, fShow); @@ -162,7 +162,7 @@ FlatSB_ShowScrollBar(HWND hwnd, int nBar, BOOL fShow) * * See GetScrollRange. */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_GetScrollRange(HWND hwnd, int nBar, LPINT min, LPINT max) { return GetScrollRange(hwnd, nBar, min, max); @@ -173,7 +173,7 @@ FlatSB_GetScrollRange(HWND hwnd, int nBar, LPINT min, LPINT max) * * See GetScrollInfo. */ -BOOL WINAPI +BOOL WINAPI DECLSPEC_HOTPATCH FlatSB_GetScrollInfo(HWND hwnd, int nBar, LPSCROLLINFO info) { return GetScrollInfo(hwnd, nBar, info); @@ -184,7 +184,7 @@ FlatSB_GetScrollInfo(HWND hwnd, int nBar, LPSCROLLINFO info) * * See GetScrollPos. */ -INT WINAPI +INT WINAPI DECLSPEC_HOTPATCH FlatSB_GetScrollPos(HWND hwnd, int nBar) { return GetScrollPos(hwnd, nBar); @@ -195,7 +195,7 @@ FlatSB_GetScrollPos(HWND hwnd, int nBar) * * See SetScrollPos. */ -INT WINAPI +INT WINAPI DECLSPEC_HOTPATCH FlatSB_SetScrollPos(HWND hwnd, int nBar, INT pos, BOOL bRedraw) { return SetScrollPos(hwnd, nBar, pos, bRedraw); @@ -206,7 +206,7 @@ FlatSB_SetScrollPos(HWND hwnd, int nBar, INT pos, BOOL bRedraw) * * See SetScrollInfo. */ -INT WINAPI +INT WINAPI DECLSPEC_HOTPATCH FlatSB_SetScrollInfo(HWND hwnd, int nBar, LPSCROLLINFO info, BOOL bRedraw) { return SetScrollInfo(hwnd, nBar, info, bRedraw); @@ -217,7 +217,7 @@ FlatSB_SetScrollInfo(HWND hwnd, int nBar, LPSCROLLINFO info, BOOL bRedraw) * * See SetScrollRange. */ -INT WINAPI +INT WINAPI DECLSPEC_HOTPATCH FlatSB_SetScrollRange(HWND hwnd, int nBar, INT min, INT max, BOOL bRedraw) { return SetScrollRange(hwnd, nBar, min, max, bRedraw); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4999
I had a quick look at HotPatch function in hotpatch.cpp and it has two paths. One under #ifdef USEMINHOOK which ends up CreateTrampolineFunction and doesn't assume exact hotpatch prologue, it does some disassembly and minds different instruction sizes. Such things were usually breaking under Wine before PE builds but with PE builds usually just work without DECLSPEC_HOTPATCH. Another path is checking this (it is the failure condition, i. e., to succeed it needs one of memcmp to match; patch_address is 5 bytes before function start): 'if(memcmp( "\x90\x90\x90\x90\x90\x8B\xFF", patch_address, 7) && memcmp( "\x90\x90\x90\x90\x90\x89\xFF", patch_address, 7)){' That is going to fail even with DECLSPEC_HOTPATCH because we usually have 0xcc (int3) instead of 0x90 (nop) as padding between functions. So I didn't try how that actually works nor studied that code completely, but are you sure that it doesn't work without DECLSPEC_HOTPATCH on all those functions and does work with DECLSPEC_HOTPATCH added? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999#note_59828
On Thu Feb 1 19:54:53 2024 +0000, Paul Gofman wrote:
I had a quick look at HotPatch function in hotpatch.cpp and it has two paths. One under #ifdef USEMINHOOK which ends up CreateTrampolineFunction and doesn't assume exact hotpatch prologue, it does some disassembly and minds different instruction sizes. Such things were usually breaking under Wine before PE builds but with PE builds usually just work without DECLSPEC_HOTPATCH. Another path is checking this (it is the failure condition, i. e., to succeed it needs one of memcmp to match; patch_address is 5 bytes before function start): 'if(memcmp( "\x90\x90\x90\x90\x90\x8B\xFF", patch_address, 7) && memcmp( "\x90\x90\x90\x90\x90\x89\xFF", patch_address, 7)){' That is going to fail even with DECLSPEC_HOTPATCH because we usually have 0xcc (int3) instead of 0x90 (nop) as padding between functions. So I didn't try how that actually works nor studied that code completely, but are you sure that it doesn't work without DECLSPEC_HOTPATCH on all those functions and does work with DECLSPEC_HOTPATCH added? Thanks for looking into it Paul. I didn't see how the code could possibly work without the `mov %edi %edi` prologue, but I missed the part where DxWnd does disassembly. I will have to figure out which checkboxes in the user interface enable which hooks and then test them individually. For now, I've downgraded the 5 patches I sent to "Draft" status.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999#note_59839
On Thu Feb 1 19:54:53 2024 +0000, Alex Henrie wrote:
Thanks for looking into it Paul. I didn't see how the code could possibly work without the `mov %edi %edi` prologue, but I missed the part where DxWnd does disassembly. I will have to figure out which checkboxes in the user interface enable which hooks and then test them individually. For now, I've downgraded the 5 patches I sent to "Draft" status. Note that on the other path I referenced (if it is ever taken even, that is under #ifdef and depends not on user boxes but on how the thing is compiled) is not going to work even with DECLSPEC_HOTPATCH, it assumes alignment filler. Without checking, I am not sure if that works on Windows even.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999#note_59840
On Thu Feb 1 20:02:00 2024 +0000, Paul Gofman wrote:
Note that on the other path I referenced (if it is ever taken even, that is under #ifdef and depends not on user boxes but on how the thing is compiled) is not going to work even with DECLSPEC_HOTPATCH, it assumes alignment filler. Without checking, I am not sure if that works on Windows even. I found DxWnd's setting to log hotpatch failures and so far I haven't seen any such errors in the log. If I notice any in the future, I will send specific patches for particular functions. I also found a "Do hook update" option that allows the "Synchronize GDI to ddraw palette" option to work in Sega Bug without crashing (see [Bug 56232 Comment 2](https://bugs.winehq.org/show_bug.cgi?id=56232#c2)). Thanks again for your insight!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999#note_59899
This merge request was closed by Alex Henrie. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4999
participants (3)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Paul Gofman (＠gofman)