[PATCH 0/3] MR375: ntdll: Fixes for runtime 64-bit shift functions.
Based on <https://source.winehq.org/patches/data/238821> by Zhao Yi. -- 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..0faabe0a991 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 __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_NAME("__regs__allshl") ) + + +LONGLONG __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_NAME("__regs__allshr") ) + + +ULONGLONG __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_NAME("__regs__aullshr") ) + + /****************************************************************************** * _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 8ae4bc1f5a9..40e21222aa0 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 | 72 ++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/dlls/ntdll/tests/large_int.c b/dlls/ntdll/tests/large_int.c index 9635cac24ad..a8f628ae989 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,24 @@ 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[] = + { + 0x31, 0xc0, /* xorl %eax,%eax */ + 0x31, 0xd2, /* xorl %edx,%edx */ + 0x31, 0xc9, /* xorl %ecx,%ecx */ + 0x87, 0x44, 0x24, 0x08, /* xchgl 8(%esp),%eax */ + 0x87, 0x54, 0x24, 0x0c, /* xchgl 12(%esp),%edx */ + 0x87, 0x4c, 0x24, 0x10, /* xchgl 16(%esp),%ecx */ + 0xff, 0x64, 0x24, 0x04, /* jmp *4(%esp) */ + }; + 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 +508,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> Based on a patch by Zhao Yi. --- dlls/ntdll/large_int.c | 7 +++++++ dlls/ntdll/tests/large_int.c | 10 +++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/large_int.c b/dlls/ntdll/large_int.c index 0faabe0a991..c6f9d64884e 100644 --- a/dlls/ntdll/large_int.c +++ b/dlls/ntdll/large_int.c @@ -20,6 +20,7 @@ */ #include <stdarg.h> +#include <limits.h> #include "ntstatus.h" #define WIN32_NO_STATUS @@ -812,6 +813,8 @@ ULONGLONG WINAPI _aulldiv( ULONGLONG a, ULONGLONG b ) LONGLONG __regs__allshl( LONGLONG a, unsigned char b ) { + if (b > sizeof(a) * CHAR_BIT) + return 0; return a << b; } @@ -828,6 +831,8 @@ __ASM_GLOBAL_FUNC( _allshl, LONGLONG __regs__allshr( LONGLONG a, unsigned char b ) { + if (b > sizeof(a) * CHAR_BIT) + return a >> ((sizeof(a) * CHAR_BIT) - 1); return a >> b; } @@ -844,6 +849,8 @@ __ASM_GLOBAL_FUNC( _allshr, ULONGLONG __regs__aullshr( ULONGLONG a, unsigned char b ) { + if (b > sizeof(a) * CHAR_BIT) + return 0; return a >> b; } diff --git a/dlls/ntdll/tests/large_int.c b/dlls/ntdll/tests/large_int.c index a8f628ae989..128c83299a9 100644 --- a/dlls/ntdll/tests/large_int.c +++ b/dlls/ntdll/tests/large_int.c @@ -519,10 +519,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); @@ -534,7 +534,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); @@ -543,7 +543,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); @@ -555,7 +555,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
Jacek Caban (@jacek) commented about dlls/ntdll/large_int.c:
return udivmod(a, b, NULL); }
+ +LONGLONG __regs__allshl( LONGLONG a, unsigned char b ) +{ + return a << b;
It's a preexisting problem, but I think that doing this entirely in assembly would be safer. _allshl is an internal compiler function and a compiler may implement this statement by emitting _allshl function call here. GCC doesn't do that, because it emits calls to libgcc instead, but clang in msvc mode is free to do so. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/375#note_3324
On 7/5/22 05:28, Jacek Caban (@jacek) wrote:
Jacek Caban (@jacek) commented about dlls/ntdll/large_int.c:
return udivmod(a, b, NULL); }
+ +LONGLONG __regs__allshl( LONGLONG a, unsigned char b ) +{ + return a << b;
It's a preexisting problem, but I think that doing this entirely in assembly would be safer. _allshl is an internal compiler function and a compiler may implement this statement by emitting _allshl function call here. GCC doesn't do that, because it emits calls to libgcc instead, but clang in msvc mode is free to do so.
I've attempted to address this by adding a patch to use 32-bit arithmetic instead.
participants (4)
-
Jacek Caban (＠jacek) -
Zebediah Figura
-
Zebediah Figura -
Zebediah Figura (＠zfigura)