[wine-gitlab] [PATCH v3 0/4] MR375: ntdll: Fixes for runtime 64-bit shift functions.
Based on <https://source.winehq.org/patches/data/238821> by Zhao Yi. -- v3: ntdll: Correctly handle shift greater than the type width in 64-bit shift functions. ntdll: Avoid depending on compiler support for 64-bit shift functions. ntdll/tests: Add tests for runtime 64-bit shift functions. ntdll: Fix the calling convention for runtime 64-bit shift functions. https://gitlab.winehq.org/wine/wine/-/merge_requests/375
From: Zebediah Figura <zfigura(a)codeweavers.com> Based on a patch by Zhao Yi. --- dlls/ntdll/large_int.c | 74 ++++++++++++++--------------- dlls/ntdll/ntdll.spec | 6 +-- dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 6 +-- 3 files changed, 42 insertions(+), 44 deletions(-) diff --git a/dlls/ntdll/large_int.c b/dlls/ntdll/large_int.c index b38074158c8..15716b132a2 100644 --- a/dlls/ntdll/large_int.c +++ b/dlls/ntdll/large_int.c @@ -809,40 +809,55 @@ ULONGLONG WINAPI _aulldiv( ULONGLONG a, ULONGLONG b ) return udivmod(a, b, NULL); } + +LONGLONG __stdcall __regs__allshl( LONGLONG a, unsigned char b ) +{ + return a << b; +} + /****************************************************************************** * _allshl (NTDLL.@) - * - * Shift a 64 bit integer to the left. - * - * PARAMS - * a [I] Initial number. - * b [I] Number to shift a by to the left. - * - * RETURNS - * The left-shifted value. */ -LONGLONG WINAPI _allshl( LONGLONG a, LONG b ) +__ASM_GLOBAL_FUNC( _allshl, + "xchgl (%esp),%ecx\n\t" + "pushl %edx\n\t" + "pushl %eax\n\t" + "pushl %ecx\n\t" + "jmp " __ASM_STDCALL("__regs__allshl", 12) ) + + +LONGLONG __stdcall __regs__allshr( LONGLONG a, unsigned char b ) { - return a << b; + return a >> b; } /****************************************************************************** * _allshr (NTDLL.@) - * - * Shift a 64 bit integer to the right. - * - * PARAMS - * a [I] Initial number. - * b [I] Number to shift a by to the right. - * - * RETURNS - * The right-shifted value. */ -LONGLONG WINAPI _allshr( LONGLONG a, LONG b ) +__ASM_GLOBAL_FUNC( _allshr, + "xchgl (%esp),%ecx\n\t" + "pushl %edx\n\t" + "pushl %eax\n\t" + "pushl %ecx\n\t" + "jmp " __ASM_STDCALL("__regs__allshr", 12) ) + + +ULONGLONG __stdcall __regs__aullshr( ULONGLONG a, unsigned char b ) { return a >> b; } +/****************************************************************************** + * _allshr (NTDLL.@) + */ +__ASM_GLOBAL_FUNC( _aullshr, + "xchgl (%esp),%ecx\n\t" + "pushl %edx\n\t" + "pushl %eax\n\t" + "pushl %ecx\n\t" + "jmp " __ASM_STDCALL("__regs__aullshr", 12) ) + + /****************************************************************************** * _alldvrm (NTDLL.@) * @@ -899,23 +914,6 @@ ULONGLONG WINAPI _aullrem( ULONGLONG a, ULONGLONG b ) return r; } -/****************************************************************************** - * _aullshr (NTDLL.@) - * - * Shift a 64 bit unsigned integer to the right. - * - * PARAMS - * a [I] Initial number. - * b [I] Number to shift a by to the right. - * - * RETURNS - * The right-shifted value. - */ -ULONGLONG WINAPI _aullshr( ULONGLONG a, LONG b ) -{ - return a >> b; -} - /****************************************************************************** * _aulldvrm (NTDLL.@) * diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 1862358e593..89b05728951 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1492,13 +1492,13 @@ @ cdecl -norelay -arch=i386 -ret64 _allmul(int64 int64) @ cdecl -arch=i386 -norelay _alloca_probe() @ cdecl -norelay -arch=i386 -ret64 _allrem(int64 int64) -@ stdcall -arch=i386 -ret64 _allshl(int64 long) -@ stdcall -arch=i386 -ret64 _allshr(int64 long) +@ cdecl -norelay -arch=i386 -ret64 _allshl(int64 long) +@ cdecl -norelay -arch=i386 -ret64 _allshr(int64 long) @ cdecl -ret64 _atoi64(str) @ cdecl -norelay -arch=i386 -ret64 _aulldiv(int64 int64) @ cdecl -arch=i386 -norelay _aulldvrm(int64 int64) @ cdecl -norelay -arch=i386 -ret64 _aullrem(int64 int64) -@ stdcall -arch=i386 -ret64 _aullshr(int64 long) +@ cdecl -norelay -arch=i386 -ret64 _aullshr(int64 long) @ cdecl -arch=i386 -norelay _chkstk() @ stub _fltused @ cdecl -arch=i386 -ret64 _ftol() diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 8b0ee1c4b51..460d7d0459f 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -1540,12 +1540,12 @@ @ cdecl -arch=i386 -norelay -ret64 _allmul(int64 int64) @ cdecl -arch=i386 -norelay _alloca_probe() @ cdecl -arch=i386 -norelay -ret64 _allrem(int64 int64) -@ stdcall -arch=i386 -ret64 _allshl(int64 long) -@ stdcall -arch=i386 -ret64 _allshr(int64 long) +@ cdecl -arch=i386 -norelay -ret64 _allshl(int64 long) +@ cdecl -arch=i386 -norelay -ret64 _allshr(int64 long) @ cdecl -arch=i386 -norelay -ret64 _aulldiv(int64 int64) @ cdecl -arch=i386 -norelay _aulldvrm(int64 int64) @ cdecl -arch=i386 -norelay -ret64 _aullrem(int64 int64) -@ stdcall -arch=i386 -ret64 _aullshr(int64 long) +@ cdecl -arch=i386 -norelay -ret64 _aullshr(int64 long) @ cdecl -arch=i386 -norelay _chkstk() @ cdecl -arch=i386 _except_handler2(ptr ptr ptr ptr) @ cdecl -arch=i386 _except_handler3(ptr ptr ptr ptr) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/375
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/tests/large_int.c | 79 ++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/dlls/ntdll/tests/large_int.c b/dlls/ntdll/tests/large_int.c index 9635cac24ad..747aa9ea8d2 100644 --- a/dlls/ntdll/tests/large_int.c +++ b/dlls/ntdll/tests/large_int.c @@ -39,6 +39,7 @@ static LONGLONG (WINAPI *p_allrem)( LONGLONG a, LONGLONG b ); static LONGLONG (WINAPI *p_allmul)( LONGLONG a, LONGLONG b ); static ULONGLONG (WINAPI *p_aulldiv)( ULONGLONG a, ULONGLONG b ); static ULONGLONG (WINAPI *p_aullrem)( ULONGLONG a, ULONGLONG b ); +static void *p_allshl, *p_allshr, *p_aullshr; static void InitFunctionPtrs(void) { @@ -54,8 +55,11 @@ static void InitFunctionPtrs(void) p_alldiv = (void *)GetProcAddress(hntdll, "_alldiv"); p_allrem = (void *)GetProcAddress(hntdll, "_allrem"); p_allmul = (void *)GetProcAddress(hntdll, "_allmul"); + p_allshl = (void *)GetProcAddress(hntdll, "_allshl"); + p_allshr = (void *)GetProcAddress(hntdll, "_allshr"); p_aulldiv = (void *)GetProcAddress(hntdll, "_aulldiv"); p_aullrem = (void *)GetProcAddress(hntdll, "_aullrem"); + p_aullshr = (void *)GetProcAddress(hntdll, "_aullshr"); } /* if */ } @@ -445,9 +449,31 @@ static void test_RtlLargeIntegerToChar(void) static void test_builtins(void) { #ifdef __i386__ + void *code_mem; ULONGLONG u; LONGLONG l; + static const BYTE call_shift_code[] = + { + 0x55, /* pushl %ebp */ + 0x89, 0xe5, /* movl %esp,%ebp */ + 0x31, 0xc0, /* xorl %eax,%eax */ + 0x31, 0xd2, /* xorl %edx,%edx */ + 0x31, 0xc9, /* xorl %ecx,%ecx */ + 0x87, 0x45, 0x0c, /* xchgl 12(%ebp),%eax */ + 0x87, 0x55, 0x10, /* xchgl 16(%ebp),%edx */ + 0x87, 0x4d, 0x14, /* xchgl 20(%ebp),%ecx */ + 0xff, 0x55, 0x08, /* call *8(%ebp) */ + 0x39, 0xe5, /* cmpl %esp,%ebp */ + 0x75, 0x05, /* jne 1f */ + 0xb9, 0xef, 0xbe, 0xad, 0xde, /* movl $0xdeadbeef,%ecx */ + 0xc9, /* leave */ + 0xc3, /* ret */ + }; + LONGLONG (__cdecl *call_shift_func)(void *func, LONGLONG a, LONG b); + + code_mem = VirtualAlloc(NULL, 0x1000, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); + l = p_alldiv(100, 7); ok(l == 14, "_alldiv returned %s\n", wine_dbgstr_longlong(l)); @@ -489,6 +515,59 @@ static void test_builtins(void) l = p_allmul(0x300000001ll, 4); ok(l == 0xc00000004, "_allmul = %s\n", wine_dbgstr_longlong(l)); + + memcpy(code_mem, call_shift_code, sizeof(call_shift_code)); + call_shift_func = code_mem; + + l = call_shift_func(p_allshl, 0x0123456789abcdefll, 12); + ok(l == 0x3456789abcdef000ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshl, 0x0123456789abcdefll, 44); + ok(l == 0xbcdef00000000000ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshl, 0x0123456789abcdefll, 88); + todo_wine ok(!l, "got %#I64x\n", l); + + l = call_shift_func(p_allshl, 0x0123456789abcdefll, 0x88); + todo_wine ok(!l, "got %#I64x\n", l); + + l = call_shift_func(p_allshl, 0x0123456789abcdefll, 0x108); + ok(l == 0x23456789abcdef00ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x0123456789abcdefll, 12); + ok(l == 0x0123456789abcll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x0123456789abcdefll, 44); + ok(l == 0x01234ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x0123456789abcdefll, 88); + todo_wine ok(!l, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x8123456789abcdefll, 12); + ok(l == 0xfff8123456789abcll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x8123456789abcdefll, 44); + ok(l == 0xfffffffffff81234ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x8123456789abcdefll, 88); + todo_wine ok(l == -1ll, "got %#I64x\n", l); + + l = call_shift_func(p_allshr, 0x8123456789abcdefll, 0x108); + ok(l == 0xff8123456789abcdll, "got %#I64x\n", l); + + l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 12); + ok(l == 0x8123456789abcll, "got %#I64x\n", l); + + l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 44); + ok(l == 0x81234ll, "got %#I64x\n", l); + + l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 88); + todo_wine ok(!l, "got %#I64x\n", l); + + l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 0x108); + ok(l == 0x8123456789abcdll, "got %#I64x\n", l); + + VirtualFree(code_mem, 0, MEM_RELEASE); #endif /* __i386__ */ } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/375
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/large_int.c | 45 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/large_int.c b/dlls/ntdll/large_int.c index 15716b132a2..038a552545f 100644 --- a/dlls/ntdll/large_int.c +++ b/dlls/ntdll/large_int.c @@ -812,7 +812,20 @@ ULONGLONG WINAPI _aulldiv( ULONGLONG a, ULONGLONG b ) LONGLONG __stdcall __regs__allshl( LONGLONG a, unsigned char b ) { - return a << b; + const LARGE_INTEGER x = { .QuadPart = a }; + LARGE_INTEGER ret; + + if (b >= 32) + { + ret.HighPart = x.LowPart << (b & 31); + ret.LowPart = 0; + } + else + { + ret.HighPart = (x.LowPart >> (32 - b)) | (x.HighPart << b); + ret.LowPart = x.LowPart << b; + } + return ret.QuadPart; } /****************************************************************************** @@ -828,7 +841,20 @@ __ASM_GLOBAL_FUNC( _allshl, LONGLONG __stdcall __regs__allshr( LONGLONG a, unsigned char b ) { - return a >> b; + const LARGE_INTEGER x = { .QuadPart = a }; + LARGE_INTEGER ret; + + if (b >= 32) + { + ret.HighPart = x.HighPart >> 31; + ret.LowPart = x.HighPart >> (b & 31); + } + else + { + ret.HighPart = x.HighPart >> b; + ret.LowPart = (x.HighPart << (32 - b)) | (x.LowPart >> b); + } + return ret.QuadPart; } /****************************************************************************** @@ -844,7 +870,20 @@ __ASM_GLOBAL_FUNC( _allshr, ULONGLONG __stdcall __regs__aullshr( ULONGLONG a, unsigned char b ) { - return a >> b; + const ULARGE_INTEGER x = { .QuadPart = a }; + ULARGE_INTEGER ret; + + if (b >= 32) + { + ret.HighPart = 0; + ret.LowPart = x.HighPart >> (b & 31); + } + else + { + ret.HighPart = x.HighPart >> b; + ret.LowPart = (x.HighPart << (32 - b)) | (x.LowPart >> b); + } + return ret.QuadPart; } /****************************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/375
From: Zebediah Figura <zfigura(a)codeweavers.com> Based on a patch by Zhao Yi. --- dlls/ntdll/large_int.c | 15 ++++++++++++--- dlls/ntdll/tests/large_int.c | 10 +++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/large_int.c b/dlls/ntdll/large_int.c index 038a552545f..0973888d654 100644 --- a/dlls/ntdll/large_int.c +++ b/dlls/ntdll/large_int.c @@ -817,7 +817,10 @@ LONGLONG __stdcall __regs__allshl( LONGLONG a, unsigned char b ) if (b >= 32) { - ret.HighPart = x.LowPart << (b & 31); + if (b >= 64) + ret.HighPart = 0; + else + ret.HighPart = x.LowPart << (b & 31); ret.LowPart = 0; } else @@ -847,7 +850,10 @@ LONGLONG __stdcall __regs__allshr( LONGLONG a, unsigned char b ) if (b >= 32) { ret.HighPart = x.HighPart >> 31; - ret.LowPart = x.HighPart >> (b & 31); + if (b >= 64) + ret.LowPart = x.HighPart >> 31; + else + ret.LowPart = x.HighPart >> (b & 31); } else { @@ -876,7 +882,10 @@ ULONGLONG __stdcall __regs__aullshr( ULONGLONG a, unsigned char b ) if (b >= 32) { ret.HighPart = 0; - ret.LowPart = x.HighPart >> (b & 31); + if (b >= 64) + ret.LowPart = 0; + else + ret.LowPart = x.HighPart >> (b & 31); } else { diff --git a/dlls/ntdll/tests/large_int.c b/dlls/ntdll/tests/large_int.c index 747aa9ea8d2..a31100f6874 100644 --- a/dlls/ntdll/tests/large_int.c +++ b/dlls/ntdll/tests/large_int.c @@ -526,10 +526,10 @@ static void test_builtins(void) ok(l == 0xbcdef00000000000ll, "got %#I64x\n", l); l = call_shift_func(p_allshl, 0x0123456789abcdefll, 88); - todo_wine ok(!l, "got %#I64x\n", l); + ok(!l, "got %#I64x\n", l); l = call_shift_func(p_allshl, 0x0123456789abcdefll, 0x88); - todo_wine ok(!l, "got %#I64x\n", l); + ok(!l, "got %#I64x\n", l); l = call_shift_func(p_allshl, 0x0123456789abcdefll, 0x108); ok(l == 0x23456789abcdef00ll, "got %#I64x\n", l); @@ -541,7 +541,7 @@ static void test_builtins(void) ok(l == 0x01234ll, "got %#I64x\n", l); l = call_shift_func(p_allshr, 0x0123456789abcdefll, 88); - todo_wine ok(!l, "got %#I64x\n", l); + ok(!l, "got %#I64x\n", l); l = call_shift_func(p_allshr, 0x8123456789abcdefll, 12); ok(l == 0xfff8123456789abcll, "got %#I64x\n", l); @@ -550,7 +550,7 @@ static void test_builtins(void) ok(l == 0xfffffffffff81234ll, "got %#I64x\n", l); l = call_shift_func(p_allshr, 0x8123456789abcdefll, 88); - todo_wine ok(l == -1ll, "got %#I64x\n", l); + ok(l == -1ll, "got %#I64x\n", l); l = call_shift_func(p_allshr, 0x8123456789abcdefll, 0x108); ok(l == 0xff8123456789abcdll, "got %#I64x\n", l); @@ -562,7 +562,7 @@ static void test_builtins(void) ok(l == 0x81234ll, "got %#I64x\n", l); l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 88); - todo_wine ok(!l, "got %#I64x\n", l); + ok(!l, "got %#I64x\n", l); l = call_shift_func(p_aullshr, 0x8123456789abcdefll, 0x108); ok(l == 0x8123456789abcdll, "got %#I64x\n", l); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/375
Jinoh Kang (@iamahuman) commented about dlls/ntdll/tests/large_int.c:
LONGLONG l;
+ static const BYTE call_shift_code[] = + { + 0x55, /* pushl %ebp */ + 0x89, 0xe5, /* movl %esp,%ebp */ + 0x31, 0xc0, /* xorl %eax,%eax */ + 0x31, 0xd2, /* xorl %edx,%edx */ + 0x31, 0xc9, /* xorl %ecx,%ecx */ + 0x87, 0x45, 0x0c, /* xchgl 12(%ebp),%eax */ + 0x87, 0x55, 0x10, /* xchgl 16(%ebp),%edx */ + 0x87, 0x4d, 0x14, /* xchgl 20(%ebp),%ecx */ + 0xff, 0x55, 0x08, /* call *8(%ebp) */ + 0x39, 0xe5, /* cmpl %esp,%ebp */ + 0x75, 0x05, /* jne 1f */ + 0xb9, 0xef, 0xbe, 0xad, 0xde, /* movl $0xdeadbeef,%ecx */
This won't affect the test outcome. How about: ```suggestion:-1+0 0x74, 0x05, /* je 1f */ 0xb8, 0xef, 0xbe, 0xad, 0xde, /* movl $0xdeadbeef,%eax */ ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/375#note_3529
Thanks a lot! I've left my final comment on the patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/375#note_3530
participants (3)
-
Jinoh Kang (＠iamahuman) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)