[PATCH v3 0/2] MR7237: include: Fix ReadNoFence64 on i386.
64-bit volatile accesses are not atomic on i386. Both i686-w64-mingw32-gcc and x86 MSVC splits 64-bit loads into a pair of load/store ops. Fix this by using a FILD/FISTP pair, which is also used to implement C11 atomics by i686-w64-mingw32-gcc. Fixes: f82b1c1fcf770a5d6fa02c3f286282be79a201b8 -- v3: include: Prevent misuse of __WINE_ATOMIC_* helper macros for non-atomic large accesses. include: Fix ReadNoFence64 on i386. https://gitlab.winehq.org/wine/wine/-/merge_requests/7237
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> 64-bit volatile accesses are not atomic on i386. Both i686-w64-mingw32-gcc and x86 MSVC splits 64-bit loads into a pair of load/store ops. Fix this by using a FILD/FISTP pair, which is also used to implement C11 atomics by i686-w64-mingw32-gcc. Fixes: f82b1c1fcf770a5d6fa02c3f286282be79a201b8 --- include/winnt.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/include/winnt.h b/include/winnt.h index 2547f841b7c..f93c27bb4a4 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -7122,7 +7122,15 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src ) { - LONG64 value = __WINE_LOAD64_NO_FENCE( (__int64 const volatile *)src ); + LONG64 value; +#if defined(__i386__) && _MSC_VER < 1700 + __asm { + fild qword ptr [src] + fistp qword ptr [value] + } +#else + value = __WINE_LOAD64_NO_FENCE( (__int64 const volatile *)src ); +#endif return value; } @@ -7315,7 +7323,11 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src ) { LONG64 value; +#ifdef __i386__ + __asm__ __volatile__( "fildq %1\n\tfistpq %0" : "=m" (value) : "m" (src) : "memory", "st" ); +#else __WINE_ATOMIC_LOAD_RELAXED( src, &value ); +#endif return value; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7237
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- include/winnt.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/winnt.h b/include/winnt.h index f93c27bb4a4..2057c4999d3 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -7295,10 +7295,10 @@ static FORCEINLINE void MemoryBarrier(void) #if defined(__x86_64__) || defined(__i386__) /* On x86, Support old GCC with either no or buggy (GCC BZ#81316) __atomic_* support */ -#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) do { *(ret) = *(ptr); __asm__ __volatile__( "" ::: "memory" ); } while (0) -#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) do { *(ret) = *(ptr); } while (0) -#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) do { __asm__ __volatile__( "" ::: "memory" ); *(ptr) = *(val); } while (0) -#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) do { *(ptr) = *(val); } while (0) +#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ret) = *(ptr); __asm__ __volatile__( "" ::: "memory" ); } while (0) +#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ret) = *(ptr); } while (0) +#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); __asm__ __volatile__( "" ::: "memory" ); *(ptr) = *(val); } while (0) +#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ptr) = *(val); } while (0) #else #define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_ACQUIRE) #define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_RELAXED) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7237
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=151178 Your paranoid android. === debian11 (build log) === ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" Task: The win32 Wine build failed === debian11b (build log) === ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" ../wine/include/winnt.h:405:21: error: static assertion failed: "sizeof(*(&__obj->seq)) <= sizeof(void *)" Task: The wow32 Wine build failed
v2: Fix instruction (thanks @Alcaro) v3: Reword commit message a bit -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7237#note_93366
participants (3)
-
Jinoh Kang -
Jinoh Kang (@iamahuman) -
Marvin