[PATCH 0/3] MR1794: Improvements to ntdll.Rtl*ByteSwap().
- fixes effective calling convention used in test (causing some crashes #53536) - properly set entries in .spec files - only export these functions on 32bit - always inline them on 64bit -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1794
From: Eric Pouech <eric.pouech(a)gmail.com> The RtlU*ByteSwap() family: - has FASTCALL calling convention - is only exported from ntdll and ntoskrnl.exe in 32bit mode (didn't check ARM though) Wine's support for RtlUlonglongByteSwap() doesn't follow these constraints. Note: in __fastcall, 64bit paramaters are passed on the stack, to RtlUlonglongByteSwap() calling convention acts as __stdcall. So: - fix ntdll.spec (resp. ntoskrnl.exe.spec) to only export (resp. forward) RtlUlonglongByteSwap for i386 - always provide an inline implementation in winternl.h - reimplement ntdll.RtlUlonglongByteSwap() for i386 with __fastcall calling convention. - fix ntdll/tests/rtl.c to use correct calling convention. - add test in ntdll/tests/rtl.c for inlined version. Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=53536 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/ntdll/ntdll.spec | 2 +- dlls/ntdll/rtl.c | 12 ++++++++---- dlls/ntdll/tests/rtl.c | 24 ++++++++++++++---------- dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- include/winternl.h | 9 +++++++-- 5 files changed, 31 insertions(+), 18 deletions(-) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 1e6576a5422..45af5e8c66d 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1039,7 +1039,7 @@ @ stdcall RtlTryEnterCriticalSection(ptr) @ stdcall RtlUTF8ToUnicodeN(ptr long ptr ptr long) @ cdecl -i386 -norelay RtlUlongByteSwap() NTDLL_RtlUlongByteSwap -@ cdecl -ret64 RtlUlonglongByteSwap(int64) +@ stdcall -arch=i386 -norelay -ret64 RtlUlonglongByteSwap(int64) # @ stub RtlUnhandledExceptionFilter2 # @ stub RtlUnhandledExceptionFilter @ stdcall RtlUnicodeStringToAnsiSize(ptr) diff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c index 5ff6205fe05..579175592d3 100644 --- a/dlls/ntdll/rtl.c +++ b/dlls/ntdll/rtl.c @@ -641,10 +641,14 @@ DWORD WINAPI RtlComputeCrc32(DWORD dwInitial, const BYTE *pData, INT iLen) * RETURNS * The value with its bytes swapped. */ -ULONGLONG __cdecl RtlUlonglongByteSwap(ULONGLONG i) -{ - return ((ULONGLONG)RtlUlongByteSwap(i) << 32) | RtlUlongByteSwap(i>>32); -} +#ifdef __i386__ +__ASM_STDCALL_FUNC(RtlUlonglongByteSwap,8, + "movl 4(%esp),%edx\n\t" + "bswap %edx\n\t" + "movl 8(%esp),%eax\n\t" + "bswap %eax\n\t" + "ret $8") +#endif /************************************************************************* * RtlUlongByteSwap [NTDLL.@] diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index 2e0bfb650e4..8f698454957 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -68,7 +68,7 @@ static VOID (WINAPI *pRtlMoveMemory)(LPVOID,LPCVOID,SIZE_T); static VOID (WINAPI *pRtlFillMemory)(LPVOID,SIZE_T,BYTE); static VOID (WINAPI *pRtlFillMemoryUlong)(LPVOID,SIZE_T,ULONG); static VOID (WINAPI *pRtlZeroMemory)(LPVOID,SIZE_T); -static ULONGLONG (WINAPIV *pRtlUlonglongByteSwap)(ULONGLONG source); +static ULONGLONG (FASTCALL *pRtlUlonglongByteSwap)(ULONGLONG source); static DWORD (WINAPI *pRtlGetThreadErrorMode)(void); static NTSTATUS (WINAPI *pRtlSetThreadErrorMode)(DWORD, LPDWORD); static NTSTATUS (WINAPI *pRtlIpv4AddressToStringExA)(const IN_ADDR *, USHORT, LPSTR, PULONG); @@ -319,18 +319,22 @@ static void test_RtlZeroMemory(void) static void test_RtlUlonglongByteSwap(void) { - ULONGLONG result; + ULONGLONG llresult; - if ( !pRtlUlonglongByteSwap ) +#ifdef _WIN64 + /* the Rtl*ByteSwap() are always inlined and not exported from ntdll on 64bit */ + llresult = RtlUlonglongByteSwap( 0x7654321087654321ull ); + ok( 0x2143658710325476 == llresult, + "inlined RtlUlonglongByteSwap() returns %#I64x\n", llresult ); +#else + ok( pRtlUlonglongByteSwap != NULL, "RtlUlonglongByteSwap is not available\n"); + if ( pRtlUlonglongByteSwap ) { - win_skip("RtlUlonglongByteSwap is not available\n"); - return; + llresult = pRtlUlonglongByteSwap( 0x7654321087654321ull ); + ok( 0x2143658710325476ull == llresult, + "ntdll.RtlUlonglongByteSwap() returns %#I64x\n", llresult ); } - - result = pRtlUlonglongByteSwap( ((ULONGLONG)0x76543210 << 32) | 0x87654321 ); - ok( (((ULONGLONG)0x21436587 << 32) | 0x10325476) == result, - "RtlUlonglongByteSwap(0x7654321087654321) returns 0x%s, expected 0x2143658710325476\n", - wine_dbgstr_longlong(result)); +#endif } diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 0a484829ca3..ebcafe762e7 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -68,7 +68,7 @@ @ stdcall -fastcall ObfReferenceObject(ptr) @ stub RtlPrefetchMemoryNonTemporal @ cdecl -i386 -norelay RtlUlongByteSwap() -@ cdecl -ret64 RtlUlonglongByteSwap(int64) +@ stdcall -arch=i386 -norelay -ret64 RtlUlonglongByteSwap(int64) @ cdecl -i386 -norelay RtlUshortByteSwap() @ stub WmiGetClock @ stub Kei386EoiHelper diff --git a/include/winternl.h b/include/winternl.h index 5a99827fe65..4c54b5b7373 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4567,7 +4567,6 @@ NTSYSAPI BOOLEAN WINAPI RtlTryAcquireSRWLockExclusive(RTL_SRWLOCK *); NTSYSAPI BOOLEAN WINAPI RtlTryAcquireSRWLockShared(RTL_SRWLOCK *); NTSYSAPI BOOL WINAPI RtlTryEnterCriticalSection(RTL_CRITICAL_SECTION *); NTSYSAPI NTSTATUS WINAPI RtlUTF8ToUnicodeN(WCHAR*,DWORD,DWORD*,const char*,DWORD); -NTSYSAPI ULONGLONG __cdecl RtlUlonglongByteSwap(ULONGLONG); NTSYSAPI DWORD WINAPI RtlUnicodeStringToAnsiSize(const UNICODE_STRING*); NTSYSAPI NTSTATUS WINAPI RtlUnicodeStringToAnsiString(PANSI_STRING,PCUNICODE_STRING,BOOLEAN); NTSYSAPI NTSTATUS WINAPI RtlUnicodeStringToInteger(const UNICODE_STRING *,ULONG,ULONG *); @@ -4733,7 +4732,9 @@ static inline BOOLEAN RtlCheckBit(PCRTL_BITMAP lpBits, ULONG ulBit) return FALSE; } -/* These are implemented as __fastcall, so we can't let Winelib apps link with them */ +/* These are implemented as __fastcall, so we can't let Winelib apps link with them. + * Moreover, they're always inlined and not exported on 64bit systems. + */ static inline USHORT RtlUshortByteSwap(USHORT s) { return (s >> 8) | (s << 8); @@ -4748,6 +4749,10 @@ static inline ULONG RtlUlongByteSwap(ULONG i) return ((ULONG)RtlUshortByteSwap((USHORT)i) << 16) | RtlUshortByteSwap((USHORT)(i >> 16)); #endif } +static inline ULONGLONG RtlUlonglongByteSwap(ULONGLONG i) +{ + return ((ULONGLONG)RtlUlongByteSwap((ULONG)i) << 32) | RtlUlongByteSwap((ULONG)(i >> 32)); +} /* list manipulation macros */ #define InitializeListHead(le) (void)((le)->Flink = (le)->Blink = (le)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1794
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/ntdll/tests/rtl.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index 8f698454957..38e6f5a40e1 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -68,6 +68,8 @@ static VOID (WINAPI *pRtlMoveMemory)(LPVOID,LPCVOID,SIZE_T); static VOID (WINAPI *pRtlFillMemory)(LPVOID,SIZE_T,BYTE); static VOID (WINAPI *pRtlFillMemoryUlong)(LPVOID,SIZE_T,ULONG); static VOID (WINAPI *pRtlZeroMemory)(LPVOID,SIZE_T); +static USHORT (FASTCALL *pRtlUshortByteSwap)(USHORT source); +static ULONG (FASTCALL *pRtlUlongByteSwap)(ULONG source); static ULONGLONG (FASTCALL *pRtlUlonglongByteSwap)(ULONGLONG source); static DWORD (WINAPI *pRtlGetThreadErrorMode)(void); static NTSTATUS (WINAPI *pRtlSetThreadErrorMode)(DWORD, LPDWORD); @@ -108,7 +110,9 @@ static void InitFunctionPtrs(void) pRtlFillMemory = (void *)GetProcAddress(hntdll, "RtlFillMemory"); pRtlFillMemoryUlong = (void *)GetProcAddress(hntdll, "RtlFillMemoryUlong"); pRtlZeroMemory = (void *)GetProcAddress(hntdll, "RtlZeroMemory"); - pRtlUlonglongByteSwap = (void *)GetProcAddress(hntdll, "RtlUlonglongByteSwap"); + pRtlUshortByteSwap = (void *)GetProcAddress(hntdll, "RtlUshortByteSwap"); + pRtlUlongByteSwap = (void *)GetProcAddress(hntdll, "RtlUlongByteSwap"); + pRtlUlonglongByteSwap = (void *)GetProcAddress(hntdll, "RtlUlonglongByteSwap"); pRtlGetThreadErrorMode = (void *)GetProcAddress(hntdll, "RtlGetThreadErrorMode"); pRtlSetThreadErrorMode = (void *)GetProcAddress(hntdll, "RtlSetThreadErrorMode"); pRtlIpv4AddressToStringExA = (void *)GetProcAddress(hntdll, "RtlIpv4AddressToStringExA"); @@ -317,16 +321,40 @@ static void test_RtlZeroMemory(void) ZERO(9); MCMP("\0\0\0\0\0\0\0\0\0 test!"); } -static void test_RtlUlonglongByteSwap(void) +static void test_RtlByteSwap(void) { ULONGLONG llresult; + ULONG lresult; + USHORT sresult; #ifdef _WIN64 /* the Rtl*ByteSwap() are always inlined and not exported from ntdll on 64bit */ + sresult = RtlUshortByteSwap( 0x1234 ); + ok( 0x3412 == sresult, + "inlined RtlUshortByteSwap() returns 0x%x\n", sresult ); + lresult = RtlUlongByteSwap( 0x87654321 ); + ok( 0x21436587 == lresult, + "inlined RtlUlongByteSwap() returns 0x%lx\n", lresult ); llresult = RtlUlonglongByteSwap( 0x7654321087654321ull ); ok( 0x2143658710325476 == llresult, "inlined RtlUlonglongByteSwap() returns %#I64x\n", llresult ); #else + ok( pRtlUshortByteSwap != NULL, "RtlUshortByteSwap is not available\n" ); + if ( pRtlUshortByteSwap ) + { + sresult = pRtlUshortByteSwap( 0x1234u ); + ok( 0x3412u == sresult, + "ntdll.RtlUshortByteSwap() returns %#x\n", sresult ); + } + + ok( pRtlUlongByteSwap != NULL, "RtlUlongByteSwap is not available\n" ); + if ( pRtlUlongByteSwap ) + { + lresult = pRtlUlongByteSwap( 0x87654321ul ); + ok( 0x21436587ul == lresult, + "ntdll.RtlUlongByteSwap() returns %#lx\n", lresult ); + } + ok( pRtlUlonglongByteSwap != NULL, "RtlUlonglongByteSwap is not available\n"); if ( pRtlUlonglongByteSwap ) { @@ -3745,7 +3773,7 @@ START_TEST(rtl) test_RtlFillMemory(); test_RtlFillMemoryUlong(); test_RtlZeroMemory(); - test_RtlUlonglongByteSwap(); + test_RtlByteSwap(); test_RtlUniform(); test_RtlRandom(); test_RtlAreAllAccessesGranted(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1794
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/ntdll/ntdll.spec | 4 ++-- dlls/ntdll/rtl.c | 4 ++-- dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 45af5e8c66d..617f10a0b4b 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1038,7 +1038,7 @@ @ stdcall RtlTryAcquireSRWLockShared(ptr) @ stdcall RtlTryEnterCriticalSection(ptr) @ stdcall RtlUTF8ToUnicodeN(ptr long ptr ptr long) -@ cdecl -i386 -norelay RtlUlongByteSwap() NTDLL_RtlUlongByteSwap +@ stdcall -fastcall -arch=i386 -norelay RtlUlongByteSwap(long) @ stdcall -arch=i386 -norelay -ret64 RtlUlonglongByteSwap(int64) # @ stub RtlUnhandledExceptionFilter2 # @ stub RtlUnhandledExceptionFilter @@ -1072,7 +1072,7 @@ @ stdcall RtlUpperString(ptr ptr) @ stub RtlUsageHeap @ stdcall -norelay RtlUserThreadStart(ptr ptr) -@ cdecl -i386 -norelay RtlUshortByteSwap() NTDLL_RtlUshortByteSwap +@ stdcall -fastcall -arch=i386 -norelay RtlUshortByteSwap(long) @ stdcall RtlValidAcl(ptr) @ stdcall RtlValidRelativeSecurityDescriptor(ptr long long) @ stdcall RtlValidSecurityDescriptor(ptr) diff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c index 579175592d3..1e6f6345ecc 100644 --- a/dlls/ntdll/rtl.c +++ b/dlls/ntdll/rtl.c @@ -659,7 +659,7 @@ __ASM_STDCALL_FUNC(RtlUlonglongByteSwap,8, * ix86 version takes argument in %ecx. Other systems use the inline version. */ #ifdef __i386__ -__ASM_GLOBAL_FUNC(NTDLL_RtlUlongByteSwap, +__ASM_FASTCALL_FUNC(RtlUlongByteSwap,4, "movl %ecx,%eax\n\t" "bswap %eax\n\t" "ret") @@ -674,7 +674,7 @@ __ASM_GLOBAL_FUNC(NTDLL_RtlUlongByteSwap, * i386 version takes argument in %cx. Other systems use the inline version. */ #ifdef __i386__ -__ASM_GLOBAL_FUNC(NTDLL_RtlUshortByteSwap, +__ASM_FASTCALL_FUNC(RtlUshortByteSwap,4, "movb %ch,%al\n\t" "movb %cl,%ah\n\t" "ret") diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index ebcafe762e7..0aeada57caa 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -67,9 +67,9 @@ @ stdcall -fastcall ObfDereferenceObject(ptr) @ stdcall -fastcall ObfReferenceObject(ptr) @ stub RtlPrefetchMemoryNonTemporal -@ cdecl -i386 -norelay RtlUlongByteSwap() +@ stdcall -fastcall -arch=i386 -norelay RtlUlongByteSwap(long) @ stdcall -arch=i386 -norelay -ret64 RtlUlonglongByteSwap(int64) -@ cdecl -i386 -norelay RtlUshortByteSwap() +@ stdcall -fastcall -arch=i386 -norelay RtlUshortByteSwap(long) @ stub WmiGetClock @ stub Kei386EoiHelper @ stub Kii386SpinOnSpinLock -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1794
participants (2)
-
Eric Pouech -
eric pouech (＠epo)