[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
Zebediah Figura <wine(a)gitlab.winehq.org> wrote:
--- 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)
According to the tests these APIs use stdcall calling convention, what is the reason to change them to cdecl in the spec? Existing spec entries also break that rule though... -- Dmitry.
On 7/7/22 15:51, Dmitry Timoshkov wrote:
Zebediah Figura <wine(a)gitlab.winehq.org> wrote:
--- 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)
According to the tests these APIs use stdcall calling convention,
The functions are neither cdecl nor stdcall. They use a custom calling convention where arguments are passed in the order EAX (low 32-bit), EDX (high 32-bit), and ECX (shift amount).
what is the reason to change them to cdecl in the spec? Existing spec entries also break that rule though...
Because the functions don't really "pop off" arguments from the stack. -- Sincerely, Jinoh Kang
Jinoh Kang <jinoh.kang.kr(a)gmail.com> wrote:
-@ 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)
According to the tests these APIs use stdcall calling convention,
The functions are neither cdecl nor stdcall. They use a custom calling convention where arguments are passed in the order EAX (low 32-bit), EDX (high 32-bit), and ECX (shift amount).
Thanks for pointing that out.
what is the reason to change them to cdecl in the spec? Existing spec entries also break that rule though...
Because the functions don't really "pop off" arguments from the stack.
I'd guess by the same logic they shouldn't be marked as cdecl either. However for -norelay entries that doesn't really matter. -- Dmitry.
On 7/7/22 07:37, Dmitry Timoshkov wrote:
Jinoh Kang <jinoh.kang.kr(a)gmail.com> wrote:
-@ 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)
According to the tests these APIs use stdcall calling convention,
The functions are neither cdecl nor stdcall. They use a custom calling convention where arguments are passed in the order EAX (low 32-bit), EDX (high 32-bit), and ECX (shift amount).
Thanks for pointing that out.
what is the reason to change them to cdecl in the spec? Existing spec entries also break that rule though...
Because the functions don't really "pop off" arguments from the stack.
I'd guess by the same logic they shouldn't be marked as cdecl either. However for -norelay entries that doesn't really matter.
I changed them to cdecl for consistency with the others.
Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 7/7/22 07:37, Dmitry Timoshkov wrote:
Jinoh Kang <jinoh.kang.kr(a)gmail.com> wrote:
-@ 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)
According to the tests these APIs use stdcall calling convention,
The functions are neither cdecl nor stdcall. They use a custom calling convention where arguments are passed in the order EAX (low 32-bit), EDX (high 32-bit), and ECX (shift amount).
Thanks for pointing that out.
what is the reason to change them to cdecl in the spec? Existing spec entries also break that rule though...
Because the functions don't really "pop off" arguments from the stack.
I'd guess by the same logic they shouldn't be marked as cdecl either. However for -norelay entries that doesn't really matter.
I changed them to cdecl for consistency with the others.
I should have guessed that might be the case. Thanks for the explanation. -- Dmitry.
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
On 7/7/22 06:58, Jinoh Kang (@iamahuman) wrote:
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 */ ```
Yes, that is what I meant to write; thanks for catching it. I've sent a new revision.
Thanks a lot! I've left my final comment on the patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/375#note_3530
participants (6)
-
Dmitry Timoshkov -
Jinoh Kang -
Jinoh Kang (＠iamahuman) -
Zebediah Figura
-
Zebediah Figura -
Zebediah Figura (＠zfigura)