June 2022 - Wine-Devel - list.winehq.org

[PATCH v3 0/2] MR155: ntdll: Fix reading stale Information from IOSB.

by Jinoh Kang (＠iamahuman)

Enforce proper atomic update so that other threads do not read stale data from IO_STATUS_BLOCK. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> -- v3: ntdll: Fix reading stale Information from IOSB. include: Define atomic read/write helpers for 32-bit integers. https://gitlab.winehq.org/wine/wine/-/merge_requests/155

June 4, 2022

[PATCH] include: Define atomic read/write helpers for 32-bit integers.

by Jinoh Kang

Based on the corresponding functions from Windows SDK header files. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- include/winnt.h | 121 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/include/winnt.h b/include/winnt.h index 87c4b4da92d..a98201dc3b0 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -6475,6 +6475,70 @@ static FORCEINLINE void MemoryBarrier(void) #endif /* __i386__ */ +#if defined(__i386__) || defined(__x86_64__) + +static FORCEINLINE LONG ReadAcquire( LONG const volatile *src ) +{ + LONG value = *src; + return value; +} + +static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) +{ + LONG value = *src; + return value; +} + +static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) +{ + *dest = value; +} + +static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value ) +{ + *dest = value; +} + +#else + +/* __iso_volatile_* intrinsics do not need explicit declarations. */ +#pragma intrinsic(__iso_volatile_load32) +#pragma intrinsic(__iso_volatile_store32) + +#if defined(__arm__) +#define WINE_MEMORY_BARRIER_ACQUIRE() __dmb(_ARM_BARRIER_ISH) +#define WINE_MEMORY_BARRIER_RELEASE() __dmb(_ARM_BARRIER_ISH) +#elif defined(__aarch64__) +#define WINE_MEMORY_BARRIER_ACQUIRE() __dmb(_ARM64_BARRIER_ISH) +#define WINE_MEMORY_BARRIER_RELEASE() __dmb(_ARM64_BARRIER_ISH) +#endif /* defined(__arm__) */ + +static FORCEINLINE LONG ReadAcquire( LONG const volatile *src ) +{ + LONG value = __iso_volatile_load32( (int const volatile *)src ); + WINE_MEMORY_BARRIER_ACQUIRE(); + return value; +} + +static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) +{ + LONG value = __iso_volatile_load32( (int const volatile *)src ); + return value; +} + +static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) +{ + WINE_MEMORY_BARRIER_RELEASE(); + __iso_volatile_store32( (int volatile *)dest, value ); +} + +static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value ) +{ + __iso_volatile_store32( (int volatile *)dest, value ); +} + +#endif /* defined(__i386__) || defined(__x86_64__) */ + #elif defined(__GNUC__) static FORCEINLINE BOOLEAN WINAPI BitScanForward(DWORD *index, DWORD mask) @@ -6588,6 +6652,43 @@ static FORCEINLINE void MemoryBarrier(void) __sync_synchronize(); } +#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) +#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) +#define WINE_ATOMIC_STORE_RELEASE(ptr, val) __atomic_store(ptr, val, __ATOMIC_RELEASE) +#define WINE_ATOMIC_STORE_RELAXED(ptr, val) __atomic_store(ptr, val, __ATOMIC_RELAXED) +#endif /* defined(__x86_64__) || defined(__i386__) */ + +static FORCEINLINE LONG ReadAcquire( LONG const volatile *src ) +{ + LONG value; + WINE_ATOMIC_LOAD_ACQUIRE( src, &value ); + return value; +} + +static FORCEINLINE LONG ReadNoFence( LONG const volatile *src ) +{ + LONG value; + WINE_ATOMIC_LOAD_RELAXED( src, &value ); + return value; +} + +static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value ) +{ + WINE_ATOMIC_STORE_RELEASE( dest, &value ); +} + +static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value ) +{ + WINE_ATOMIC_STORE_RELAXED( dest, &value ); +} + static FORCEINLINE DECLSPEC_NORETURN void __fastfail(unsigned int code) { #if defined(__x86_64__) || defined(__i386__) @@ -6603,6 +6704,26 @@ static FORCEINLINE DECLSPEC_NORETURN void __fastfail(unsigned int code) #endif /* __GNUC__ */ +static FORCEINLINE ULONG ReadULongAcquire( ULONG const volatile *src ) +{ + return (ULONG)ReadAcquire( (LONG const volatile *)src ); +} + +static FORCEINLINE ULONG ReadULongNoFence( ULONG const volatile *src ) +{ + return (ULONG)ReadNoFence( (LONG const volatile *)src ); +} + +static FORCEINLINE void WriteULongRelease( ULONG volatile *dest, ULONG value ) +{ + WriteRelease( (LONG volatile *)dest, value ); +} + +static FORCEINLINE void WriteULongNoFence( ULONG volatile *dest, ULONG value ) +{ + WriteNoFence( (LONG volatile *)dest, value ); +} + #ifdef _WIN64 #define InterlockedCompareExchange128 _InterlockedCompareExchange128 -- 2.34.3

June 4, 2022

[PATCH 1/1] secur32/schannel: Avoid use-after-free in AcquireClientCredentials() (Coverity).

by Nikolay Sivov

From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/secur32/schannel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c index bc0867ca94b..5b4fb196aca 100644 --- a/dlls/secur32/schannel.c +++ b/dlls/secur32/schannel.c @@ -589,8 +589,9 @@ static SECURITY_STATUS schan_AcquireClientCredentials(const void *schanCred, } params.key_size = key_size; params.key_blob = key_blob; - if (GNUTLS_CALL( allocate_certificate_credentials, &params )) goto fail; + status = GNUTLS_CALL( allocate_certificate_credentials, &params ); free(key_blob); + if (status) goto fail; handle = schan_alloc_handle(creds, SCHAN_HANDLE_CRED); if (handle == SCHAN_INVALID_HANDLE) goto fail; @@ -609,7 +610,6 @@ static SECURITY_STATUS schan_AcquireClientCredentials(const void *schanCred, fail: free(creds); - free(key_blob); return SEC_E_INTERNAL_ERROR; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/186

June 4, 2022

[PATCH 0/1] MR186: secur32/schannel: Avoid use-after-free in AcquireClientCredentials() (Coverity).

by Nikolay Sivov (＠nsivov)

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/186

June 4, 2022

Re: [PATCH v3 0/2] MR135: Cancel asyncs queued by thread when the thread is terminated

by Paul Gofman

>> > > I don't understand what you mean by this. Sending SIGUSR1 doesn't mean that the async isn't queued; it's still put into the system_apc list. SIGUSR1 doesn't carry the async as a payload or anything, it's just a way to force the thread to check its own queue. Am I missing something here? > Oops, sorry… no, it is me missing an obvious fact which I knew for years, something made me see the problem where there is not. I will update the patch.

June 4, 2022

[PATCH 1/1] d2d1: Use d3d11 constants for constant buffer description.

by Nikolay Sivov

From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d2d1/device.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/d2d1/device.c b/dlls/d2d1/device.c index 12aa2acc331..8f71a85a3f6 100644 --- a/dlls/d2d1/device.c +++ b/dlls/d2d1/device.c @@ -4011,8 +4011,8 @@ static HRESULT d2d_device_context_init(struct d2d_device_context *render_target, buffer_desc.ByteWidth = sizeof(struct d2d_vs_cb); buffer_desc.Usage = D3D11_USAGE_DYNAMIC; - buffer_desc.BindFlags = D3D10_BIND_CONSTANT_BUFFER; - buffer_desc.CPUAccessFlags = D3D10_CPU_ACCESS_WRITE; + buffer_desc.BindFlags = D3D11_BIND_CONSTANT_BUFFER; + buffer_desc.CPUAccessFlags = D3D11_CPU_ACCESS_WRITE; buffer_desc.MiscFlags = 0; if (FAILED(hr = ID3D11Device1_CreateBuffer(render_target->d3d_device, &buffer_desc, NULL, @@ -4031,8 +4031,8 @@ static HRESULT d2d_device_context_init(struct d2d_device_context *render_target, buffer_desc.ByteWidth = sizeof(struct d2d_ps_cb); buffer_desc.Usage = D3D11_USAGE_DYNAMIC; - buffer_desc.BindFlags = D3D10_BIND_CONSTANT_BUFFER; - buffer_desc.CPUAccessFlags = D3D10_CPU_ACCESS_WRITE; + buffer_desc.BindFlags = D3D11_BIND_CONSTANT_BUFFER; + buffer_desc.CPUAccessFlags = D3D11_CPU_ACCESS_WRITE; buffer_desc.MiscFlags = 0; if (FAILED(hr = ID3D11Device1_CreateBuffer(render_target->d3d_device, &buffer_desc, NULL, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/185

June 4, 2022

[PATCH 0/1] MR185: d2d1: Use d3d11 constants for constant buffer description.

by Nikolay Sivov (＠nsivov)

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/185

June 4, 2022

Re: [PATCH v3] d2d1: Make ID2D1Device::CreateImageBrush() accept only bitmap as a source image.

by Nikolay Sivov

On 6/3/22 15:40, Dmitry Timoshkov wrote: > Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >> On 5/26/22 17:36, Dmitry Timoshkov wrote: >>> Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >>> >>>> On 5/26/22 17:15, Dmitry Timoshkov wrote: >>>>> Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >>>>> >>>>>> On 5/26/22 14:34, Dmitry Timoshkov wrote: >>>>>>> Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >>>>>>> >>>>>>>> On 5/25/22 18:58, Dmitry Timoshkov wrote: >>>>>>>>> Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: >>>>>>>>> >>>>>>>>>> ID2D1Bitmap derives from ID2D1Image, which in turn derives from ID2D1Resource. >>>>>>>>>> That means that ID2D1Device::CreateImageBrush() can't be really passed anything >>>>>>>>>> but a ID2D1Bitmap* represented as a ID2D1Image*. >>>>>>>>>> >>>>>>>>>> I've added QueryInterface+FIXME just in case, probably it could be dropped. >>>>>>>>>> >>>>>>>>>> v2: Fix test crashes with image == NULL. >>>>>>>>>> v3: Add a QueryInterface() check to SetImage(). >>>>>>>>> Is there anything that could be improved in this patch to make it acceptable? >>>>>>>>> Probably I should add once again, that this patch allows a bitmap work as an >>>>>>>>> image brush (unlike current broken state) and actually makes work the app that >>>>>>>>> have here. >>>>>>>>> >>>>>>>> Same thing I mention last time - it's backwards. Bitmap brush could >>>>>>>> probably be implemented as image brush, but not the other way. I don't >>>>>>>> think you'll need any shader changes for that. >>>>>>> Do you mean something like the attached patch, or do I miss something? >>>>>>> With the attached patch (on top of just sent ::Map()/::Unmap() patches) >>>>>>> I get areas filled with black in the application that I'm working on, >>>>>>> unlike with the proposed patch where I get correctly filled areas with >>>>>>> a proper brush. >>>>>>> >>>>>> I suspect black color is from this in sample_brush(): >>>>>> >>>>>> return float4(0.0, 0.0, 0.0, brush.opacity); >>>>>> >>>>>> Which means you didn't set brush type correctly in the constant buffer. >>>>> I also think that's the source of the problem. However that's the shader >>>>> code, and you mentioned above that I don't need them to make image brush >>>>> work. Or did I misunderstand something? Do you have a suggestion for that? >>>>> >>>> Brush type is set in d2d_brush_fill_cb(), trying setting it "correctly" >>>> to bitmap brush type. >>> Thank you very much, setting cb->type to TYPE_BITMAP in the TYPE_IMAGE case >>> has fixed painting in my application. Do you think that the patch I attached >>> in my previous reply with this fix is an acceptable solution, or there are >>> other things you'd like to see addressed? >>> >> I think we should avoid duplication of bind_bitmap() function, since >> it's exactly the same. Probably passing image brush description there, >> which could be filled from bitmap brush description, expect for source >> rectangle. > Thanks for the helpful pointers. Does the attached patch match your suggestions? > The patch works prety well with the application that I have here. > Yes, along those lines. > +static void d2d_brush_bind_bitmap(struct d2d_brush *brush, struct > d2d_device_context *context, > + unsigned int brush_idx) > +{ > + D2D1_IMAGE_BRUSH_PROPERTIES image_brush_desc; > + > + image_brush_desc.sourceRectangle.left = 0.0f; > + image_brush_desc.sourceRectangle.top = 0.0f; > + image_brush_desc.sourceRectangle.right = > brush->u.bitmap.bitmap->pixel_size.width; > + image_brush_desc.sourceRectangle.bottom = > brush->u.bitmap.bitmap->pixel_size.height; This needs a test, so we don't assume coordinate system here. We have tests for filling with 4x4 bitmap brush, it will be a matter of creating image brush with same bitmap, and rectangle as (0,0-4,4) vs (0,0-1,1). If that shows no difference it means this is using normalized coordinates, and we should initialize it here as (0,0-1,1). > @@ -1043,7 +1043,7 @@ static void STDMETHODCALLTYPE > d2d_device_context_FillGeometry(ID2D1DeviceContext > if (FAILED(context->error.code)) > return; > > - if (opacity_brush && brush_impl->type != D2D_BRUSH_TYPE_BITMAP) > + if (opacity_brush && !(brush_impl->type == D2D_BRUSH_TYPE_BITMAP > || brush_impl->type == D2D_BRUSH_TYPE_IMAGE)) > { > d2d_device_context_set_error(context, > D2DERR_INCOMPATIBLE_BRUSH_TYPES); > return; Same here, we have a test for this case already, that needs to be extended to verify this change.

June 4, 2022

Re: [PATCH v3 0/2] MR135: Cancel asyncs queued by thread when the thread is terminated

by Zebediah Figura

On 6/3/22 20:46, Paul Gofman wrote: >> * If a thread dies naturally (terminates itself), we can cancel its >> asyncs from the terminate_thread handler, and then, in the client side >> but before calling exit_thread(), do a zero-length wait with >> SELECT_INTERRUPTIBLE, exactly as in v1 of this patch set. My >> understanding is that these alone are actually sufficient to ensure >> that any APC_ASYNC_IO resulting from the async termination are >> executed by the thread itself before it frees its stack. I.e. we don't >> need any of the "queue_only" bits that were in v1. This holds because: >> >> - terminate_thread doesn't set the state to TERMINATED yet in the >> "self" case; > > Yes. > > >> >> - as a result queue_apc() will queue the (system) APC to that >> specific thread; > > It will not put it to the queue, it will send SIGUSR1. As (without the > long and probably ugly part which was in v1 of the patch) it puts the > APC in queue only if the thread is waiting in server select or already > has some APC in queue. The other way (probably not super nice as well) > would be to put APC_NONE to the thread system APC queue, then the logic > in queue_apc() would queue our async cancels to the queue as well. I don't understand what you mean by this. Sending SIGUSR1 doesn't mean that the async isn't queued; it's still put into the system_apc list. SIGUSR1 doesn't carry the async as a payload or anything, it's just a way to force the thread to check its own queue. Am I missing something here? The way I see it, the exiting thread will select manually, and it'll also be sent SIGUSR1. If SIGUSR1 comes first, that'll trigger a select, and the system APC(s) will be consumed. If the manual select comes first, the same thing happens, and then it doesn't matter whether the SIGUSR1 is received or if it comes too late and gets masked off.

June 4, 2022

Wine staging 7.10 release

by Alistair Leslie-Hughes

Binary packages for various distributions will be available from: https://www.winehq.org/download Summary since last release * Rebased to current wine 7.10 (545 patches are applied to wine vanilla) Upstreamed (Either directly from staging or fixed with a similar patch). * kernelbase: Implement sortkey generation on official tables * kernelbase: Implement sortkey punctuation * kernelbase: Implement sortkey for Japanese characters * kernelbase: Implement sortkey expansion * kernelbase: Implement sortkey language support * kernelbase: Implement CompareString functions Added: * [#53038] - dwmapi: Supply some defaults values for DwmGetCompositionTimingInfo. * [#53035] - dwmapi: DwmFlush return S_OK. Updated: * None Where can you help * Run Steam/Battle.net/GOG/UPlay/Epic * Test your favorite game. * Test your favorite applications. * Improve staging patches and get them accepted upstream. As always, if you find a bug, please report it via https://bugs.winehq.org Best Regards Alistair.

June 4, 2022