[PATCH v2 0/2] MR920: kernel32: Map system heap anywhere if the standard address is unavailable.

[Brendan Shanks (＠bshanks)]

`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

[PATCH v2 1/2] ntdll: Remove shared heap functionality.

[Brendan Shanks]

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

[PATCH v2 2/2] kernel32: Remove shared heap functionality.

[Brendan Shanks]

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

[PATCH v2 2/2] kernel32: Remove shared heap functionality.

[Marvin]

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.