June 2022 - Wine-Devel - list.winehq.org

[PATCH 5/5] ntdll: Remove heap pointer from SUBHEAP structure.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 69 +++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index acaf3e53bc5..40c317417ce 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -165,7 +165,7 @@ typedef struct DECLSPEC_ALIGN(ALIGNMENT) tagSUBHEAP SIZE_T block_size; SIZE_T data_size; struct list entry; - struct heap *heap; + void *user_value; struct block block; } SUBHEAP; @@ -623,10 +623,9 @@ static SUBHEAP *find_subheap( const struct heap *heap, const struct block *block } -static inline BOOL subheap_commit( SUBHEAP *subheap, const struct block *block, SIZE_T block_size ) +static inline BOOL subheap_commit( const struct heap *heap, SUBHEAP *subheap, const struct block *block, SIZE_T block_size ) { const char *end = (char *)subheap_base( subheap ) + subheap_size( subheap ), *commit_end; - struct heap *heap = subheap->heap; ULONG flags = heap->flags; SIZE_T size; void *addr; @@ -651,10 +650,9 @@ static inline BOOL subheap_commit( SUBHEAP *subheap, const struct block *block, return TRUE; } -static inline BOOL subheap_decommit( SUBHEAP *subheap, const void *commit_end ) +static inline BOOL subheap_decommit( const struct heap *heap, SUBHEAP *subheap, const void *commit_end ) { char *base = subheap_base( subheap ); - struct heap *heap = subheap->heap; SIZE_T size; void *addr; @@ -676,11 +674,10 @@ static inline BOOL subheap_decommit( SUBHEAP *subheap, const void *commit_end ) } -static void create_free_block( SUBHEAP *subheap, struct block *block, SIZE_T block_size ) +static void create_free_block( struct heap *heap, SUBHEAP *subheap, struct block *block, 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 heap *heap = subheap->heap; DWORD flags = heap->flags; struct block *next; @@ -717,9 +714,8 @@ static void create_free_block( SUBHEAP *subheap, struct block *block, SIZE_T blo } -static void free_used_block( SUBHEAP *subheap, struct block *block ) +static void free_used_block( struct heap *heap, SUBHEAP *subheap, struct block *block ) { - struct heap *heap = subheap->heap; struct entry *entry; SIZE_T block_size; @@ -744,7 +740,7 @@ static void free_used_block( SUBHEAP *subheap, struct block *block ) } else entry = (struct entry *)block; - create_free_block( subheap, block, block_size ); + create_free_block( heap, subheap, block, block_size ); if (next_block( subheap, block )) return; /* not the last block */ if (block == first_block( subheap ) && subheap != &heap->subheap) @@ -760,19 +756,19 @@ static void free_used_block( SUBHEAP *subheap, struct block *block ) else if (!heap->shared) { /* keep room for a full commited block as hysteresis */ - subheap_decommit( subheap, (char *)(entry + 1) + (COMMIT_MASK + 1) ); + subheap_decommit( heap, subheap, (char *)(entry + 1) + (COMMIT_MASK + 1) ); } } -static inline void shrink_used_block( SUBHEAP *subheap, struct block *block, UINT flags, +static inline void shrink_used_block( struct heap *heap, SUBHEAP *subheap, struct block *block, UINT flags, SIZE_T old_block_size, SIZE_T block_size, SIZE_T size ) { if (old_block_size >= block_size + HEAP_MIN_BLOCK_SIZE) { block_set_size( block, flags, block_size ); block->tail_size = block_size - sizeof(*block) - size; - create_free_block( subheap, next_block( subheap, block ), old_block_size - block_size ); + create_free_block( heap, subheap, next_block( subheap, block ), old_block_size - block_size ); } else { @@ -896,9 +892,10 @@ static BOOL validate_large_arena( const struct heap *heap, const ARENA_LARGE *ar /*********************************************************************** * HEAP_CreateSubHeap */ -static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD flags, +static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWORD flags, SIZE_T commitSize, SIZE_T totalSize ) { + struct heap *heap = *heap_ptr; struct entry *pEntry; SIZE_T block_size; SUBHEAP *subheap; @@ -932,7 +929,6 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla /* If this is a secondary subheap, insert it into list */ subheap = address; - subheap->heap = heap; subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); list_add_head( &heap->subheap_list, &subheap->entry ); } @@ -952,7 +948,6 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla list_init( &heap->large_list ); subheap = &heap->subheap; - subheap->heap = heap; subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); list_add_head( &heap->subheap_list, &subheap->entry ); @@ -1000,8 +995,9 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla block_size = subheap_size( subheap ) - subheap_overhead( subheap ); block_size &= ~(ALIGNMENT - 1); - create_free_block( subheap, first_block( subheap ), block_size ); + create_free_block( heap, subheap, first_block( subheap ), block_size ); + *heap_ptr = heap; return subheap; } @@ -1023,7 +1019,7 @@ static struct block *find_free_block( struct heap *heap, SIZE_T block_size, SUBH if (block_get_size( block ) >= block_size) { *subheap = find_subheap( heap, block, FALSE ); - if (!subheap_commit( *subheap, block, block_size )) return NULL; + if (!subheap_commit( heap, *subheap, block, block_size )) return NULL; list_remove( &entry->entry ); return block; } @@ -1041,7 +1037,7 @@ static struct block *find_free_block( struct heap *heap, SIZE_T block_size, SUBH total_size = sizeof(SUBHEAP) + block_size + sizeof(struct entry); if (total_size < block_size) return NULL; /* overflow */ - if ((*subheap = HEAP_CreateSubHeap( heap, NULL, heap->flags, total_size, + if ((*subheap = HEAP_CreateSubHeap( &heap, NULL, heap->flags, total_size, max( heap->grow_size, total_size ) ))) { if (heap->grow_size < 128 * 1024 * 1024) heap->grow_size *= 2; @@ -1050,7 +1046,7 @@ static struct block *find_free_block( struct heap *heap, SIZE_T block_size, SUBH { if (heap->grow_size <= total_size || heap->grow_size <= 4 * 1024 * 1024) return NULL; heap->grow_size /= 2; - *subheap = HEAP_CreateSubHeap( heap, NULL, heap->flags, total_size, + *subheap = HEAP_CreateSubHeap( &heap, NULL, heap->flags, total_size, max( heap->grow_size, total_size ) ); } @@ -1072,11 +1068,10 @@ static BOOL is_valid_free_block( const struct heap *heap, const struct block *bl return FALSE; } -static BOOL validate_free_block( const SUBHEAP *subheap, const struct block *block ) +static BOOL validate_free_block( const struct heap *heap, const SUBHEAP *subheap, const struct block *block ) { const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); const struct entry *entry = (struct entry *)block; - struct heap *heap = subheap->heap; const struct block *prev, *next; DWORD flags = heap->flags; @@ -1126,10 +1121,9 @@ static BOOL validate_free_block( const SUBHEAP *subheap, const struct block *blo } -static BOOL validate_used_block( const SUBHEAP *subheap, const struct block *block ) +static BOOL validate_used_block( const struct heap *heap, const SUBHEAP *subheap, const struct block *block ) { const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); - const struct heap *heap = subheap->heap; DWORD flags = heap->flags; const struct block *next; int i; @@ -1198,7 +1192,7 @@ static BOOL heap_validate_ptr( const struct heap *heap, const void *ptr, SUBHEAP return validate_large_arena( heap, large_arena ); } - return validate_used_block( *subheap, arena ); + return validate_used_block( heap, *subheap, arena ); } static BOOL heap_validate( const struct heap *heap ) @@ -1220,11 +1214,11 @@ static BOOL heap_validate( const struct heap *heap ) { if (block_get_flags( block ) & BLOCK_FLAG_FREE) { - if (!validate_free_block( subheap, block )) return FALSE; + if (!validate_free_block( heap, subheap, block )) return FALSE; } else { - if (!validate_used_block( subheap, block )) return FALSE; + if (!validate_used_block( heap, subheap, block )) return FALSE; } } } @@ -1372,7 +1366,7 @@ static void heap_set_debug_flags( HANDLE handle ) HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T commitSize, PVOID unknown, PRTL_HEAP_DEFINITION definition ) { - struct heap *heap; + struct heap *heap = NULL; SUBHEAP *subheap; /* Allocate the heap block */ @@ -1382,8 +1376,7 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c if (!process_heap || !totalSize || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; if (!totalSize) totalSize = HEAP_DEF_SIZE; - if (!(subheap = HEAP_CreateSubHeap( NULL, addr, flags, commitSize, totalSize ))) return 0; - heap = subheap->heap; + if (!(subheap = HEAP_CreateSubHeap( &heap, addr, flags, commitSize, totalSize ))) return 0; heap_set_debug_flags( heap ); @@ -1441,7 +1434,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE handle ) heap->pending_free = NULL; for (tmp = pending; *tmp && tmp != pending + MAX_FREE_PENDING; ++tmp) if ((subheap = find_subheap( heap, *tmp, FALSE ))) - free_used_block( subheap, *tmp ); + free_used_block( heap, subheap, *tmp ); RtlFreeHeap( handle, 0, pending ); } @@ -1478,7 +1471,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE handle ) return 0; } -static SIZE_T heap_get_block_size( struct heap *heap, ULONG flags, SIZE_T size ) +static SIZE_T heap_get_block_size( const struct heap *heap, ULONG flags, SIZE_T size ) { static const ULONG padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS | HEAP_ADD_USER_INFO; static const ULONG check_flags = HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED | HEAP_CHECKING_ENABLED; @@ -1517,7 +1510,7 @@ static NTSTATUS heap_allocate( struct heap *heap, ULONG flags, SIZE_T size, void old_block_size = block_get_size( block ); block_set_type( block, ARENA_INUSE_MAGIC ); - shrink_used_block( subheap, block, 0, old_block_size, block_size, size ); + shrink_used_block( heap, subheap, block, 0, old_block_size, block_size, size ); initialize_block( block + 1, size, flags ); mark_block_tail( block, flags ); @@ -1558,7 +1551,7 @@ static NTSTATUS heap_free( struct heap *heap, void *ptr ) if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) return STATUS_INVALID_PARAMETER; if (!subheap) free_large_block( heap, ptr ); - else free_used_block( subheap, block ); + else free_used_block( heap, subheap, block ); return STATUS_SUCCESS; } @@ -1621,7 +1614,7 @@ static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE struct entry *entry = (struct entry *)next; list_remove( &entry->entry ); old_block_size += block_get_size( next ); - if (!subheap_commit( subheap, block, block_size )) return STATUS_NO_MEMORY; + if (!subheap_commit( heap, subheap, block, block_size )) return STATUS_NO_MEMORY; } else { @@ -1631,13 +1624,13 @@ static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE memcpy( *ret, block + 1, old_size ); if (flags & HEAP_ZERO_MEMORY) memset( (char *)*ret + old_size, 0, size - old_size ); valgrind_notify_free( ptr ); - free_used_block( subheap, block ); + free_used_block( heap, subheap, block ); return STATUS_SUCCESS; } } valgrind_notify_resize( block + 1, old_size, size ); - shrink_used_block( subheap, block, block_get_flags( block ), old_block_size, block_size, size ); + shrink_used_block( heap, subheap, block, block_get_flags( block ), old_block_size, block_size, size ); if (size > old_size) initialize_block( (char *)(block + 1) + old_size, size - old_size, flags ); mark_block_tail( block, flags ); @@ -1739,7 +1732,7 @@ BOOLEAN WINAPI RtlUnlockHeap( HANDLE handle ) } -static NTSTATUS heap_size( struct heap *heap, const void *ptr, SIZE_T *size ) +static NTSTATUS heap_size( const struct heap *heap, const void *ptr, SIZE_T *size ) { const struct block *block; SUBHEAP *subheap; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

[PATCH 4/5] ntdll: Use block_size and data_size in subheap structure.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Respectively describing allocated size vs commit size, relative to the first subheap block, instead of size / commitSize. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 7e64eea4602..acaf3e53bc5 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -162,10 +162,10 @@ static const SIZE_T free_list_sizes[] = typedef struct DECLSPEC_ALIGN(ALIGNMENT) tagSUBHEAP { SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)]; - SIZE_T size; /* Size of the whole sub-heap */ - SIZE_T commitSize; /* Committed size of the sub-heap */ - struct list entry; /* Entry in sub-heap list */ - struct heap *heap; /* Main heap structure */ + SIZE_T block_size; + SIZE_T data_size; + struct list entry; + struct heap *heap; struct block block; } SUBHEAP; @@ -269,19 +269,25 @@ static inline void *subheap_base( const SUBHEAP *subheap ) return ROUND_ADDR( subheap, COMMIT_MASK ); } -static inline SIZE_T subheap_size( const SUBHEAP *subheap ) +static inline SIZE_T subheap_overhead( const SUBHEAP *subheap ) { - return subheap->size; + return (char *)&subheap->block - (char *)subheap_base( subheap ); } -static inline SIZE_T subheap_overhead( const SUBHEAP *subheap ) +static inline SIZE_T subheap_size( const SUBHEAP *subheap ) { - return (char *)&subheap->block - (char *)subheap_base( subheap ); + return subheap->block_size + subheap_overhead( subheap ); } static inline const void *subheap_commit_end( const SUBHEAP *subheap ) { - return (char *)subheap_base( subheap ) + subheap->commitSize; + return (char *)(subheap + 1) + subheap->data_size; +} + +static void subheap_set_bounds( SUBHEAP *subheap, char *commit_end, char *end ) +{ + subheap->block_size = end - (char *)&subheap->block; + subheap->data_size = commit_end - (char *)(subheap + 1); } static inline void *first_block( const SUBHEAP *subheap ) @@ -304,8 +310,7 @@ static inline struct block *next_block( const SUBHEAP *subheap, const struct blo static inline BOOL check_subheap( const SUBHEAP *subheap ) { - const char *base = ROUND_ADDR( subheap, COMMIT_MASK ); - return contains( base, subheap->size, &subheap->block, base + subheap->commitSize - (char *)&subheap->block ); + return contains( &subheap->block, subheap->block_size, subheap + 1, subheap->data_size ); } static BOOL heap_validate( const struct heap *heap ); @@ -642,7 +647,7 @@ static inline BOOL subheap_commit( SUBHEAP *subheap, const struct block *block, return FALSE; } - subheap->commitSize = (char *)commit_end - (char *)subheap_base( subheap ); + subheap->data_size = (char *)commit_end - (char *)(subheap + 1); return TRUE; } @@ -666,7 +671,7 @@ static inline BOOL subheap_decommit( SUBHEAP *subheap, const void *commit_end ) return FALSE; } - subheap->commitSize = (char *)commit_end - (char *)subheap_base( subheap ); + subheap->data_size = (char *)commit_end - (char *)(subheap + 1); return TRUE; } @@ -928,8 +933,7 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla subheap = address; subheap->heap = heap; - subheap->size = totalSize; - subheap->commitSize = commitSize; + subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); list_add_head( &heap->subheap_list, &subheap->entry ); } else @@ -949,8 +953,7 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla subheap = &heap->subheap; subheap->heap = heap; - subheap->size = totalSize; - subheap->commitSize = commitSize; + subheap_set_bounds( subheap, (char *)address + commitSize, (char *)address + totalSize ); list_add_head( &heap->subheap_list, &subheap->entry ); /* Build the free lists */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

[PATCH 3/5] ntdll: Rename HEAP_GetPtr to unsafe_heap_from_handle.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> And related heap variables to handle and heapPtr to heap. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index d68d8577532..7e64eea4602 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -574,13 +574,7 @@ static const char *debugstr_heap_entry( struct rtl_heap_entry *entry ) entry->Region.dwUnCommittedSize, entry->Region.lpFirstBlock, entry->Region.lpLastBlock ); } -/*********************************************************************** - * HEAP_GetPtr - * RETURNS - * Pointer to the heap - * NULL: Failure - */ -static struct heap *HEAP_GetPtr( HANDLE handle ) +static struct heap *unsafe_heap_from_handle( HANDLE handle ) { struct heap *heap = handle; BOOL valid = TRUE; @@ -1298,7 +1292,7 @@ static DWORD heap_flags_from_global_flag( DWORD flag ) */ static void heap_set_debug_flags( HANDLE handle ) { - struct heap *heap = HEAP_GetPtr( handle ); + struct heap *heap = unsafe_heap_from_handle( handle ); ULONG global_flags = RtlGetNtGlobalFlags(); DWORD flags, force_flags; @@ -1421,16 +1415,17 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c */ HANDLE WINAPI RtlDestroyHeap( HANDLE handle ) { - struct heap *heap = HEAP_GetPtr( handle ); SUBHEAP *subheap, *next; ARENA_LARGE *arena, *arena_next; struct block **pending, **tmp; + struct heap *heap; SIZE_T size; void *addr; TRACE( "handle %p\n", handle ); - if (!heap && handle && (((struct heap *)handle)->flags & HEAP_VALIDATE_PARAMS) && + if (!(heap = unsafe_heap_from_handle( handle )) && handle && + (((struct heap *)handle)->flags & HEAP_VALIDATE_PARAMS) && NtCurrentTeb()->Peb->BeingDebugged) { DbgPrint( "Attempt to destroy an invalid heap\n" ); @@ -1447,7 +1442,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE handle ) RtlFreeHeap( handle, 0, pending ); } - if (heap == process_heap) return heap; /* cannot delete the main process heap */ + if (heap == process_heap) return handle; /* cannot delete the main process heap */ /* remove it from the per-process list */ RtlEnterCriticalSection( &process_heap->cs ); @@ -1536,7 +1531,7 @@ void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE handle, ULONG flags, SIZE void *ptr = NULL; NTSTATUS status; - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) status = STATUS_INVALID_HANDLE; else { @@ -1577,7 +1572,7 @@ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE handle, ULONG flags, void * valgrind_notify_free( ptr ); - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) status = STATUS_INVALID_PARAMETER; else { @@ -1661,7 +1656,7 @@ void *WINAPI RtlReAllocateHeap( HANDLE handle, ULONG flags, void *ptr, SIZE_T si if (!ptr) return NULL; - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) status = STATUS_INVALID_HANDLE; else { @@ -1713,7 +1708,7 @@ ULONG WINAPI RtlCompactHeap( HANDLE handle, ULONG flags ) */ BOOLEAN WINAPI RtlLockHeap( HANDLE handle ) { - struct heap *heap = HEAP_GetPtr( handle ); + struct heap *heap = unsafe_heap_from_handle( handle ); if (!heap) return FALSE; heap_lock( heap, 0 ); return TRUE; @@ -1734,7 +1729,7 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE handle ) */ BOOLEAN WINAPI RtlUnlockHeap( HANDLE handle ) { - struct heap *heap = HEAP_GetPtr( handle ); + struct heap *heap = unsafe_heap_from_handle( handle ); if (!heap) return FALSE; heap_unlock( heap, 0 ); return TRUE; @@ -1766,7 +1761,7 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE handle, ULONG flags, const void *ptr ) struct heap *heap; NTSTATUS status; - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) status = STATUS_INVALID_PARAMETER; else { @@ -1790,7 +1785,7 @@ BOOLEAN WINAPI RtlValidateHeap( HANDLE handle, ULONG flags, const void *ptr ) SUBHEAP *subheap; BOOLEAN ret; - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) ret = FALSE; else { @@ -1914,7 +1909,7 @@ NTSTATUS WINAPI RtlWalkHeap( HANDLE handle, void *entry_ptr ) if (!entry) return STATUS_INVALID_PARAMETER; - if (!(heap = HEAP_GetPtr( handle ))) + if (!(heap = unsafe_heap_from_handle( handle ))) status = STATUS_INVALID_HANDLE; else { @@ -2008,7 +2003,7 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void * *user_value = 0; *user_flags = 0; - if (!(heap = HEAP_GetPtr( handle ))) return TRUE; + if (!(heap = unsafe_heap_from_handle( handle ))) return TRUE; heap_lock( heap, flags ); if ((block = unsafe_block_from_ptr( heap, ptr, &subheap )) && !subheap) @@ -2038,7 +2033,7 @@ BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void TRACE( "handle %p, flags %#x, ptr %p, user_value %p.\n", handle, flags, ptr, user_value ); - if (!(heap = HEAP_GetPtr( handle ))) return TRUE; + if (!(heap = unsafe_heap_from_handle( handle ))) return TRUE; heap_lock( heap, flags ); if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) ret = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

[PATCH 2/5] ntdll: Consistently name heap handles and pointers.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 213 +++++++++++++++++++++++----------------------- 1 file changed, 107 insertions(+), 106 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index f1c1f6c32e8..d68d8577532 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -218,7 +218,7 @@ C_ASSERT( offsetof(struct heap, subheap) <= COMMIT_MASK ); #define HEAP_VALIDATE_PARAMS 0x40000000 #define HEAP_CHECKING_ENABLED 0x80000000 -static struct heap *processHeap; /* main process heap */ +static struct heap *process_heap; /* main process heap */ /* check if memory range a contains memory range b */ static inline BOOL contains( const void *a, SIZE_T a_size, const void *b, SIZE_T b_size ) @@ -580,30 +580,30 @@ static const char *debugstr_heap_entry( struct rtl_heap_entry *entry ) * Pointer to the heap * NULL: Failure */ -static struct heap *HEAP_GetPtr( HANDLE heap ) +static struct heap *HEAP_GetPtr( HANDLE handle ) { - struct heap *heapPtr = heap; + struct heap *heap = handle; BOOL valid = TRUE; - if (!heapPtr || (heapPtr->magic != HEAP_MAGIC)) + if (!heap || (heap->magic != HEAP_MAGIC)) { - ERR("Invalid heap %p!\n", heap ); + ERR( "Invalid handle %p!\n", handle ); return NULL; } - if (heapPtr->flags & HEAP_VALIDATE_ALL) + if (heap->flags & HEAP_VALIDATE_ALL) { - heap_lock( heapPtr, 0 ); - valid = heap_validate( heapPtr ); - heap_unlock( heapPtr, 0 ); + heap_lock( heap, 0 ); + valid = heap_validate( heap ); + heap_unlock( heap, 0 ); if (!valid && TRACE_ON(heap)) { - heap_dump( heapPtr ); + heap_dump( heap ); assert( FALSE ); } } - return valid ? heapPtr : NULL; + return valid ? heap : NULL; } @@ -971,7 +971,7 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla /* Initialize critical section */ - if (!processHeap) /* do it by hand to avoid memory allocations */ + if (!process_heap) /* do it by hand to avoid memory allocations */ { heap->cs.DebugInfo = &process_heap_cs_debug; heap->cs.LockCount = -1; @@ -996,7 +996,7 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD fla NtDuplicateObject( NtCurrentProcess(), sem, NtCurrentProcess(), &sem, 0, 0, DUPLICATE_MAKE_GLOBAL | DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE ); heap->cs.LockSemaphore = sem; - RtlFreeHeap( processHeap, 0, heap->cs.DebugInfo ); + RtlFreeHeap( process_heap, 0, heap->cs.DebugInfo ); heap->cs.DebugInfo = NULL; } } @@ -1381,8 +1381,8 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c /* Allocate the heap block */ flags &= ~(HEAP_TAIL_CHECKING_ENABLED|HEAP_FREE_CHECKING_ENABLED); - if (processHeap) flags |= HEAP_PRIVATE; - if (!processHeap || !totalSize || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; + if (process_heap) flags |= HEAP_PRIVATE; + if (!process_heap || !totalSize || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; if (!totalSize) totalSize = HEAP_DEF_SIZE; if (!(subheap = HEAP_CreateSubHeap( NULL, addr, flags, commitSize, totalSize ))) return 0; @@ -1391,16 +1391,16 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c heap_set_debug_flags( heap ); /* link it into the per-process heap list */ - if (processHeap) + if (process_heap) { - RtlEnterCriticalSection( &processHeap->cs ); - list_add_head( &processHeap->entry, &heap->entry ); - RtlLeaveCriticalSection( &processHeap->cs ); + RtlEnterCriticalSection( &process_heap->cs ); + list_add_head( &process_heap->entry, &heap->entry ); + RtlLeaveCriticalSection( &process_heap->cs ); } else if (!addr) { - processHeap = heap; /* assume the first heap we create is the process main heap */ - list_init( &processHeap->entry ); + process_heap = heap; /* assume the first heap we create is the process main heap */ + list_init( &process_heap->entry ); } return heap; @@ -1419,60 +1419,61 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c * Success: A NULL HANDLE, if heap is NULL or it was destroyed * Failure: The Heap handle, if heap is the process heap. */ -HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) +HANDLE WINAPI RtlDestroyHeap( HANDLE handle ) { - struct heap *heapPtr = HEAP_GetPtr( heap ); + struct heap *heap = HEAP_GetPtr( handle ); SUBHEAP *subheap, *next; ARENA_LARGE *arena, *arena_next; struct block **pending, **tmp; SIZE_T size; void *addr; - TRACE("%p\n", heap ); - if (!heapPtr && heap && (((struct heap *)heap)->flags & HEAP_VALIDATE_PARAMS) && + TRACE( "handle %p\n", handle ); + + if (!heap && handle && (((struct heap *)handle)->flags & HEAP_VALIDATE_PARAMS) && NtCurrentTeb()->Peb->BeingDebugged) { DbgPrint( "Attempt to destroy an invalid heap\n" ); DbgBreakPoint(); } - if (!heapPtr) return heap; + if (!heap) return handle; - if ((pending = heapPtr->pending_free)) + if ((pending = heap->pending_free)) { - heapPtr->pending_free = NULL; + heap->pending_free = NULL; for (tmp = pending; *tmp && tmp != pending + MAX_FREE_PENDING; ++tmp) if ((subheap = find_subheap( heap, *tmp, FALSE ))) free_used_block( subheap, *tmp ); - RtlFreeHeap( heap, 0, pending ); + RtlFreeHeap( handle, 0, pending ); } - if (heap == processHeap) return heap; /* cannot delete the main process heap */ + if (heap == process_heap) return heap; /* cannot delete the main process heap */ /* remove it from the per-process list */ - RtlEnterCriticalSection( &processHeap->cs ); - list_remove( &heapPtr->entry ); - RtlLeaveCriticalSection( &processHeap->cs ); + RtlEnterCriticalSection( &process_heap->cs ); + list_remove( &heap->entry ); + RtlLeaveCriticalSection( &process_heap->cs ); - heapPtr->cs.DebugInfo->Spare[0] = 0; - RtlDeleteCriticalSection( &heapPtr->cs ); + heap->cs.DebugInfo->Spare[0] = 0; + RtlDeleteCriticalSection( &heap->cs ); - LIST_FOR_EACH_ENTRY_SAFE( arena, arena_next, &heapPtr->large_list, ARENA_LARGE, entry ) + LIST_FOR_EACH_ENTRY_SAFE( arena, arena_next, &heap->large_list, ARENA_LARGE, entry ) { list_remove( &arena->entry ); size = 0; addr = arena; NtFreeVirtualMemory( NtCurrentProcess(), &addr, &size, MEM_RELEASE ); } - LIST_FOR_EACH_ENTRY_SAFE( subheap, next, &heapPtr->subheap_list, SUBHEAP, entry ) + LIST_FOR_EACH_ENTRY_SAFE( subheap, next, &heap->subheap_list, SUBHEAP, entry ) { - if (subheap == &heapPtr->subheap) continue; /* do this one last */ + if (subheap == &heap->subheap) continue; /* do this one last */ valgrind_notify_free_all( subheap ); list_remove( &subheap->entry ); size = 0; addr = ROUND_ADDR( subheap, COMMIT_MASK ); NtFreeVirtualMemory( NtCurrentProcess(), &addr, &size, MEM_RELEASE ); } - valgrind_notify_free_all( &heapPtr->subheap ); + valgrind_notify_free_all( &heap->subheap ); size = 0; addr = heap; NtFreeVirtualMemory( NtCurrentProcess(), &addr, &size, MEM_RELEASE ); @@ -1529,25 +1530,25 @@ static NTSTATUS heap_allocate( struct heap *heap, ULONG flags, SIZE_T size, void /*********************************************************************** * RtlAllocateHeap (NTDLL.@) */ -void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T size ) +void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE handle, ULONG flags, SIZE_T size ) { - struct heap *heapPtr; + struct heap *heap; void *ptr = NULL; NTSTATUS status; - if (!(heapPtr = HEAP_GetPtr( heap ))) + if (!(heap = HEAP_GetPtr( handle ))) status = STATUS_INVALID_HANDLE; else { - heap_lock( heapPtr, flags ); - status = heap_allocate( heapPtr, heap_get_flags( heapPtr, flags ), size, &ptr ); - heap_unlock( heapPtr, flags ); + heap_lock( heap, flags ); + status = heap_allocate( heap, heap_get_flags( heap, flags ), size, &ptr ); + heap_unlock( heap, flags ); } if (!status) valgrind_notify_alloc( ptr, size, flags & HEAP_ZERO_MEMORY ); - TRACE( "heap %p, flags %#x, size %#Ix, return %p, status %#x.\n", heap, flags, size, ptr, status ); - heap_set_status( heapPtr, flags, status ); + TRACE( "handle %p, flags %#x, size %#Ix, return %p, status %#x.\n", handle, flags, size, ptr, status ); + heap_set_status( heap, flags, status ); return ptr; } @@ -1567,26 +1568,26 @@ static NTSTATUS heap_free( struct heap *heap, void *ptr ) /*********************************************************************** * RtlFreeHeap (NTDLL.@) */ -BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *ptr ) +BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE handle, ULONG flags, void *ptr ) { - struct heap *heapPtr; + struct heap *heap; NTSTATUS status; if (!ptr) return TRUE; valgrind_notify_free( ptr ); - if (!(heapPtr = HEAP_GetPtr( heap ))) + if (!(heap = HEAP_GetPtr( handle ))) status = STATUS_INVALID_PARAMETER; else { - heap_lock( heapPtr, flags ); - status = heap_free( heapPtr, ptr ); - heap_unlock( heapPtr, flags ); + heap_lock( heap, flags ); + status = heap_free( heap, ptr ); + heap_unlock( heap, flags ); } - TRACE( "heap %p, flags %#x, ptr %p, return %u, status %#x.\n", heap, flags, ptr, !status, status ); - heap_set_status( heapPtr, flags, status ); + TRACE( "handle %p, flags %#x, ptr %p, return %u, status %#x.\n", handle, flags, ptr, !status, status ); + heap_set_status( heap, flags, status ); return !status; } @@ -1652,24 +1653,24 @@ static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE /*********************************************************************** * RtlReAllocateHeap (NTDLL.@) */ -void *WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, void *ptr, SIZE_T size ) +void *WINAPI RtlReAllocateHeap( HANDLE handle, ULONG flags, void *ptr, SIZE_T size ) { - struct heap *heapPtr; + struct heap *heap; void *ret = NULL; NTSTATUS status; if (!ptr) return NULL; - if (!(heapPtr = HEAP_GetPtr( heap ))) + if (!(heap = HEAP_GetPtr( handle ))) status = STATUS_INVALID_HANDLE; else { - heap_lock( heapPtr, flags ); - status = heap_reallocate( heapPtr, heap_get_flags( heapPtr, flags ), ptr, size, &ret ); - heap_unlock( heapPtr, flags ); + heap_lock( heap, flags ); + status = heap_reallocate( heap, heap_get_flags( heap, flags ), ptr, size, &ret ); + heap_unlock( heap, flags ); } - TRACE( "heap %p, flags %#x, ptr %p, size %#Ix, return %p, status %#x.\n", heap, flags, ptr, size, ret, status ); + TRACE( "handle %p, flags %#x, ptr %p, size %#Ix, return %p, status %#x.\n", handle, flags, ptr, size, ret, status ); heap_set_status( heap, flags, status ); return ret; } @@ -1690,10 +1691,10 @@ void *WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, void *ptr, SIZE_T size * NOTES * This function is a harmless stub. */ -ULONG WINAPI RtlCompactHeap( HANDLE heap, ULONG flags ) +ULONG WINAPI RtlCompactHeap( HANDLE handle, ULONG flags ) { static BOOL reported; - if (!reported++) FIXME( "(%p, 0x%x) stub\n", heap, flags ); + if (!reported++) FIXME( "handle %p, flags %#x stub!\n", handle, flags ); return 0; } @@ -1710,11 +1711,11 @@ ULONG WINAPI RtlCompactHeap( HANDLE heap, ULONG flags ) * Success: TRUE. The Heap is locked. * Failure: FALSE, if heap is invalid. */ -BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) +BOOLEAN WINAPI RtlLockHeap( HANDLE handle ) { - struct heap *heapPtr = HEAP_GetPtr( heap ); - if (!heapPtr) return FALSE; - heap_lock( heapPtr, 0 ); + struct heap *heap = HEAP_GetPtr( handle ); + if (!heap) return FALSE; + heap_lock( heap, 0 ); return TRUE; } @@ -1731,11 +1732,11 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) * Success: TRUE. The Heap is unlocked. * Failure: FALSE, if heap is invalid. */ -BOOLEAN WINAPI RtlUnlockHeap( HANDLE heap ) +BOOLEAN WINAPI RtlUnlockHeap( HANDLE handle ) { - struct heap *heapPtr = HEAP_GetPtr( heap ); - if (!heapPtr) return FALSE; - heap_unlock( heapPtr, 0 ); + struct heap *heap = HEAP_GetPtr( handle ); + if (!heap) return FALSE; + heap_unlock( heap, 0 ); return TRUE; } @@ -1759,23 +1760,23 @@ static NTSTATUS heap_size( struct heap *heap, const void *ptr, SIZE_T *size ) /*********************************************************************** * RtlSizeHeap (NTDLL.@) */ -SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) +SIZE_T WINAPI RtlSizeHeap( HANDLE handle, ULONG flags, const void *ptr ) { SIZE_T size = ~(SIZE_T)0; - struct heap *heapPtr; + struct heap *heap; NTSTATUS status; - if (!(heapPtr = HEAP_GetPtr( heap ))) + if (!(heap = HEAP_GetPtr( handle ))) status = STATUS_INVALID_PARAMETER; else { - heap_lock( heapPtr, flags ); - status = heap_size( heapPtr, ptr, &size ); - heap_unlock( heapPtr, flags ); + heap_lock( heap, flags ); + status = heap_size( heap, ptr, &size ); + heap_unlock( heap, flags ); } - TRACE( "heap %p, flags %#x, ptr %p, return %#Ix, status %#x.\n", heap, flags, ptr, size, status ); - heap_set_status( heapPtr, flags, status ); + TRACE( "handle %p, flags %#x, ptr %p, return %#Ix, status %#x.\n", handle, flags, ptr, size, status ); + heap_set_status( heap, flags, status ); return size; } @@ -1783,23 +1784,23 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) /*********************************************************************** * RtlValidateHeap (NTDLL.@) */ -BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) +BOOLEAN WINAPI RtlValidateHeap( HANDLE handle, ULONG flags, const void *ptr ) { - struct heap *heapPtr; + struct heap *heap; SUBHEAP *subheap; BOOLEAN ret; - if (!(heapPtr = HEAP_GetPtr( heap ))) + if (!(heap = HEAP_GetPtr( handle ))) ret = FALSE; else { - heap_lock( heapPtr, flags ); - if (ptr) ret = heap_validate_ptr( heapPtr, ptr, &subheap ); - else ret = heap_validate( heapPtr ); - heap_unlock( heapPtr, flags ); + heap_lock( heap, flags ); + if (ptr) ret = heap_validate_ptr( heap, ptr, &subheap ); + else ret = heap_validate( heap ); + heap_unlock( heap, flags ); } - TRACE( "heap %p, flags %#x, ptr %p, return %u.\n", heap, flags, ptr, !!ret ); + TRACE( "handle %p, flags %#x, ptr %p, return %u.\n", handle, flags, ptr, !!ret ); return ret; } @@ -1905,24 +1906,24 @@ static NTSTATUS heap_walk( const struct heap *heap, struct rtl_heap_entry *entry /*********************************************************************** * RtlWalkHeap (NTDLL.@) */ -NTSTATUS WINAPI RtlWalkHeap( HANDLE heap, void *entry_ptr ) +NTSTATUS WINAPI RtlWalkHeap( HANDLE handle, void *entry_ptr ) { struct rtl_heap_entry *entry = entry_ptr; - struct heap *heapPtr; + struct heap *heap; NTSTATUS status; if (!entry) return STATUS_INVALID_PARAMETER; - if (!(heapPtr = HEAP_GetPtr(heap))) + if (!(heap = HEAP_GetPtr( handle ))) status = STATUS_INVALID_HANDLE; else { - heap_lock( heapPtr, 0 ); - status = heap_walk( heapPtr, entry ); - heap_unlock( heapPtr, 0 ); + heap_lock( heap, 0 ); + status = heap_walk( heap, entry ); + heap_unlock( heap, 0 ); } - TRACE( "heap %p, entry %p %s, return %#x\n", heap, entry, + TRACE( "handle %p, entry %p %s, return %#x\n", handle, entry, status ? "<empty>" : debugstr_heap_entry(entry), status ); return status; } @@ -1946,23 +1947,23 @@ ULONG WINAPI RtlGetProcessHeaps( ULONG count, HANDLE *heaps ) ULONG total = 1; /* main heap */ struct list *ptr; - RtlEnterCriticalSection( &processHeap->cs ); - LIST_FOR_EACH( ptr, &processHeap->entry ) total++; + RtlEnterCriticalSection( &process_heap->cs ); + LIST_FOR_EACH( ptr, &process_heap->entry ) total++; if (total <= count) { - *heaps++ = processHeap; - LIST_FOR_EACH( ptr, &processHeap->entry ) + *heaps++ = process_heap; + LIST_FOR_EACH( ptr, &process_heap->entry ) *heaps++ = LIST_ENTRY( ptr, struct heap, entry ); } - RtlLeaveCriticalSection( &processHeap->cs ); + RtlLeaveCriticalSection( &process_heap->cs ); return total; } /*********************************************************************** * RtlQueryHeapInformation (NTDLL.@) */ -NTSTATUS WINAPI RtlQueryHeapInformation( HANDLE heap, HEAP_INFORMATION_CLASS info_class, - PVOID info, SIZE_T size_in, PSIZE_T size_out) +NTSTATUS WINAPI RtlQueryHeapInformation( HANDLE handle, HEAP_INFORMATION_CLASS info_class, + void *info, SIZE_T size_in, PSIZE_T size_out ) { switch (info_class) { @@ -1984,9 +1985,9 @@ NTSTATUS WINAPI RtlQueryHeapInformation( HANDLE heap, HEAP_INFORMATION_CLASS inf /*********************************************************************** * RtlSetHeapInformation (NTDLL.@) */ -NTSTATUS WINAPI RtlSetHeapInformation( HANDLE heap, HEAP_INFORMATION_CLASS info_class, PVOID info, SIZE_T size) +NTSTATUS WINAPI RtlSetHeapInformation( HANDLE handle, HEAP_INFORMATION_CLASS info_class, void *info, SIZE_T size ) { - FIXME("%p %d %p %ld stub\n", heap, info_class, info, size); + FIXME( "handle %p, info_class %d, info %p, size %ld stub!\n", handle, info_class, info, size ); return STATUS_SUCCESS; } @@ -2056,8 +2057,8 @@ BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void /*********************************************************************** * RtlSetUserFlagsHeap (NTDLL.@) */ -BOOLEAN WINAPI RtlSetUserFlagsHeap( HANDLE heap, ULONG flags, void *ptr, ULONG clear, ULONG set ) +BOOLEAN WINAPI RtlSetUserFlagsHeap( HANDLE handle, ULONG flags, void *ptr, ULONG clear, ULONG set ) { - FIXME( "heap %p, flags %#x, ptr %p, clear %#x, set %#x stub!\n", heap, flags, ptr, clear, set ); + FIXME( "handle %p, flags %#x, ptr %p, clear %#x, set %#x stub!\n", handle, flags, ptr, clear, set ); return FALSE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

[PATCH 1/5] ntdll: Rename HEAP to struct heap and remove typedef.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/heap.c | 119 ++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 61 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 8ebd561dd1c..f1c1f6c32e8 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -159,15 +159,13 @@ static const SIZE_T free_list_sizes[] = }; #define HEAP_NB_FREE_LISTS (ARRAY_SIZE(free_list_sizes) + HEAP_NB_SMALL_FREE_LISTS) -struct tagHEAP; - typedef struct DECLSPEC_ALIGN(ALIGNMENT) tagSUBHEAP { SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)]; SIZE_T size; /* Size of the whole sub-heap */ SIZE_T commitSize; /* Committed size of the sub-heap */ struct list entry; /* Entry in sub-heap list */ - struct tagHEAP *heap; /* Main heap structure */ + struct heap *heap; /* Main heap structure */ struct block block; } SUBHEAP; @@ -175,7 +173,7 @@ typedef struct DECLSPEC_ALIGN(ALIGNMENT) tagSUBHEAP C_ASSERT( sizeof(SUBHEAP) == offsetof(SUBHEAP, block) + sizeof(struct block) ); C_ASSERT( sizeof(SUBHEAP) == 4 * ALIGNMENT ); -typedef struct tagHEAP +struct heap { /* win32/win64 */ DWORD_PTR unknown1[2]; /* 0000/0000 */ DWORD ffeeffee; /* 0008/0010 */ @@ -199,12 +197,12 @@ typedef struct tagHEAP RTL_CRITICAL_SECTION cs; struct entry free_lists[HEAP_NB_FREE_LISTS]; SUBHEAP subheap; -} HEAP; +}; /* subheap must be last and aligned */ -C_ASSERT( sizeof(HEAP) == offsetof(HEAP, subheap) + sizeof(SUBHEAP) ); -C_ASSERT( sizeof(HEAP) % ALIGNMENT == 0 ); -C_ASSERT( offsetof(HEAP, subheap) <= COMMIT_MASK ); +C_ASSERT( sizeof(struct heap) == offsetof(struct heap, subheap) + sizeof(SUBHEAP) ); +C_ASSERT( sizeof(struct heap) % ALIGNMENT == 0 ); +C_ASSERT( offsetof(struct heap, subheap) <= COMMIT_MASK ); #define HEAP_MAGIC ((DWORD)('H' | ('E'<<8) | ('A'<<16) | ('P'<<24))) @@ -220,7 +218,7 @@ C_ASSERT( offsetof(HEAP, subheap) <= COMMIT_MASK ); #define HEAP_VALIDATE_PARAMS 0x40000000 #define HEAP_CHECKING_ENABLED 0x80000000 -static HEAP *processHeap; /* main process heap */ +static struct heap *processHeap; /* main process heap */ /* check if memory range a contains memory range b */ static inline BOOL contains( const void *a, SIZE_T a_size, const void *b, SIZE_T b_size ) @@ -310,7 +308,7 @@ static inline BOOL check_subheap( const SUBHEAP *subheap ) return contains( base, subheap->size, &subheap->block, base + subheap->commitSize - (char *)&subheap->block ); } -static BOOL heap_validate( const HEAP *heap ); +static BOOL heap_validate( const struct heap *heap ); /* mark a block of memory as innacessible for debugging purposes */ static inline void valgrind_make_noaccess( void const *ptr, SIZE_T size ) @@ -428,7 +426,7 @@ static void valgrind_notify_free_all( SUBHEAP *subheap ) /* locate a free list entry of the appropriate size */ /* size is the size of the whole block including the arena header */ -static inline struct entry *find_free_list( HEAP *heap, SIZE_T block_size, BOOL last ) +static inline struct entry *find_free_list( struct heap *heap, SIZE_T block_size, BOOL last ) { struct entry *list, *end = heap->free_lists + ARRAY_SIZE(heap->free_lists); unsigned int i; @@ -456,32 +454,32 @@ static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug = 0, 0, { (DWORD_PTR)(__FILE__ ": main process heap section") } }; -static inline ULONG heap_get_flags( const HEAP *heap, ULONG flags ) +static inline ULONG heap_get_flags( const struct heap *heap, ULONG flags ) { if (flags & (HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED)) flags |= HEAP_CHECKING_ENABLED; flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY | HEAP_CHECKING_ENABLED | HEAP_ADD_USER_INFO; return heap->flags | flags; } -static void heap_lock( HEAP *heap, ULONG flags ) +static void heap_lock( struct heap *heap, ULONG flags ) { if (heap_get_flags( heap, flags ) & HEAP_NO_SERIALIZE) return; RtlEnterCriticalSection( &heap->cs ); } -static void heap_unlock( HEAP *heap, ULONG flags ) +static void heap_unlock( struct heap *heap, ULONG flags ) { if (heap_get_flags( heap, flags ) & HEAP_NO_SERIALIZE) return; RtlLeaveCriticalSection( &heap->cs ); } -static void heap_set_status( const HEAP *heap, ULONG flags, NTSTATUS status ) +static void heap_set_status( const struct heap *heap, ULONG flags, NTSTATUS status ) { if (status == STATUS_NO_MEMORY && (flags & HEAP_GENERATE_EXCEPTIONS)) RtlRaiseStatus( status ); if (status) RtlSetLastWin32ErrorAndNtStatusFromNtStatus( status ); } -static void heap_dump( const HEAP *heap ) +static void heap_dump( const struct heap *heap ) { const struct block *block; const ARENA_LARGE *large; @@ -490,7 +488,7 @@ static void heap_dump( const HEAP *heap ) SIZE_T size; TRACE( "heap: %p\n", heap ); - TRACE( " next %p\n", LIST_ENTRY( heap->entry.next, HEAP, entry ) ); + TRACE( " next %p\n", LIST_ENTRY( heap->entry.next, struct heap, entry ) ); TRACE( " free_lists: %p\n", heap->free_lists ); for (i = 0; i < HEAP_NB_FREE_LISTS; i++) @@ -582,10 +580,9 @@ static const char *debugstr_heap_entry( struct rtl_heap_entry *entry ) * Pointer to the heap * NULL: Failure */ -static HEAP *HEAP_GetPtr( - HANDLE heap /* [in] Handle to the heap */ -) { - HEAP *heapPtr = heap; +static struct heap *HEAP_GetPtr( HANDLE heap ) +{ + struct heap *heapPtr = heap; BOOL valid = TRUE; if (!heapPtr || (heapPtr->magic != HEAP_MAGIC)) @@ -610,7 +607,7 @@ static HEAP *HEAP_GetPtr( } -static SUBHEAP *find_subheap( const HEAP *heap, const struct block *block, BOOL heap_walk ) +static SUBHEAP *find_subheap( const struct heap *heap, const struct block *block, BOOL heap_walk ) { SUBHEAP *subheap; @@ -630,7 +627,7 @@ static SUBHEAP *find_subheap( const HEAP *heap, const struct block *block, BOOL static inline BOOL subheap_commit( SUBHEAP *subheap, const struct block *block, SIZE_T block_size ) { const char *end = (char *)subheap_base( subheap ) + subheap_size( subheap ), *commit_end; - HEAP *heap = subheap->heap; + struct heap *heap = subheap->heap; ULONG flags = heap->flags; SIZE_T size; void *addr; @@ -658,7 +655,7 @@ static inline BOOL subheap_commit( SUBHEAP *subheap, const struct block *block, static inline BOOL subheap_decommit( SUBHEAP *subheap, const void *commit_end ) { char *base = subheap_base( subheap ); - HEAP *heap = subheap->heap; + struct heap *heap = subheap->heap; SIZE_T size; void *addr; @@ -684,7 +681,7 @@ static void create_free_block( SUBHEAP *subheap, struct block *block, SIZE_T blo { const char *end = (char *)block + block_size, *commit_end = subheap_commit_end( subheap ); struct entry *entry = (struct entry *)block, *list; - HEAP *heap = subheap->heap; + struct heap *heap = subheap->heap; DWORD flags = heap->flags; struct block *next; @@ -723,7 +720,7 @@ static void create_free_block( SUBHEAP *subheap, struct block *block, SIZE_T blo static void free_used_block( SUBHEAP *subheap, struct block *block ) { - HEAP *heap = subheap->heap; + struct heap *heap = subheap->heap; struct entry *entry; SIZE_T block_size; @@ -791,7 +788,7 @@ static inline void shrink_used_block( SUBHEAP *subheap, struct block *block, UIN /*********************************************************************** * allocate_large_block */ -static void *allocate_large_block( HEAP *heap, DWORD flags, SIZE_T size ) +static void *allocate_large_block( struct heap *heap, DWORD flags, SIZE_T size ) { ARENA_LARGE *arena; SIZE_T block_size = ROUND_SIZE( sizeof(*arena) + size, COMMIT_MASK ); @@ -820,7 +817,7 @@ static void *allocate_large_block( HEAP *heap, DWORD flags, SIZE_T size ) /*********************************************************************** * free_large_block */ -static void free_large_block( HEAP *heap, void *ptr ) +static void free_large_block( struct heap *heap, void *ptr ) { ARENA_LARGE *arena = (ARENA_LARGE *)ptr - 1; LPVOID address = arena; @@ -834,7 +831,7 @@ static void free_large_block( HEAP *heap, void *ptr ) /*********************************************************************** * realloc_large_block */ -static void *realloc_large_block( HEAP *heap, DWORD flags, void *ptr, SIZE_T size ) +static void *realloc_large_block( struct heap *heap, DWORD flags, void *ptr, SIZE_T size ) { ARENA_LARGE *arena = (ARENA_LARGE *)ptr - 1; SIZE_T old_size = arena->data_size; @@ -867,7 +864,7 @@ static void *realloc_large_block( HEAP *heap, DWORD flags, void *ptr, SIZE_T siz /*********************************************************************** * find_large_block */ -static ARENA_LARGE *find_large_block( const HEAP *heap, const void *ptr ) +static ARENA_LARGE *find_large_block( const struct heap *heap, const void *ptr ) { ARENA_LARGE *arena; @@ -877,7 +874,7 @@ static ARENA_LARGE *find_large_block( const HEAP *heap, const void *ptr ) return NULL; } -static BOOL validate_large_arena( const HEAP *heap, const ARENA_LARGE *arena ) +static BOOL validate_large_arena( const struct heap *heap, const ARENA_LARGE *arena ) { const char *err = NULL; @@ -900,7 +897,7 @@ static BOOL validate_large_arena( const HEAP *heap, const ARENA_LARGE *arena ) /*********************************************************************** * HEAP_CreateSubHeap */ -static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags, +static SUBHEAP *HEAP_CreateSubHeap( struct heap *heap, LPVOID address, DWORD flags, SIZE_T commitSize, SIZE_T totalSize ) { struct entry *pEntry; @@ -1012,7 +1009,7 @@ static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags, } -static struct block *find_free_block( HEAP *heap, SIZE_T block_size, SUBHEAP **subheap ) +static struct block *find_free_block( struct heap *heap, SIZE_T block_size, SUBHEAP **subheap ) { struct list *ptr = &find_free_list( heap, block_size, FALSE )->entry; struct entry *entry; @@ -1068,7 +1065,7 @@ static struct block *find_free_block( HEAP *heap, SIZE_T block_size, SUBHEAP **s } -static BOOL is_valid_free_block( const HEAP *heap, const struct block *block ) +static BOOL is_valid_free_block( const struct heap *heap, const struct block *block ) { const SUBHEAP *subheap; unsigned int i; @@ -1082,8 +1079,8 @@ static BOOL validate_free_block( const SUBHEAP *subheap, const struct block *blo { const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); const struct entry *entry = (struct entry *)block; + struct heap *heap = subheap->heap; const struct block *prev, *next; - HEAP *heap = subheap->heap; DWORD flags = heap->flags; if ((ULONG_PTR)(block + 1) % ALIGNMENT) @@ -1135,7 +1132,7 @@ static BOOL validate_free_block( const SUBHEAP *subheap, const struct block *blo static BOOL validate_used_block( const SUBHEAP *subheap, const struct block *block ) { const char *err = NULL, *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ); - const HEAP *heap = subheap->heap; + const struct heap *heap = subheap->heap; DWORD flags = heap->flags; const struct block *next; int i; @@ -1188,7 +1185,7 @@ static BOOL validate_used_block( const SUBHEAP *subheap, const struct block *blo } -static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subheap ) +static BOOL heap_validate_ptr( const struct heap *heap, const void *ptr, SUBHEAP **subheap ) { const struct block *arena = (struct block *)ptr - 1; const ARENA_LARGE *large_arena; @@ -1207,7 +1204,7 @@ static BOOL heap_validate_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subh return validate_used_block( *subheap, arena ); } -static BOOL heap_validate( const HEAP *heap ) +static BOOL heap_validate( const struct heap *heap ) { const ARENA_LARGE *large_arena; const struct block *block; @@ -1241,7 +1238,7 @@ static BOOL heap_validate( const HEAP *heap ) return TRUE; } -static inline struct block *unsafe_block_from_ptr( const HEAP *heap, const void *ptr, SUBHEAP **subheap ) +static inline struct block *unsafe_block_from_ptr( const struct heap *heap, const void *ptr, SUBHEAP **subheap ) { struct block *block = (struct block *)ptr - 1; const char *err = NULL, *base, *commit_end; @@ -1301,7 +1298,7 @@ static DWORD heap_flags_from_global_flag( DWORD flag ) */ static void heap_set_debug_flags( HANDLE handle ) { - HEAP *heap = HEAP_GetPtr( handle ); + struct heap *heap = HEAP_GetPtr( handle ); ULONG global_flags = RtlGetNtGlobalFlags(); DWORD flags, force_flags; @@ -1378,8 +1375,8 @@ static void heap_set_debug_flags( HANDLE handle ) HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T commitSize, PVOID unknown, PRTL_HEAP_DEFINITION definition ) { + struct heap *heap; SUBHEAP *subheap; - HEAP *heap; /* Allocate the heap block */ @@ -1424,7 +1421,7 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c */ HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) { - HEAP *heapPtr = HEAP_GetPtr( heap ); + struct heap *heapPtr = HEAP_GetPtr( heap ); SUBHEAP *subheap, *next; ARENA_LARGE *arena, *arena_next; struct block **pending, **tmp; @@ -1432,7 +1429,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) void *addr; TRACE("%p\n", heap ); - if (!heapPtr && heap && (((HEAP *)heap)->flags & HEAP_VALIDATE_PARAMS) && + if (!heapPtr && heap && (((struct heap *)heap)->flags & HEAP_VALIDATE_PARAMS) && NtCurrentTeb()->Peb->BeingDebugged) { DbgPrint( "Attempt to destroy an invalid heap\n" ); @@ -1482,7 +1479,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) return 0; } -static SIZE_T heap_get_block_size( HEAP *heap, ULONG flags, SIZE_T size ) +static SIZE_T heap_get_block_size( struct heap *heap, ULONG flags, SIZE_T size ) { static const ULONG padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS | HEAP_ADD_USER_INFO; static const ULONG check_flags = HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED | HEAP_CHECKING_ENABLED; @@ -1498,7 +1495,7 @@ static SIZE_T heap_get_block_size( HEAP *heap, ULONG flags, SIZE_T size ) return ROUND_SIZE( size + overhead, ALIGNMENT - 1 ); } -static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret ) +static NTSTATUS heap_allocate( struct heap *heap, ULONG flags, SIZE_T size, void **ret ) { SIZE_T old_block_size, block_size; struct block *block; @@ -1534,9 +1531,9 @@ static NTSTATUS heap_allocate( HEAP *heap, ULONG flags, SIZE_T size, void **ret */ void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T size ) { + struct heap *heapPtr; void *ptr = NULL; NTSTATUS status; - HEAP *heapPtr; if (!(heapPtr = HEAP_GetPtr( heap ))) status = STATUS_INVALID_HANDLE; @@ -1555,7 +1552,7 @@ void *WINAPI DECLSPEC_HOTPATCH RtlAllocateHeap( HANDLE heap, ULONG flags, SIZE_T } -static NTSTATUS heap_free( HEAP *heap, void *ptr ) +static NTSTATUS heap_free( struct heap *heap, void *ptr ) { struct block *block; SUBHEAP *subheap; @@ -1572,8 +1569,8 @@ static NTSTATUS heap_free( HEAP *heap, void *ptr ) */ BOOLEAN WINAPI DECLSPEC_HOTPATCH RtlFreeHeap( HANDLE heap, ULONG flags, void *ptr ) { + struct heap *heapPtr; NTSTATUS status; - HEAP *heapPtr; if (!ptr) return TRUE; @@ -1594,7 +1591,7 @@ 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 ) +static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE_T size, void **ret ) { SIZE_T old_block_size, old_size, block_size; struct block *next, *block; @@ -1657,9 +1654,9 @@ static NTSTATUS heap_reallocate( HEAP *heap, ULONG flags, void *ptr, SIZE_T size */ void *WINAPI RtlReAllocateHeap( HANDLE heap, ULONG flags, void *ptr, SIZE_T size ) { + struct heap *heapPtr; void *ret = NULL; NTSTATUS status; - HEAP *heapPtr; if (!ptr) return NULL; @@ -1715,7 +1712,7 @@ ULONG WINAPI RtlCompactHeap( HANDLE heap, ULONG flags ) */ BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) { - HEAP *heapPtr = HEAP_GetPtr( heap ); + struct heap *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; heap_lock( heapPtr, 0 ); return TRUE; @@ -1736,14 +1733,14 @@ BOOLEAN WINAPI RtlLockHeap( HANDLE heap ) */ BOOLEAN WINAPI RtlUnlockHeap( HANDLE heap ) { - HEAP *heapPtr = HEAP_GetPtr( heap ); + struct heap *heapPtr = HEAP_GetPtr( heap ); if (!heapPtr) return FALSE; heap_unlock( heapPtr, 0 ); return TRUE; } -static NTSTATUS heap_size( HEAP *heap, const void *ptr, SIZE_T *size ) +static NTSTATUS heap_size( struct heap *heap, const void *ptr, SIZE_T *size ) { const struct block *block; SUBHEAP *subheap; @@ -1765,8 +1762,8 @@ static NTSTATUS heap_size( HEAP *heap, const void *ptr, SIZE_T *size ) SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) { SIZE_T size = ~(SIZE_T)0; + struct heap *heapPtr; NTSTATUS status; - HEAP *heapPtr; if (!(heapPtr = HEAP_GetPtr( heap ))) status = STATUS_INVALID_PARAMETER; @@ -1788,8 +1785,8 @@ SIZE_T WINAPI RtlSizeHeap( HANDLE heap, ULONG flags, const void *ptr ) */ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) { + struct heap *heapPtr; SUBHEAP *subheap; - HEAP *heapPtr; BOOLEAN ret; if (!(heapPtr = HEAP_GetPtr( heap ))) @@ -1807,7 +1804,7 @@ BOOLEAN WINAPI RtlValidateHeap( HANDLE heap, ULONG flags, const void *ptr ) } -static NTSTATUS heap_walk_blocks( const HEAP *heap, const SUBHEAP *subheap, struct rtl_heap_entry *entry ) +static NTSTATUS heap_walk_blocks( const struct heap *heap, const SUBHEAP *subheap, struct rtl_heap_entry *entry ) { const char *base = subheap_base( subheap ), *commit_end = subheap_commit_end( subheap ), *end = base + subheap_size( subheap ); const struct block *block, *blocks = first_block( subheap ); @@ -1852,7 +1849,7 @@ static NTSTATUS heap_walk_blocks( const HEAP *heap, const SUBHEAP *subheap, stru return STATUS_SUCCESS; } -static NTSTATUS heap_walk( const HEAP *heap, struct rtl_heap_entry *entry ) +static NTSTATUS heap_walk( const struct heap *heap, struct rtl_heap_entry *entry ) { const ARENA_LARGE *large; const struct list *next; @@ -1911,8 +1908,8 @@ static NTSTATUS heap_walk( const HEAP *heap, struct rtl_heap_entry *entry ) NTSTATUS WINAPI RtlWalkHeap( HANDLE heap, void *entry_ptr ) { struct rtl_heap_entry *entry = entry_ptr; + struct heap *heapPtr; NTSTATUS status; - HEAP *heapPtr; if (!entry) return STATUS_INVALID_PARAMETER; @@ -1955,7 +1952,7 @@ ULONG WINAPI RtlGetProcessHeaps( ULONG count, HANDLE *heaps ) { *heaps++ = processHeap; LIST_FOR_EACH( ptr, &processHeap->entry ) - *heaps++ = LIST_ENTRY( ptr, HEAP, entry ); + *heaps++ = LIST_ENTRY( ptr, struct heap, entry ); } RtlLeaveCriticalSection( &processHeap->cs ); return total; @@ -2000,8 +1997,8 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void * { ARENA_LARGE *large = (ARENA_LARGE *)ptr - 1; struct block *block; + struct heap *heap; SUBHEAP *subheap; - HEAP *heap; char *tmp; TRACE( "handle %p, flags %#x, ptr %p, user_value %p, user_flags %p semi-stub!\n", @@ -2034,8 +2031,8 @@ BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void ARENA_LARGE *large = (ARENA_LARGE *)ptr - 1; struct block *block; BOOLEAN ret = TRUE; + struct heap *heap; SUBHEAP *subheap; - HEAP *heap; char *tmp; TRACE( "handle %p, flags %#x, ptr %p, user_value %p.\n", handle, flags, ptr, user_value ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

[PATCH 0/5] MR235: ntdll: Preparation for heap region factorization.

by Rémi Bernon

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/235

June 13, 2022

Re: HLSL offsetting

by Francisco Casas

Hello, >> Functions that cannot receive structs or arrays may receive a >> "flattened" component offset that can then be translated into a route, >> other functions would require the route as an array. > > Not immediately sure what functions you're thinking of, but I imagine > things like hlsl_compute_component_offset() would now have to translate > the offset into an array. > For instance, hlsl_new_load(), hlsl_new_resource_load(), and hlsl_new_store() will have to receive routes instead of single nodes as offsets now. >> At this point I can see the benefits of (b) over (a), but also, >> several complications that may arise (you have pointed most of them): >> - We will have to translate many things that are already in terms of >> register offsets into component offsets. > > How many things, though? As far as I can see it's: > > - copy-prop > > - copy splitting > > - hlsl_offset_from_deref() [which is part of the point of the whole > exercise] > > That's pretty much it. > Well, maybe it is not too much, but we also have to add initializers to the list and everywhere else the previous 3 functions are called. >> - Once we start supporting non-constant offsets, we may also want to >> introduce a common subexpression elimination pass for the registers >> offsets expressions that will arise (currently, the creation of common >> expressions is mainly avoided by the recursive structure of the split >> passes). > > What cases are you thinking of that would want CSE? > Matteo pointed it out. Basically if we have several copies that come from a deep struct, let's say 4-dimension array: float4 arr[10][10][10][10]; Consider the following 2 loads: arr[i][j][k][0] arr[i][j][k][1] The idea is that the common part of the SMx-specific register offsets (4000 * i + 400 * j + 40 * k) is shared among both and not computed twice. >> Solely because I have spent a considerable amount of time implementing >> option (a) (and some time giving up on implementing component offsets >> as a single hlsl_src, instead of a path) I am rushing (a) to see how >> the patch turns out in the end, before trying (b). >> >> I so think that (a) can be cleansed one step at the time. Even if the >> register sizes and offsets depend on the SM, we can write an interface >> to abstract the rest of the code of using them directly, and gradually >> migrate the code that does to use this interface instead. >> >> >> But so far, yeah, I am being convinced that (b) is better, however >> more difficult. >> If we do (b), I suggest we try to do it in separate steps, with some >> "scaffolding code": >> >> - Add the component offset route to hlsl_deref without deleting the >> register offset. >> - Create a simple pass that initializes the register offset field >> using the route in all the hlsl_derefs. >> - Translate all the parse code first to work with the routes, and >> apply the pass just after parsing. >> - Translate some more compilation passes and move the translation pass >> forward. >> - Repeat until all the passes that can be written in terms of >> component offsets are. >> - Write the SMxIR·s and the SMxIR translations. >> - Only then, remove the the register offset from hlsl_deref and the >> translation pass from the codebase. >> >> >> Otherwise we may end up writing a very big patch that may take too >> long to complete (!). > > Ech, I think that trying to do two things at once is going to be more > confusing than the alternative. I also don't think there's *that* much > code that we'd have to change, i.e. a monolithic patch wouldn't be that > bad. > Okay, I stopped working on multiple register offsets and now I am working on this approach. I still think that adding a pass to translate these "route"s to the original register "offset"s, so that we can implement this change gradually, is good. We can move the pass forward as we change more passes to work with component offsets. And we can debug more easily the things that we don't translate correctly. I am aiming to implement the new approach until right after split_matrix_copies, and put the translation afterwards. So far it seems to be going nicely. > For that matter, I'd be happy to try writing those patches myself :-) > Give me a couple of days to finish a patch with the previous idea first. Even if we don't want to introduce the ugliness of this transitional state upstream, it may be a good starting point for the big patch. > ἔρρωσθε, > Zeb

June 13, 2022

[tools 3/3] testbot/WineSendLog: Don't include the test logs in the job report.

by Francois Gouget

The logs can be pretty big, causing the email to exceed typical email size limits and cause bounces. The logs can be downloaded from the website anyway. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineSendLog.pl | 47 +++----------------------------------- 1 file changed, 3 insertions(+), 44 deletions(-) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 8e2048e23..9351d5d6f 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -51,9 +51,6 @@ use WineTestBot::StepsTasks; use WineTestBot::Utils; -my $PART_BOUNDARY = "==13F70BD1-BA1B-449A-9CCB-B6A8E90CED47=="; - - # # Logging and error handling helpers # @@ -216,18 +213,9 @@ sub SendLog($) print $Sendmail "In-Reply-To: ", $Job->Patch->MessageId, "\n"; print $Sendmail "References: ", $Job->Patch->MessageId, "\n"; } - print $Sendmail <<"EOF"; -MIME-Version: 1.0 -Content-Type: multipart/mixed; boundary="$PART_BOUNDARY" - ---$PART_BOUNDARY -Content-Type: text/plain; charset="UTF-8" -MIME-Version: 1.0 -Content-Transfer-Encoding: 8bit -Content-Disposition: inline -VM Status Failures Command -EOF + print $Sendmail "\n"; + print $Sendmail "VM Status Failures Command\n"; foreach my $StepTask (@$SortedStepsTasks) { my $TestFailures = $StepTask->TestFailures; @@ -242,7 +230,7 @@ EOF $TestFailures, $Cmd; } - print $Sendmail "\nYou can also see the results at:\n$JobURL\n\n"; + print $Sendmail "\nThe full results can be found at:\n$JobURL\n\n"; # Print the job summary my $JobErrors; @@ -271,35 +259,6 @@ EOF } } - # Print the log attachments - foreach my $StepTask (@$SortedStepsTasks) - { - my $TaskDir = $StepTask->GetTaskDir(); - - foreach my $LogName (@{$JobErrors->{$StepTask->Id}->{LogNames}}) - { - print $Sendmail <<"EOF"; - ---$PART_BOUNDARY -Content-Type: text/plain; charset="UTF-8" -MIME-Version: 1.0 -Content-Transfer-Encoding: 8bit -EOF - # Prepend the VM name to get unique attachment filenames for this email. - print $Sendmail "Content-Disposition: attachment; filename=", - $StepTask->VM->Name, "-$LogName\n\n"; - if ($Debug) - { - print $Sendmail "Not dumping logs in debug mode\n"; - } - else - { - DumpLogAndErr($Sendmail, "$TaskDir/$LogName"); - } - } - } - - print $Sendmail "\n--$PART_BOUNDARY--\n"; close($Sendmail); # This is all for jobs submitted from the website -- 2.30.2

June 13, 2022

[tools 2/3] testbot/WineSendLog: Use GetSortedItems() to sort the Steps/Tasks.

by Francois Gouget

This avoids having to look up the object in every loop. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineSendLog.pl | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 5affba606..8e2048e23 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -183,7 +183,7 @@ sub SendLog($) } my $StepsTasks = CreateStepsTasks(undef, $Job); - my @SortedKeys = sort { $a <=> $b } @{$StepsTasks->GetKeys()}; + my $SortedStepsTasks = $StepsTasks->GetSortedItems(); my $JobURL = MakeOfficialURL("/JobDetails.pl?Key=". $Job->GetKey()); @@ -228,10 +228,8 @@ Content-Disposition: inline VM Status Failures Command EOF - foreach my $Key (@SortedKeys) + foreach my $StepTask (@$SortedStepsTasks) { - my $StepTask = $StepsTasks->GetItem($Key); - my $TestFailures = $StepTask->TestFailures; $TestFailures = "" if (!defined $TestFailures); my $Status = $StepTask->Status; @@ -248,19 +246,18 @@ EOF # Print the job summary my $JobErrors; - foreach my $Key (@SortedKeys) + foreach my $StepTask (@$SortedStepsTasks) { - my $StepTask = $StepsTasks->GetItem($Key); + my $Id = $StepTask->Id; my $TaskDir = $StepTask->GetTaskDir(); - my $LogNames = GetLogFileNames($TaskDir); - $JobErrors->{$Key}->{LogNames} = $LogNames; + $JobErrors->{$Id}->{LogNames} = $LogNames; foreach my $LogName (@$LogNames) { my $LogInfo = LoadLogErrors("$TaskDir/$LogName"); next if (!defined $LogInfo->{BadLog} and !$LogInfo->{ErrCount}); - $JobErrors->{$Key}->{HasErrors} = 1; - $JobErrors->{$Key}->{$LogName} = $LogInfo; + $JobErrors->{$Id}->{HasErrors} = 1; + $JobErrors->{$Id}->{$LogName} = $LogInfo; print $Sendmail "\n=== ", GetTitle($StepTask, $LogName), " ===\n"; print $Sendmail "$LogInfo->{BadLog}\n" if (defined $LogInfo->{BadLog}); @@ -275,12 +272,11 @@ EOF } # Print the log attachments - foreach my $Key (@SortedKeys) + foreach my $StepTask (@$SortedStepsTasks) { - my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) + foreach my $LogName (@{$JobErrors->{$StepTask->Id}->{LogNames}}) { print $Sendmail <<"EOF"; @@ -325,19 +321,19 @@ EOF # However in practice this would only be an issue if the patch reintroduced # an error that just disappeared in the latest Wine which is highly unlikely. my @Messages; - foreach my $Key (@SortedKeys) + foreach my $StepTask (@$SortedStepsTasks) { - next if (!$JobErrors->{$Key}->{HasErrors}); - my $StepTask = $StepsTasks->GetItem($Key); + my $Id = $StepTask->Id; + next if (!$JobErrors->{$Id}->{HasErrors}); # Note: We could check $StepTask->Status for TestBot errors. However, # whether they are caused by the patch or not, they prevent the TestBot # from checking for new errors which justifies sending an email to the # mailing list so that the patch receives greater scrutiny. - foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) + foreach my $LogName (@{$JobErrors->{$Id}->{LogNames}}) { - my $LogInfo = $JobErrors->{$Key}->{$LogName}; + my $LogInfo = $JobErrors->{$Id}->{$LogName}; # Skip if there are no errors next if (!$LogInfo->{NewCount}); @@ -428,12 +424,10 @@ EOF DebugTee($Result, "Job-ID: ". $Job->Id ."\n"); DebugTee($Result, "URL: $JobURL\n"); - foreach my $Key (@SortedKeys) + foreach my $StepTask (@$SortedStepsTasks) { - my $StepTask = $StepsTasks->GetItem($Key); my $TaskDir = $StepTask->GetTaskDir(); - - foreach my $LogName (@{$JobErrors->{$Key}->{LogNames}}) + foreach my $LogName (@{$JobErrors->{$StepTask->Id}->{LogNames}}) { print $Result "=== ", GetTitle($StepTask, $LogName), " ===\n"; DumpLogAndErr($Result, "$TaskDir/$LogName"); -- 2.30.2

June 13, 2022

[tools 1/3] testbot/WineSendLog: Remove a couple of unused variables.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineSendLog.pl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 37949e45f..5affba606 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -247,7 +247,7 @@ EOF print $Sendmail "\nYou can also see the results at:\n$JobURL\n\n"; # Print the job summary - my ($JobErrors, $ReportCounts); + my $JobErrors; foreach my $Key (@SortedKeys) { my $StepTask = $StepsTasks->GetItem($Key); @@ -329,7 +329,6 @@ EOF { next if (!$JobErrors->{$Key}->{HasErrors}); my $StepTask = $StepsTasks->GetItem($Key); - my $TaskDir = $StepTask->GetTaskDir(); # Note: We could check $StepTask->Status for TestBot errors. However, # whether they are caused by the patch or not, they prevent the TestBot -- 2.30.2

June 13, 2022