[PATCH v4 0/1] MR10036: winemac.drv: Unconditionally use CreateIconFromResourceEx for app icons.
Sidesteps an issue cause by some apps having an inaccurate size for the resource in the GRPICONDIR size ; CreateIconFromResourceEx handles that case, but the old CGImage path would fault if the GRPICONDIR size was too large. --- It seems that some apps have different sizes in the GRPICONDIR and the IMAGE_RESOURCE_DATA_ENTRY. winemenubuilder has had a workaround for this since 2014 - see c205e6800a63a5df9960d8484a2e67687d53bc50. The size difference is great enough in some situations (e.g. Brotato) that the attempt read the resource may fault, but that's only because we passed the PNG data over to the Unix side of winemac.drv directly, and used CG methods to deal with it, which will attempt to copy in the entire array of bytes. If we use CreateIconFromResourceEx instead, for all cases (it can handle pngs), we do not fault, since that method does not attempt to copy the entire resource size. The original mailing list discussion about the winemenubuilder patch is [here](https://marc.info/?l=wine-patches&m=140903555724711&w=2). Huge thanks to @bshanks for spotting that patch. This has apparently always been faulting for some apps, but only became an issue after !10032. The subsequent syscall fault unwinds out of the pthread_once that !10032 added, so future threads that wind up there will hang. -- v4: winemac.drv: Unconditionally use CreateIconFromResourceEx for app icons. https://gitlab.winehq.org/wine/wine/-/merge_requests/10036
From: Tim Clem <tclem@codeweavers.com> Sidesteps an issue cause by some apps having an inaccurate size for the resource in the GRPICONDIR size ; CreateIconFromResourceEx handles that case, but the old CGImage path would fault if the GRPICONDIR size was too large. --- dlls/winemac.drv/dllmain.c | 23 ++++++----------------- dlls/winemac.drv/image.c | 25 +++---------------------- dlls/winemac.drv/unixlib.h | 2 -- 3 files changed, 9 insertions(+), 41 deletions(-) diff --git a/dlls/winemac.drv/dllmain.c b/dlls/winemac.drv/dllmain.c index 7ece4dc99a3..67ced51bb6b 100644 --- a/dlls/winemac.drv/dllmain.c +++ b/dlls/winemac.drv/dllmain.c @@ -334,31 +334,20 @@ static NTSTATUS WINAPI macdrv_app_icon(void *arg, ULONG size) icon_bits = LockResource(icon_res_data); if (icon_bits) { - static const BYTE png_magic[] = { 0x89, 0x50, 0x4e, 0x47 }; + HICON icon; entry->width = width; entry->height = height; - entry->size = icon_dir->idEntries[i].dwBytesInRes; - if (!memcmp(icon_bits, png_magic, sizeof(png_magic))) + icon = CreateIconFromResourceEx(icon_bits, icon_dir->idEntries[i].dwBytesInRes, + TRUE, 0x00030000, width, height, 0); + if (icon) { - entry->png = (UINT_PTR)icon_bits; - entry->icon = 0; + entry->icon = HandleToUlong(icon); count++; } else - { - HICON icon = CreateIconFromResourceEx(icon_bits, icon_dir->idEntries[i].dwBytesInRes, - TRUE, 0x00030000, width, height, 0); - if (icon) - { - entry->icon = HandleToUlong(icon); - entry->png = 0; - count++; - } - else - WARN("failed to create icon %d from resource with ID %hd\n", i, icon_dir->idEntries[i].nID); - } + WARN("failed to create icon %d from resource with ID %hd\n", i, icon_dir->idEntries[i].nID); } else WARN("failed to lock RT_ICON resource %d with ID %hd\n", i, icon_dir->idEntries[i].nID); diff --git a/dlls/winemac.drv/image.c b/dlls/winemac.drv/image.c index eb5939ce438..cf927ce7d1a 100644 --- a/dlls/winemac.drv/image.c +++ b/dlls/winemac.drv/image.c @@ -278,28 +278,9 @@ CFArrayRef create_app_icon_images(void) { struct app_icon_entry *icon = &entries[i]; CGImageRef cgimage = NULL; - - if (icon->png) - { - CFDataRef data = CFDataCreate(NULL, param_ptr(icon->png), icon->size); - if (data) - { - CGDataProviderRef provider = CGDataProviderCreateWithCFData(data); - CFRelease(data); - if (provider) - { - cgimage = CGImageCreateWithPNGDataProvider(provider, NULL, FALSE, - kCGRenderingIntentDefault); - CGDataProviderRelease(provider); - } - } - } - else - { - HICON handle = UlongToHandle(icon->icon); - cgimage = create_cgimage_from_icon(handle, icon->width, icon->height); - NtUserDestroyCursor(handle, 0); - } + HICON handle = UlongToHandle(icon->icon); + cgimage = create_cgimage_from_icon(handle, icon->width, icon->height); + NtUserDestroyCursor(handle, 0); if (cgimage) { diff --git a/dlls/winemac.drv/unixlib.h b/dlls/winemac.drv/unixlib.h index d39b3ea5b83..dc22da02d20 100644 --- a/dlls/winemac.drv/unixlib.h +++ b/dlls/winemac.drv/unixlib.h @@ -54,9 +54,7 @@ { UINT32 width; UINT32 height; - UINT32 size; UINT32 icon; - UINT64 png; }; /* macdrv_app_quit_request params */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10036
On Thu Feb 5 21:56:41 2026 +0000, Tim Clem wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/10036/diffs?diff_id=242412&start_sha=b75239139fa3cecdfe3461808711c23ed2176edb#be8b5f4a9db563a69a7a00b5ab48dfe1acb10bef_341_341) Right you are. Thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10036#note_128923
On Thu Feb 5 21:49:29 2026 +0000, Brendan Shanks wrote:
I guess I'll defer to whichever way Alexandre wants to go, but even if it works it seems like a bad idea to knowingly give an incorrect length to `CreateIconFromResourceEx` (`dwResSize` is documented as "The size, in bytes, of the set of bits pointed to by the pbIconBits parameter.") Yes, it's the max accessible size, so using `SizeofResource()` would be more correct.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10036#note_128924
participants (3)
-
Alexandre Julliard (＠julliard) -
Tim Clem
-
Tim Clem (＠tclem)