[PATCH 0/7] MR1629: ntdll: Finish heap refactoring in preparation for LFH support.
Cleaning up and splitting subheap creation helpers and free block merge logic. Should be the last refactoring needed before LFH specific changes, as implemented in https://gitlab.winehq.org/wine/wine/-/merge_requests/1628. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 58 +++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 1de9fe6a4b6..af95e78d7e9 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -832,22 +832,44 @@ static inline void shrink_used_block( struct heap *heap, ULONG flags, struct blo } +static NTSTATUS allocate_region( ULONG flags, SIZE_T *commit_size, SIZE_T *reserve_size, void **addr ) +{ + NTSTATUS status; + + if (!(flags & HEAP_GROWABLE)) return STATUS_NO_MEMORY; + + if (reserve_size) *commit_size = min( *commit_size, *reserve_size ); + *addr = NULL; + + /* allocate the memory block */ + if (reserve_size && *reserve_size != *commit_size && + (status = NtAllocateVirtualMemory( NtCurrentProcess(), addr, 0, reserve_size, MEM_RESERVE, + get_protection_type( flags ) ))) + { + WARN( "Could not allocate %#Ix bytes, status %#lx\n", *reserve_size, status ); + return status; + } + if ((status = NtAllocateVirtualMemory( NtCurrentProcess(), addr, 0, commit_size, MEM_COMMIT, + get_protection_type( flags ) ))) + { + WARN( "Could not commit %#Ix bytes, status %#lx\n", *commit_size, status ); + return status; + } + + return status; +} + + static NTSTATUS heap_allocate_large( struct heap *heap, ULONG flags, SIZE_T block_size, SIZE_T size, void **ret ) { ARENA_LARGE *arena; SIZE_T total_size = ROUND_SIZE( sizeof(*arena) + size, REGION_ALIGN - 1 ); - LPVOID address = NULL; struct block *block; + void *address; - if (!(flags & HEAP_GROWABLE)) return STATUS_NO_MEMORY; if (total_size < size) return STATUS_NO_MEMORY; /* overflow */ - if (NtAllocateVirtualMemory( NtCurrentProcess(), &address, 0, &total_size, MEM_COMMIT, - get_protection_type( heap->flags ) )) - { - WARN( "Could not allocate block for %#Ix bytes\n", size ); - return STATUS_NO_MEMORY; - } + if (allocate_region( flags, &total_size, NULL, &address )) return STATUS_NO_MEMORY; arena = address; block = &arena->block; @@ -937,24 +959,12 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWOR if (!address) { - if (!commitSize) commitSize = REGION_ALIGN; - totalSize = min( totalSize, 0xffff0000 ); /* don't allow a heap larger than 4GB */ - if (totalSize < commitSize) totalSize = commitSize; - commitSize = min( totalSize, (SIZE_T)ROUND_ADDR( commitSize + REGION_ALIGN - 1, REGION_ALIGN - 1 ) ); + commitSize = ROUND_SIZE( max( commitSize, REGION_ALIGN ), REGION_ALIGN - 1 ); + totalSize = min( max( commitSize, totalSize ), 0xffff0000 ); /* don't allow a heap larger than 4GB */ /* allocate the memory block */ - if (NtAllocateVirtualMemory( NtCurrentProcess(), &address, 0, &totalSize, MEM_RESERVE, - get_protection_type( flags ) )) - { - WARN( "Could not allocate %#Ix bytes\n", totalSize ); - return NULL; - } - if (NtAllocateVirtualMemory( NtCurrentProcess(), &address, 0, &commitSize, MEM_COMMIT, - get_protection_type( flags ) )) - { - WARN( "Could not commit %#Ix bytes for sub-heap %p\n", commitSize, address ); - return NULL; - } + if (!heap) flags |= HEAP_GROWABLE; + if (allocate_region( flags, &commitSize, &totalSize, &address )) return NULL; } if (heap) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> And rename HEAP_CreateSubHeap to create_subheap. --- dlls/ntdll/heap.c | 154 ++++++++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 86 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index af95e78d7e9..21ae5996dfd 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -945,90 +945,23 @@ static BOOL validate_large_block( const struct heap *heap, const struct block *b return !err; } -/*********************************************************************** - * HEAP_CreateSubHeap - */ -static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWORD flags, - SIZE_T commitSize, SIZE_T totalSize ) + +static SUBHEAP *create_subheap( struct heap *heap, DWORD flags, SIZE_T commit_size, SIZE_T total_size ) { - struct heap *heap = *heap_ptr; - struct entry *pEntry; SIZE_T block_size; SUBHEAP *subheap; - unsigned int i; - - if (!address) - { - commitSize = ROUND_SIZE( max( commitSize, REGION_ALIGN ), REGION_ALIGN - 1 ); - totalSize = min( max( commitSize, totalSize ), 0xffff0000 ); /* don't allow a heap larger than 4GB */ - - /* allocate the memory block */ - if (!heap) flags |= HEAP_GROWABLE; - if (allocate_region( flags, &commitSize, &totalSize, &address )) return NULL; - } - - if (heap) - { - /* If this is a secondary subheap, insert it into list */ - - subheap = address; - subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); - list_add_head( &heap->subheap_list, &subheap->entry ); - } - else - { - /* If this is a primary subheap, initialize main heap */ - heap = address; - heap->ffeeffee = 0xffeeffee; - heap->auto_flags = (flags & HEAP_GROWABLE); - heap->flags = (flags & ~HEAP_SHARED); - heap->magic = HEAP_MAGIC; - heap->grow_size = max( HEAP_DEF_SIZE, totalSize ); - heap->min_size = commitSize; - list_init( &heap->subheap_list ); - list_init( &heap->large_list ); + commit_size = ROUND_SIZE( max( commit_size, REGION_ALIGN ), REGION_ALIGN - 1 ); + total_size = min( max( commit_size, total_size ), 0xffff0000 ); /* don't allow a heap larger than 4GB */ - subheap = &heap->subheap; - subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); - list_add_head( &heap->subheap_list, &subheap->entry ); + if (allocate_region( flags, &commit_size, &total_size, (void **)&subheap )) return NULL; - /* Build the free lists */ - - list_init( &heap->free_lists[0].entry ); - for (i = 0, pEntry = heap->free_lists; i < HEAP_NB_FREE_LISTS; i++, pEntry++) - { - block_set_flags( &pEntry->block, ~0, BLOCK_FLAG_FREE_LINK ); - block_set_size( &pEntry->block, 0 ); - block_set_type( &pEntry->block, BLOCK_TYPE_FREE ); - block_set_base( &pEntry->block, heap ); - if (i) list_add_after( &pEntry[-1].entry, &pEntry->entry ); - } - - /* Initialize critical section */ - - if (!process_heap) /* do it by hand to avoid memory allocations */ - { - heap->cs.DebugInfo = &process_heap_cs_debug; - heap->cs.LockCount = -1; - heap->cs.RecursionCount = 0; - heap->cs.OwningThread = 0; - heap->cs.LockSemaphore = 0; - heap->cs.SpinCount = 0; - process_heap_cs_debug.CriticalSection = &heap->cs; - } - else - { - RtlInitializeCriticalSection( &heap->cs ); - heap->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": heap.cs"); - } - } + subheap_set_bounds( subheap, (char *)subheap + commit_size, (char *)subheap + total_size ); + list_add_head( &heap->subheap_list, &subheap->entry ); - block_size = subheap_size( subheap ) - subheap_overhead( subheap ); - block_size &= ~(BLOCK_ALIGN - 1); + block_size = (SIZE_T)ROUND_ADDR( subheap_size( subheap ) - subheap_overhead( subheap ), BLOCK_ALIGN - 1 ); create_free_block( heap, flags, subheap, first_block( subheap ), block_size ); - *heap_ptr = heap; return subheap; } @@ -1068,8 +1001,7 @@ static struct block *find_free_block( struct heap *heap, ULONG flags, SIZE_T blo total_size = sizeof(SUBHEAP) + block_size + sizeof(struct entry); if (total_size < block_size) return NULL; /* overflow */ - if ((subheap = HEAP_CreateSubHeap( &heap, NULL, flags, total_size, - max( heap->grow_size, total_size ) ))) + if ((subheap = create_subheap( heap, flags, total_size, max( heap->grow_size, total_size ) ))) { if (heap->grow_size <= HEAP_MAX_FREE_BLOCK_SIZE / 2) heap->grow_size *= 2; } @@ -1077,8 +1009,7 @@ static struct block *find_free_block( struct heap *heap, ULONG flags, SIZE_T blo { if (heap->grow_size <= total_size || heap->grow_size <= 4 * 1024 * 1024) return NULL; heap->grow_size /= 2; - subheap = HEAP_CreateSubHeap( &heap, NULL, flags, total_size, - max( heap->grow_size, total_size ) ); + subheap = create_subheap( heap, flags, total_size, max( heap->grow_size, total_size ) ); } TRACE( "created new sub-heap %p of %#Ix bytes for heap %p\n", subheap, subheap_size( subheap ), heap ); @@ -1399,20 +1330,71 @@ static void heap_set_debug_flags( HANDLE handle ) * Success: A HANDLE to the newly created heap. * Failure: a NULL HANDLE. */ -HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T commitSize, - PVOID unknown, PRTL_HEAP_DEFINITION definition ) +HANDLE WINAPI RtlCreateHeap( ULONG flags, void *addr, SIZE_T total_size, SIZE_T commit_size, + void *unknown, RTL_HEAP_DEFINITION *definition ) { - struct heap *heap = NULL; + struct entry *entry; + struct heap *heap; + SIZE_T block_size; SUBHEAP *subheap; + unsigned int i; - /* Allocate the heap block */ + TRACE( "flags %#lx, addr %p, total_size %#Ix, commit_size %#Ix, unknown %p, definition %p\n", + flags, addr, total_size, commit_size, unknown, definition ); flags &= ~(HEAP_TAIL_CHECKING_ENABLED|HEAP_FREE_CHECKING_ENABLED); if (process_heap) flags |= HEAP_PRIVATE; - if (!process_heap || !totalSize || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; - if (!totalSize) totalSize = HEAP_DEF_SIZE; + if (!process_heap || !total_size || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; + if (!total_size) total_size = HEAP_DEF_SIZE; + + if (!(heap = addr)) + { + commit_size = ROUND_SIZE( max( commit_size, REGION_ALIGN ), REGION_ALIGN - 1 ); + total_size = min( max( commit_size, total_size ), 0xffff0000 ); /* don't allow a heap larger than 4GB */ + if (allocate_region( flags | HEAP_GROWABLE, &commit_size, &total_size, (void **)&heap )) return 0; + } + + heap->ffeeffee = 0xffeeffee; + heap->auto_flags = (flags & HEAP_GROWABLE); + heap->flags = (flags & ~HEAP_SHARED); + heap->magic = HEAP_MAGIC; + heap->grow_size = max( HEAP_DEF_SIZE, total_size ); + heap->min_size = commit_size; + list_init( &heap->subheap_list ); + list_init( &heap->large_list ); - if (!(subheap = HEAP_CreateSubHeap( &heap, addr, flags, commitSize, totalSize ))) return 0; + subheap = &heap->subheap; + subheap_set_bounds( subheap, (char *)heap + commit_size, (char *)heap + total_size ); + list_add_head( &heap->subheap_list, &subheap->entry ); + + list_init( &heap->free_lists[0].entry ); + for (i = 0, entry = heap->free_lists; i < HEAP_NB_FREE_LISTS; i++, entry++) + { + block_set_flags( &entry->block, ~0, BLOCK_FLAG_FREE_LINK ); + block_set_size( &entry->block, 0 ); + block_set_type( &entry->block, BLOCK_TYPE_FREE ); + block_set_base( &entry->block, heap ); + if (i) list_add_after( &entry[-1].entry, &entry->entry ); + } + + if (!process_heap) /* do it by hand to avoid memory allocations */ + { + heap->cs.DebugInfo = &process_heap_cs_debug; + heap->cs.LockCount = -1; + heap->cs.RecursionCount = 0; + heap->cs.OwningThread = 0; + heap->cs.LockSemaphore = 0; + heap->cs.SpinCount = 0; + process_heap_cs_debug.CriticalSection = &heap->cs; + } + else + { + RtlInitializeCriticalSection( &heap->cs ); + heap->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": heap.cs"); + } + + block_size = (SIZE_T)ROUND_ADDR( subheap_size( subheap ) - subheap_overhead( subheap ), BLOCK_ALIGN - 1 ); + create_free_block( heap, flags, subheap, first_block( subheap ), block_size ); heap_set_debug_flags( heap ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 21ae5996dfd..2039f51f3ab 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -824,10 +824,8 @@ static inline void shrink_used_block( struct heap *heap, ULONG flags, struct blo } else { - struct block *next; block_set_size( block, old_block_size ); block->tail_size = old_block_size - sizeof(*block) - size; - if ((next = next_block( subheap, block ))) block_set_flags( next, BLOCK_FLAG_PREV_FREE, 0 ); } } @@ -1511,14 +1509,16 @@ static SIZE_T heap_get_block_size( const struct heap *heap, ULONG flags, SIZE_T static NTSTATUS heap_allocate_block( struct heap *heap, ULONG flags, SIZE_T block_size, SIZE_T size, void **ret ) { + struct block *block, *next; SIZE_T old_block_size; - struct block *block; + SUBHEAP *subheap; /* Locate a suitable free block */ if (!(block = find_free_block( heap, flags, block_size ))) return STATUS_NO_MEMORY; /* read the free block size, changing block type or flags may alter it */ old_block_size = block_get_size( block ); + subheap = block_get_subheap( heap, block ); block_set_type( block, BLOCK_TYPE_USED ); block_set_flags( block, ~0, BLOCK_USER_FLAGS( flags ) ); @@ -1526,6 +1526,8 @@ static NTSTATUS heap_allocate_block( struct heap *heap, ULONG flags, SIZE_T bloc initialize_block( block, 0, size, flags ); mark_block_tail( block, flags ); + if ((next = next_block( subheap, block ))) block_set_flags( next, BLOCK_FLAG_PREV_FREE, 0 ); + *ret = block + 1; return STATUS_SUCCESS; } @@ -1653,10 +1655,11 @@ static NTSTATUS heap_resize_block( struct heap *heap, ULONG flags, struct block valgrind_notify_resize( block + 1, *old_size, size ); block_set_flags( block, BLOCK_FLAG_USER_MASK & ~BLOCK_FLAG_USER_INFO, BLOCK_USER_FLAGS( flags ) ); shrink_used_block( heap, flags, block, old_block_size, block_size, size ); - initialize_block( block, *old_size, size, flags ); mark_block_tail( block, flags ); + if ((next = next_block( subheap, block ))) block_set_flags( next, BLOCK_FLAG_PREV_FREE, 0 ); + heap_unlock( heap, flags ); *ret = block + 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 2039f51f3ab..84f171f449c 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -811,22 +811,19 @@ static NTSTATUS heap_free_block( struct heap *heap, ULONG flags, struct block *b } -static inline void shrink_used_block( struct heap *heap, ULONG flags, struct block *block, - SIZE_T old_block_size, SIZE_T block_size, SIZE_T size ) +static struct block *split_block( struct heap *heap, ULONG flags, struct block *block, + SIZE_T old_block_size, SIZE_T block_size ) { SUBHEAP *subheap = block_get_subheap( heap, block ); if (old_block_size >= block_size + HEAP_MIN_BLOCK_SIZE) { block_set_size( block, block_size ); - block->tail_size = block_size - sizeof(*block) - size; - create_free_block( heap, flags, subheap, next_block( subheap, block ), old_block_size - block_size ); - } - else - { - block_set_size( block, old_block_size ); - block->tail_size = old_block_size - sizeof(*block) - size; + return next_block( subheap, block ); } + + block_set_size( block, old_block_size ); + return NULL; } @@ -1520,9 +1517,12 @@ static NTSTATUS heap_allocate_block( struct heap *heap, ULONG flags, SIZE_T bloc old_block_size = block_get_size( block ); subheap = block_get_subheap( heap, block ); + if ((next = split_block( heap, flags, block, old_block_size, block_size ))) + create_free_block( heap, flags, subheap, next, old_block_size - block_size ); + block_set_type( block, BLOCK_TYPE_USED ); block_set_flags( block, ~0, BLOCK_USER_FLAGS( flags ) ); - shrink_used_block( heap, flags, block, old_block_size, block_size, size ); + block->tail_size = block_get_size( block ) - sizeof(*block) - size; initialize_block( block, 0, size, flags ); mark_block_tail( block, flags ); @@ -1652,9 +1652,12 @@ static NTSTATUS heap_resize_block( struct heap *heap, ULONG flags, struct block old_block_size += block_get_size( next ); } + if ((next = split_block( heap, flags, block, old_block_size, block_size ))) + create_free_block( heap, flags, subheap, next, old_block_size - block_size ); + valgrind_notify_resize( block + 1, *old_size, size ); block_set_flags( block, BLOCK_FLAG_USER_MASK & ~BLOCK_FLAG_USER_INFO, BLOCK_USER_FLAGS( flags ) ); - shrink_used_block( heap, flags, block, old_block_size, block_size, size ); + block->tail_size = block_get_size( block ) - sizeof(*block) - size; initialize_block( block, *old_size, size, flags ); mark_block_tail( block, flags ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> As create_free_block is only called in other places where it is not possible to have a free block either before or after. --- dlls/ntdll/heap.c | 50 +++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 84f171f449c..411ea2a0c53 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -725,16 +725,6 @@ static void create_free_block( struct heap *heap, ULONG flags, SUBHEAP *subheap, if (end > commit_end) end = commit_end; if (end > (char *)(entry + 1)) mark_block_free( entry + 1, end - (char *)(entry + 1), flags ); - if ((next = next_block( subheap, block )) && (block_get_flags( next ) & BLOCK_FLAG_FREE)) - { - /* merge with the next block if it is free */ - struct entry *next_entry = (struct entry *)next; - list_remove( &next_entry->entry ); - block_size += block_get_size( next ); - block_set_size( block, block_size ); - mark_block_free( next_entry, sizeof(*next_entry), flags ); - } - if ((next = next_block( subheap, block ))) { /* set the next block PREV_FREE flag and back pointer */ @@ -772,40 +762,44 @@ static struct block *heap_delay_free( struct heap *heap, ULONG flags, struct blo static NTSTATUS heap_free_block( struct heap *heap, ULONG flags, struct block *block ) { + SUBHEAP *subheap = block_get_subheap( heap, block ); + SIZE_T block_size = block_get_size( block ); struct entry *entry; - SIZE_T block_size; - SUBHEAP *subheap; + struct block *next; + + if ((next = next_block( subheap, block )) && (block_get_flags( next ) & BLOCK_FLAG_FREE)) + { + /* merge with next block if it is free */ + entry = (struct entry *)next; + block_size += block_get_size( &entry->block ); + list_remove( &entry->entry ); + next = next_block( subheap, next ); + } - block_size = block_get_size( block ); if (block_get_flags( block ) & BLOCK_FLAG_PREV_FREE) { /* merge with previous block if it is free */ - block = *((struct block **)block - 1); - block_size += block_get_size( block ); - entry = (struct entry *)block; + entry = *((struct entry **)block - 1); + block_size += block_get_size( &entry->block ); list_remove( &entry->entry ); + block = &entry->block; } - else entry = (struct entry *)block; - subheap = block_get_subheap( heap, block ); - create_free_block( heap, flags, subheap, block, block_size ); - if (next_block( subheap, block )) return STATUS_SUCCESS; /* not the last block */ - - if (block == first_block( subheap ) && subheap != &heap->subheap) + if (block == first_block( subheap ) && !next && subheap != &heap->subheap) { /* free the subheap if it's empty and not the main one */ void *addr = subheap_base( subheap ); SIZE_T size = 0; - list_remove( &entry->entry ); list_remove( &subheap->entry ); NtFreeVirtualMemory( NtCurrentProcess(), &addr, &size, MEM_RELEASE ); + return STATUS_SUCCESS; } - else - { - /* keep room for a full committed block as hysteresis */ - subheap_decommit( heap, subheap, (char *)(entry + 1) + REGION_ALIGN ); - } + + create_free_block( heap, flags, subheap, block, block_size ); + + /* keep room for a full committed block as hysteresis */ + if (!next) subheap_decommit( heap, subheap, (char *)((struct entry *)block + 1) + REGION_ALIGN ); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 411ea2a0c53..b0b2695065e 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -707,12 +707,10 @@ static inline BOOL subheap_decommit( const struct heap *heap, SUBHEAP *subheap, return TRUE; } - -static void create_free_block( struct heap *heap, ULONG flags, SUBHEAP *subheap, struct block *block, SIZE_T block_size ) +static void block_init_free( struct block *block, ULONG flags, SUBHEAP *subheap, SIZE_T block_size ) { const char *end = (char *)block + block_size, *commit_end = subheap_commit_end( subheap ); - struct entry *entry = (struct entry *)block, *list; - struct block *next; + struct entry *entry = (struct entry *)block; valgrind_make_writable( block, sizeof(*entry) ); block_set_type( block, BLOCK_TYPE_FREE ); @@ -724,6 +722,12 @@ static void create_free_block( struct heap *heap, ULONG flags, SUBHEAP *subheap, if (end > commit_end) end = commit_end; if (end > (char *)(entry + 1)) mark_block_free( entry + 1, end - (char *)(entry + 1), flags ); +} + +static void insert_free_block( struct heap *heap, ULONG flags, SUBHEAP *subheap, struct block *block ) +{ + struct entry *entry = (struct entry *)block, *list; + struct block *next; if ((next = next_block( subheap, block ))) { @@ -796,7 +800,8 @@ static NTSTATUS heap_free_block( struct heap *heap, ULONG flags, struct block *b return STATUS_SUCCESS; } - create_free_block( heap, flags, subheap, block, block_size ); + block_init_free( block, flags, subheap, block_size ); + insert_free_block( heap, flags, subheap, block ); /* keep room for a full committed block as hysteresis */ if (!next) subheap_decommit( heap, subheap, (char *)((struct entry *)block + 1) + REGION_ALIGN ); @@ -949,7 +954,8 @@ static SUBHEAP *create_subheap( struct heap *heap, DWORD flags, SIZE_T commit_si list_add_head( &heap->subheap_list, &subheap->entry ); block_size = (SIZE_T)ROUND_ADDR( subheap_size( subheap ) - subheap_overhead( subheap ), BLOCK_ALIGN - 1 ); - create_free_block( heap, flags, subheap, first_block( subheap ), block_size ); + block_init_free( first_block( subheap ), flags, subheap, block_size ); + insert_free_block( heap, flags, subheap, first_block( subheap ) ); return subheap; } @@ -1383,7 +1389,8 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, void *addr, SIZE_T total_size, SIZE_T } block_size = (SIZE_T)ROUND_ADDR( subheap_size( subheap ) - subheap_overhead( subheap ), BLOCK_ALIGN - 1 ); - create_free_block( heap, flags, subheap, first_block( subheap ), block_size ); + block_init_free( first_block( subheap ), flags, subheap, block_size ); + insert_free_block( heap, flags, subheap, first_block( subheap ) ); heap_set_debug_flags( heap ); @@ -1512,7 +1519,10 @@ static NTSTATUS heap_allocate_block( struct heap *heap, ULONG flags, SIZE_T bloc subheap = block_get_subheap( heap, block ); if ((next = split_block( heap, flags, block, old_block_size, block_size ))) - create_free_block( heap, flags, subheap, next, old_block_size - block_size ); + { + block_init_free( next, flags, subheap, old_block_size - block_size ); + insert_free_block( heap, flags, subheap, next ); + } block_set_type( block, BLOCK_TYPE_USED ); block_set_flags( block, ~0, BLOCK_USER_FLAGS( flags ) ); @@ -1647,7 +1657,10 @@ static NTSTATUS heap_resize_block( struct heap *heap, ULONG flags, struct block } if ((next = split_block( heap, flags, block, old_block_size, block_size ))) - create_free_block( heap, flags, subheap, next, old_block_size - block_size ); + { + block_init_free( next, flags, subheap, old_block_size - block_size ); + insert_free_block( heap, flags, subheap, next ); + } valgrind_notify_resize( block + 1, *old_size, size ); block_set_flags( block, BLOCK_FLAG_USER_MASK & ~BLOCK_FLAG_USER_INFO, BLOCK_USER_FLAGS( flags ) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index b0b2695065e..7cc849749d8 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -955,7 +955,6 @@ static SUBHEAP *create_subheap( struct heap *heap, DWORD flags, SIZE_T commit_si block_size = (SIZE_T)ROUND_ADDR( subheap_size( subheap ) - subheap_overhead( subheap ), BLOCK_ALIGN - 1 ); block_init_free( first_block( subheap ), flags, subheap, block_size ); - insert_free_block( heap, flags, subheap, first_block( subheap ) ); return subheap; } @@ -1009,9 +1008,7 @@ static struct block *find_free_block( struct heap *heap, ULONG flags, SIZE_T blo TRACE( "created new sub-heap %p of %#Ix bytes for heap %p\n", subheap, subheap_size( subheap ), heap ); - entry = first_block( subheap ); - list_remove( &entry->entry ); - return &entry->block; + return first_block( subheap ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1629
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=127063 Your paranoid android. === debian11 (32 bit report) === comctl32: dpa.c:216: Test failed: ret=1 error=0 dpa.c:216: Test failed: ret=1 error=1411
On Thu Dec 1 12:59:58 2022 +0000, **** wrote:
Marvin replied on the mailing list: ``` 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=127063 Your paranoid android. === debian11 (32 bit report) === comctl32: dpa.c:216: Test failed: ret=1 error=0 dpa.c:216: Test failed: ret=1 error=1411 ``` This looks genuine, I'll have a look.
I think I can also add an additional safety check to detect heap mismatches that were recently introduced with CRT alloc conversions, and which are causing most of the crashes in https://gitlab.winehq.org/wine/wine/-/merge_requests/1628. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1629#note_18241
participants (2)
-
Marvin -
Rémi Bernon