[PATCH v2 0/2] MR920: kernel32: Map system heap anywhere if the standard address is unavailable.
`mscomctl.ocx` (part of the VB6 common controls) calls `HeapCreate(0x04000000,1,0)`, that flag is the old and undocumented `HEAP_SHARED`. `HEAP_CreateSystemHeap()` tries to map memory at `0x80000000`, this fails on Wow64 (and probably should for all 32-bit platforms for non-LAA EXEs). As a workaround, map the memory anywhere if it fails at `0x80000000`. I wasn't able to find much documentation about `HEAP_SHARED`, it dates back to Win9x and I guess Wine may have needed it for some native DLLs. It's not clear whether modern Windows really implements any shared heap functionality any more, every 'shared' heap created returns a different pointer (even in the same process). Maybe Wine should remove the functionality too? -- v2: kernel32: Remove shared heap functionality. ntdll: Remove shared heap functionality. https://gitlab.winehq.org/wine/wine/-/merge_requests/920
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/ntdll/heap.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index ef91096f258..8530bdf223f 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -186,7 +186,6 @@ struct heap DWORD force_flags; /* 0044/0074 */ /* end of the Windows 10 compatible struct layout */ - BOOL shared; /* System shared heap */ struct list entry; /* Entry in process heap list */ struct list subheap_list; /* Sub-heap list */ struct list large_list; /* Large blocks list */ @@ -753,7 +752,7 @@ static void free_used_block( struct heap *heap, SUBHEAP *subheap, struct block * list_remove( &subheap->entry ); NtFreeVirtualMemory( NtCurrentProcess(), &addr, &size, MEM_RELEASE ); } - else if (!heap->shared) + else { /* keep room for a full committed block as hysteresis */ subheap_decommit( heap, subheap, (char *)(entry + 1) + (COMMIT_MASK + 1) ); @@ -921,7 +920,6 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWOR if (!commitSize) commitSize = COMMIT_MASK + 1; totalSize = min( totalSize, 0xffff0000 ); /* don't allow a heap larger than 4GB */ if (totalSize < commitSize) totalSize = commitSize; - if (flags & HEAP_SHARED) commitSize = totalSize; /* always commit everything in a shared heap */ commitSize = min( totalSize, (commitSize + COMMIT_MASK) & ~COMMIT_MASK ); /* allocate the memory block */ @@ -955,7 +953,6 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWOR heap->ffeeffee = 0xffeeffee; heap->auto_flags = (flags & HEAP_GROWABLE); heap->flags = (flags & ~HEAP_SHARED); - heap->shared = (flags & HEAP_SHARED) != 0; heap->magic = HEAP_MAGIC; heap->grow_size = max( HEAP_DEF_SIZE, totalSize ); heap->min_size = commitSize; @@ -993,19 +990,6 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWOR RtlInitializeCriticalSection( &heap->cs ); heap->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": heap.cs"); } - - if (heap->shared) - { - /* let's assume that only one thread at a time will try to do this */ - HANDLE sem = heap->cs.LockSemaphore; - if (!sem) NtCreateSemaphore( &sem, SEMAPHORE_ALL_ACCESS, NULL, 0, 1 ); - - NtDuplicateObject( NtCurrentProcess(), sem, NtCurrentProcess(), &sem, 0, 0, - DUPLICATE_MAKE_GLOBAL | DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE ); - heap->cs.LockSemaphore = sem; - RtlFreeHeap( process_heap, 0, heap->cs.DebugInfo ); - heap->cs.DebugInfo = NULL; - } } block_size = subheap_size( subheap ) - subheap_overhead( subheap ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/920
From: Brendan Shanks <bshanks(a)codeweavers.com> --- dlls/kernel32/heap.c | 66 ++------------------------------------------ 1 file changed, 2 insertions(+), 64 deletions(-) diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c index a2910688e44..1c1e6b05f25 100644 --- a/dlls/kernel32/heap.c +++ b/dlls/kernel32/heap.c @@ -41,57 +41,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(globalmem); -/* address where we try to map the system heap */ -#define SYSTEM_HEAP_BASE ((void*)0x80000000) -#define SYSTEM_HEAP_SIZE 0x1000000 /* Default heap size = 16Mb */ - -static HANDLE systemHeap; /* globally shared heap */ - BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void **user_value, ULONG *user_flags ); BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value ); -/*********************************************************************** - * HEAP_CreateSystemHeap - * - * Create the system heap. - */ -static inline HANDLE HEAP_CreateSystemHeap(void) -{ - int created; - void *base; - HANDLE map, event; - - /* create the system heap event first */ - event = CreateEventA( NULL, TRUE, FALSE, "__wine_system_heap_event" ); - - if (!(map = CreateFileMappingA( INVALID_HANDLE_VALUE, NULL, SEC_COMMIT | PAGE_READWRITE, - 0, SYSTEM_HEAP_SIZE, "__wine_system_heap" ))) return 0; - created = (GetLastError() != ERROR_ALREADY_EXISTS); - - if (!(base = MapViewOfFileEx( map, FILE_MAP_ALL_ACCESS, 0, 0, 0, SYSTEM_HEAP_BASE ))) - { - /* pre-defined address not available */ - ERR( "system heap base address %p not available\n", SYSTEM_HEAP_BASE ); - return 0; - } - - if (created) /* newly created heap */ - { - systemHeap = RtlCreateHeap( HEAP_SHARED, base, SYSTEM_HEAP_SIZE, - SYSTEM_HEAP_SIZE, NULL, NULL ); - SetEvent( event ); - } - else - { - /* wait for the heap to be initialized */ - WaitForSingleObject( event, INFINITE ); - systemHeap = base; - } - CloseHandle( map ); - return systemHeap; -} - - /*********************************************************************** * HeapCreate (KERNEL32.@) * @@ -108,17 +60,8 @@ HANDLE WINAPI HeapCreate( ) { HANDLE ret; - if ( flags & HEAP_SHARED ) - { - if (!systemHeap) HEAP_CreateSystemHeap(); - else WARN( "Shared Heap requested, returning system heap.\n" ); - ret = systemHeap; - } - else - { - ret = RtlCreateHeap( flags, NULL, maxSize, initialSize, NULL, NULL ); - if (!ret) SetLastError( ERROR_NOT_ENOUGH_MEMORY ); - } + ret = RtlCreateHeap( flags, NULL, maxSize, initialSize, NULL, NULL ); + if (!ret) SetLastError( ERROR_NOT_ENOUGH_MEMORY ); return ret; } @@ -134,11 +77,6 @@ HANDLE WINAPI HeapCreate( */ BOOL WINAPI HeapDestroy( HANDLE heap /* [in] Handle of heap */ ) { - if (heap == systemHeap) - { - WARN( "attempt to destroy system heap, returning TRUE!\n" ); - return TRUE; - } if (!RtlDestroyHeap( heap )) return TRUE; SetLastError( ERROR_INVALID_HANDLE ); return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/920
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=124903 Your paranoid android. === debian11 (build log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24692. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24692. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24692.
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
Marvin