From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 374b4e1f209..75622da7a6a 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -134,7 +134,7 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0 ); /* minimum size that must remain to shrink an allocated block */ #define HEAP_MIN_SHRINK_SIZE (HEAP_MIN_DATA_SIZE+sizeof(ARENA_FREE)) /* minimum size to start allocating large blocks */ -#define HEAP_MIN_LARGE_BLOCK_SIZE 0x7f000 +#define HEAP_MIN_LARGE_BLOCK_SIZE (0x10000 * ALIGNMENT - 0x1000) /* extra size to add at the end of block for tail checking */ #define HEAP_TAIL_EXTRA_SIZE(flags) \ ((flags & HEAP_TAIL_CHECKING_ENABLED) || RUNNING_ON_VALGRIND ? ALIGNMENT : 0)
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=114996
Your paranoid android.
=== debian11 (32 bit report) ===
ntdll: change.c:277: Test failed: should be ready
From: Rémi Bernon rbernon@codeweavers.com
This tweaks the test sizes so that it passes on Wine too. The created heap should be large enough to hold the requested alloc size, but the allocation still fails.
Windows seems to have a specific lower threshold for large blocks with non growable heaps, but it's probably not very useful to match it.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 6 ++---- dlls/ntdll/heap.c | 5 +++-- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 1b8e019c0ec..d9cae9f723b 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -456,7 +456,7 @@ static void test_HeapCreate(void) ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() );
- heap = HeapCreate( 0, 2 * alloc_size, 5 * alloc_size ); + heap = HeapCreate( 0, 8 * alloc_size, 8 * alloc_size ); ok( !!heap, "HeapCreate failed, error %lu\n", GetLastError() ); ok( !((ULONG_PTR)heap & 0xffff), "wrong heap alignment\n" );
@@ -470,10 +470,8 @@ static void test_HeapCreate(void) /* cannot allocate large blocks from fixed size heap */
SetLastError( 0xdeadbeef ); - ptr1 = HeapAlloc( heap, 0, 3 * alloc_size ); - todo_wine + ptr1 = HeapAlloc( heap, 0, 4 * alloc_size ); ok( !ptr1, "HeapAlloc succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_NOT_ENOUGH_MEMORY, "got error %lu\n", GetLastError() ); ret = HeapFree( heap, 0, ptr1 ); ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 75622da7a6a..fa9f617f482 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -841,6 +841,7 @@ static void *allocate_large_block( HEAP *heap, DWORD flags, SIZE_T size ) SIZE_T block_size = sizeof(*arena) + ROUND_SIZE(size) + HEAP_TAIL_EXTRA_SIZE(flags); LPVOID address = NULL;
+ if (!(flags & HEAP_GROWABLE)) return NULL; if (block_size < size) return NULL; /* overflow */ if (NtAllocateVirtualMemory( NtCurrentProcess(), &address, 0, &block_size, MEM_COMMIT, get_protection_type( flags ))) @@ -1561,7 +1562,7 @@ static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret if (rounded_size < size) return STATUS_NO_MEMORY; /* overflow */ if (rounded_size < HEAP_MIN_DATA_SIZE) rounded_size = HEAP_MIN_DATA_SIZE;
- if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE && (flags & HEAP_GROWABLE)) + if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE) { if (!(*ret = allocate_large_block( heap, flags, size ))) return STATUS_NO_MEMORY; return STATUS_SUCCESS; @@ -1685,7 +1686,7 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size { struct block *next;
- if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE && (flags & HEAP_GROWABLE)) + if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE) { if (flags & HEAP_REALLOC_IN_PLACE_ONLY) return STATUS_NO_MEMORY; if (!(*ret = allocate_large_block( heap, flags, size ))) return STATUS_NO_MEMORY;
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 45 ++++++++------------------------------------- 1 file changed, 8 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index fa9f617f482..eb45a94da95 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -1663,9 +1663,11 @@ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *pt
static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size, void **ret ) { + struct block *next; ARENA_INUSE *pArena; SUBHEAP *subheap; SIZE_T oldBlockSize, oldActualSize, rounded_size; + NTSTATUS status;
rounded_size = ROUND_SIZE(size) + HEAP_TAIL_EXTRA_SIZE(flags); if (rounded_size < size) return STATUS_NO_MEMORY; /* overflow */ @@ -1684,20 +1686,8 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size oldActualSize = (pArena->size & ARENA_SIZE_MASK) - pArena->unused_bytes; if (rounded_size > oldBlockSize) { - struct block *next; - - if (rounded_size >= HEAP_MIN_LARGE_BLOCK_SIZE) - { - if (flags & HEAP_REALLOC_IN_PLACE_ONLY) return STATUS_NO_MEMORY; - if (!(*ret = allocate_large_block( heap, flags, size ))) return STATUS_NO_MEMORY; - memcpy( *ret, pArena + 1, oldActualSize ); - notify_free( pArena + 1 ); - HEAP_MakeInUseBlockFree( subheap, pArena ); - return STATUS_SUCCESS; - } - if ((next = next_block( subheap, pArena )) && (block_get_flags( next ) & ARENA_FLAG_FREE) && - (oldBlockSize + block_get_size( next ) >= rounded_size)) + rounded_size < HEAP_MIN_LARGE_BLOCK_SIZE && rounded_size <= oldBlockSize + block_get_size( next )) { /* The next block is free and large enough */ struct entry *entry = (struct entry *)next; @@ -1707,34 +1697,15 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size notify_realloc( pArena + 1, oldActualSize, size ); HEAP_ShrinkBlock( subheap, pArena, rounded_size ); } - else /* Do it the hard way */ + else { - ARENA_FREE *pNew; - ARENA_INUSE *pInUse; - SUBHEAP *newsubheap; - if (flags & HEAP_REALLOC_IN_PLACE_ONLY) return STATUS_NO_MEMORY; - if (!(pNew = HEAP_FindFreeBlock( heap, rounded_size, &newsubheap ))) return STATUS_NO_MEMORY; - - /* Build the in-use arena */ - - list_remove( &pNew->entry ); - pInUse = (ARENA_INUSE *)pNew; - pInUse->size = (pInUse->size & ~ARENA_FLAG_FREE) - + sizeof(ARENA_FREE) - sizeof(ARENA_INUSE); - pInUse->magic = ARENA_INUSE_MAGIC; - HEAP_ShrinkBlock( newsubheap, pInUse, rounded_size ); - - mark_block_initialized( pInUse + 1, oldActualSize ); - notify_alloc( pInUse + 1, size, FALSE ); - memcpy( pInUse + 1, pArena + 1, oldActualSize ); - - /* Free the previous block */ - + if ((status = heap_allocate( heap, flags & ~HEAP_ZERO_MEMORY, size, ret ))) return status; + memcpy( *ret, pArena + 1, oldActualSize ); + if (flags & HEAP_ZERO_MEMORY) memset( (char *)*ret + oldActualSize, 0, size - oldActualSize ); notify_free( pArena + 1 ); HEAP_MakeInUseBlockFree( subheap, pArena ); - subheap = newsubheap; - pArena = pInUse; + return STATUS_SUCCESS; } } else
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index eb45a94da95..3a1a4268683 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -815,17 +815,17 @@ static void HEAP_MakeInUseBlockFree( SUBHEAP *subheap, ARENA_INUSE *pArena ) */ static void HEAP_ShrinkBlock(SUBHEAP *subheap, ARENA_INUSE *pArena, SIZE_T size) { - if ((pArena->size & ARENA_SIZE_MASK) >= size + HEAP_MIN_SHRINK_SIZE) + SIZE_T old_data_size = pArena->size & ARENA_SIZE_MASK; + if (old_data_size >= size + HEAP_MIN_SHRINK_SIZE) { - HEAP_CreateFreeBlock( subheap, (char *)(pArena + 1) + size, - (pArena->size & ARENA_SIZE_MASK) - size ); /* assign size plus previous arena flags */ pArena->size = size | (pArena->size & ~ARENA_SIZE_MASK); + HEAP_CreateFreeBlock( subheap, (char *)(pArena + 1) + size, old_data_size - size ); } else { /* Turn off PREV_FREE flag in next block */ - char *pNext = (char *)(pArena + 1) + (pArena->size & ARENA_SIZE_MASK); + char *pNext = (char *)(pArena + 1) + old_data_size; if (pNext < (char *)subheap->base + subheap->size) *(DWORD *)pNext &= ~ARENA_FLAG_PREV_FREE; }
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 3a1a4268683..f0f8704029c 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -808,26 +808,18 @@ static void HEAP_MakeInUseBlockFree( SUBHEAP *subheap, ARENA_INUSE *pArena ) }
-/*********************************************************************** - * HEAP_ShrinkBlock - * - * Shrink an in-use block. - */ -static void HEAP_ShrinkBlock(SUBHEAP *subheap, ARENA_INUSE *pArena, SIZE_T size) +static void shrink_used_block( SUBHEAP *subheap, struct block *block, SIZE_T data_size ) { - SIZE_T old_data_size = pArena->size & ARENA_SIZE_MASK; - if (old_data_size >= size + HEAP_MIN_SHRINK_SIZE) + SIZE_T old_data_size = block_get_size( block ) - sizeof(*block); + if (old_data_size >= data_size + HEAP_MIN_SHRINK_SIZE) { - /* assign size plus previous arena flags */ - pArena->size = size | (pArena->size & ~ARENA_SIZE_MASK); - HEAP_CreateFreeBlock( subheap, (char *)(pArena + 1) + size, old_data_size - size ); + block->size = data_size | block_get_flags( block ); + HEAP_CreateFreeBlock( subheap, next_block( subheap, block ), old_data_size - data_size ); } else { - /* Turn off PREV_FREE flag in next block */ - char *pNext = (char *)(pArena + 1) + old_data_size; - if (pNext < (char *)subheap->base + subheap->size) - *(DWORD *)pNext &= ~ARENA_FLAG_PREV_FREE; + struct block *next; + if ((next = next_block( subheap, block ))) next->size &= ~ARENA_FLAG_PREV_FREE; } }
@@ -1118,7 +1110,7 @@ static ARENA_FREE *HEAP_FindFreeBlock( HEAP *heap, SIZE_T size, * last free arena in ! * So just one heap struct, one first free arena which will eventually * get used, and a second free arena that might get assigned all remaining - * free space in HEAP_ShrinkBlock() */ + * free space in shrink_used_block() */ total_size = size + ROUND_SIZE(sizeof(SUBHEAP)) + sizeof(ARENA_INUSE) + sizeof(ARENA_FREE); if (total_size < size) return NULL; /* overflow */
@@ -1587,7 +1579,7 @@ static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret
/* Shrink the block */
- HEAP_ShrinkBlock( subheap, pInUse, rounded_size ); + shrink_used_block( subheap, pInUse, rounded_size ); pInUse->unused_bytes = (pInUse->size & ARENA_SIZE_MASK) - size;
notify_alloc( pInUse + 1, size, flags & HEAP_ZERO_MEMORY ); @@ -1695,7 +1687,7 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size pArena->size += block_get_size( next ); if (!HEAP_Commit( subheap, pArena, rounded_size )) return STATUS_NO_MEMORY; notify_realloc( pArena + 1, oldActualSize, size ); - HEAP_ShrinkBlock( subheap, pArena, rounded_size ); + shrink_used_block( subheap, pArena, rounded_size ); } else { @@ -1711,7 +1703,7 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size else { notify_realloc( pArena + 1, oldActualSize, size ); - HEAP_ShrinkBlock( subheap, pArena, rounded_size ); + shrink_used_block( subheap, pArena, rounded_size ); }
pArena->unused_bytes = (pArena->size & ARENA_SIZE_MASK) - size;
From: Rémi Bernon rbernon@codeweavers.com
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index f0f8704029c..44c9ad53196 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -808,17 +808,19 @@ static void HEAP_MakeInUseBlockFree( SUBHEAP *subheap, ARENA_INUSE *pArena ) }
-static void shrink_used_block( SUBHEAP *subheap, struct block *block, SIZE_T data_size ) +static void shrink_used_block( SUBHEAP *subheap, struct block *block, SIZE_T data_size, SIZE_T size ) { SIZE_T old_data_size = block_get_size( block ) - sizeof(*block); if (old_data_size >= data_size + HEAP_MIN_SHRINK_SIZE) { block->size = data_size | block_get_flags( block ); + block->unused_bytes = data_size - size; HEAP_CreateFreeBlock( subheap, next_block( subheap, block ), old_data_size - data_size ); } else { struct block *next; + block->unused_bytes = old_data_size - size; if ((next = next_block( subheap, block ))) next->size &= ~ARENA_FLAG_PREV_FREE; } } @@ -1579,8 +1581,7 @@ static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret
/* Shrink the block */
- shrink_used_block( subheap, pInUse, rounded_size ); - pInUse->unused_bytes = (pInUse->size & ARENA_SIZE_MASK) - size; + shrink_used_block( subheap, pInUse, rounded_size, size );
notify_alloc( pInUse + 1, size, flags & HEAP_ZERO_MEMORY ); initialize_block( pInUse + 1, size, pInUse->unused_bytes, flags ); @@ -1687,7 +1688,7 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size pArena->size += block_get_size( next ); if (!HEAP_Commit( subheap, pArena, rounded_size )) return STATUS_NO_MEMORY; notify_realloc( pArena + 1, oldActualSize, size ); - shrink_used_block( subheap, pArena, rounded_size ); + shrink_used_block( subheap, pArena, rounded_size, size ); } else { @@ -1703,11 +1704,9 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size else { notify_realloc( pArena + 1, oldActualSize, size ); - shrink_used_block( subheap, pArena, rounded_size ); + shrink_used_block( subheap, pArena, rounded_size, size ); }
- pArena->unused_bytes = (pArena->size & ARENA_SIZE_MASK) - size; - /* Clear the extra bytes if needed */
if (size > oldActualSize)
-
Marvin -
Rémi Bernon -
Rémi Bernon (＠rbernon)