[PATCH v2 0/7] MR1312: ntdll: Implement heap user flags and fix GlobalHandle / LocalHandle with heap pointers.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 -- v2: ntdll: Implement RtlSetUserFlagsHeap. ntdll: Check block user flags in RtlGetUserInfoHeap. ntdll: Check block user flags in RtlSetUserValueHeap. ntdll: Store the user flags in the heap block flags. ntdll: Introduce a new heap block_set_flags helper. kernel32/tests: Test (Global|Local)Handle with heap allocated pointers. kernel32/tests: Test more Global/Local functions with invalid pointers. https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/kernel32/tests/heap.c | 109 +++++++++++++++++++++++++++---------- 1 file changed, 79 insertions(+), 30 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 83daa4dafa1..309274a66bf 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -241,34 +241,36 @@ static void test_HeapCreate(void) ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); ret = HeapFree( heap, 0, NULL ); ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); -#if 0 /* crashes */ - SetLastError( 0xdeadbeef ); - ret = HeapFree( heap, 0, (void *)0xdeadbe00 ); - ok( !ret, "HeapFree succeeded\n" ); - ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); - SetLastError( 0xdeadbeef ); - ptr = (BYTE *)((UINT_PTR)buffer & ~63) + 64; - ret = HeapFree( heap, 0, ptr ); - ok( !ret, "HeapFree succeeded\n" ); - ok( GetLastError() == 0xdeadbeef, "got error %lu\n", GetLastError() ); -#endif + if (0) /* crashes */ + { + SetLastError( 0xdeadbeef ); + ret = HeapFree( heap, 0, (void *)0xdeadbe00 ); + ok( !ret, "HeapFree succeeded\n" ); + ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + ptr = (BYTE *)((UINT_PTR)buffer & ~63) + 64; + ret = HeapFree( heap, 0, ptr ); + ok( !ret, "HeapFree succeeded\n" ); + ok( GetLastError() == 0xdeadbeef, "got error %lu\n", GetLastError() ); + } SetLastError( 0xdeadbeef ); ptr = HeapReAlloc( heap, 0, NULL, 1 ); ok( !ptr, "HeapReAlloc succeeded\n" ); todo_wine ok( GetLastError() == NO_ERROR, "got error %lu\n", GetLastError() ); -#if 0 /* crashes */ - SetLastError( 0xdeadbeef ); - ptr1 = HeapReAlloc( heap, 0, (void *)0xdeadbe00, 1 ); - ok( !ptr1, "HeapReAlloc succeeded\n" ); - ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); - SetLastError( 0xdeadbeef ); - ptr = (BYTE *)((UINT_PTR)buffer & ~63) + 64; - ptr1 = HeapReAlloc( heap, 0, ptr, 1 ); - ok( !ptr1, "HeapReAlloc succeeded\n" ); - ok( GetLastError() == 0xdeadbeef, "got error %lu\n", GetLastError() ); -#endif + if (0) /* crashes */ + { + SetLastError( 0xdeadbeef ); + ptr1 = HeapReAlloc( heap, 0, (void *)0xdeadbe00, 1 ); + ok( !ptr1, "HeapReAlloc succeeded\n" ); + ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + ptr = (BYTE *)((UINT_PTR)buffer & ~63) + 64; + ptr1 = HeapReAlloc( heap, 0, ptr, 1 ); + ok( !ptr1, "HeapReAlloc succeeded\n" ); + ok( GetLastError() == 0xdeadbeef, "got error %lu\n", GetLastError() ); + } SetLastError( 0xdeadbeef ); ret = HeapValidate( heap, 0, NULL ); @@ -1557,14 +1559,18 @@ static void test_GlobalAlloc(void) todo_wine ok( ret, "GlobalUnlock failed, error %lu\n", GetLastError() ); ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); -#if 0 /* corrupts Wine heap */ SetLastError( 0xdeadbeef ); tmp_mem = GlobalReAlloc( mem, 0, GMEM_MOVEABLE ); - todo_wine ok( !tmp_mem, "GlobalReAlloc succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); -#endif + if (sizeof(void *) != 8) /* crashes on 64-bit */ + { + SetLastError( 0xdeadbeef ); + tmp_mem = GlobalHandle( mem ); + ok( !tmp_mem, "GlobalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + } /* invalid handles are caught */ SetLastError( 0xdeadbeef ); @@ -1592,6 +1598,20 @@ static void test_GlobalAlloc(void) tmp_mem = GlobalReAlloc( invalid_mem, 0, GMEM_MOVEABLE ); ok( !tmp_mem, "GlobalReAlloc succeeded\n" ); ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + if (sizeof(void *) != 8) /* crashes on 64-bit */ + { + SetLastError( 0xdeadbeef ); + tmp_mem = GlobalHandle( invalid_mem ); + ok( !tmp_mem, "GlobalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, invalid_mem, (void **)&tmp_ptr, &tmp_flags ); + todo_wine + ok( !ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + todo_wine + ok( GetLastError() == ERROR_INVALID_PARAMETER, "got error %lu\n", GetLastError() ); + } /* invalid pointers are caught */ SetLastError( 0xdeadbeef ); @@ -1624,6 +1644,18 @@ static void test_GlobalAlloc(void) ok( !tmp_mem, "GlobalReAlloc succeeded\n" ); todo_wine ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + tmp_mem = GlobalHandle( invalid_ptr ); + ok( !tmp_mem, "GlobalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + if (0) /* crashes */ + { + SetLastError( 0xdeadbeef ); + ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, invalid_ptr, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + } /* GMEM_FIXED block doesn't allow resize, though it succeeds with GMEM_MODIFY */ mem = GlobalAlloc( GMEM_FIXED, 10 ); @@ -1912,14 +1944,18 @@ static void test_LocalAlloc(void) ret = LocalUnlock( mem ); ok( !ret, "LocalUnlock succeeded\n" ); ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); -#if 0 /* corrupts Wine heap */ SetLastError( 0xdeadbeef ); tmp_mem = LocalReAlloc( mem, 0, LMEM_MOVEABLE ); - todo_wine ok( !tmp_mem, "LocalReAlloc succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); -#endif + if (sizeof(void *) != 8) /* crashes on 64-bit */ + { + SetLastError( 0xdeadbeef ); + tmp_mem = LocalHandle( mem ); + ok( !tmp_mem, "LocalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + } /* invalid handles are caught */ SetLastError( 0xdeadbeef ); @@ -1946,6 +1982,14 @@ static void test_LocalAlloc(void) tmp_mem = LocalReAlloc( invalid_mem, 0, LMEM_MOVEABLE ); ok( !tmp_mem, "LocalReAlloc succeeded\n" ); ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + if (sizeof(void *) != 8) /* crashes on 64-bit */ + { + SetLastError( 0xdeadbeef ); + tmp_mem = LocalHandle( invalid_mem ); + ok( !tmp_mem, "LocalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); + } /* invalid pointers are caught */ SetLastError( 0xdeadbeef ); @@ -1977,6 +2021,11 @@ static void test_LocalAlloc(void) ok( !tmp_mem, "LocalReAlloc succeeded\n" ); todo_wine ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + tmp_mem = LocalHandle( invalid_ptr ); + ok( !tmp_mem, "LocalHandle succeeded\n" ); + todo_wine + ok( GetLastError() == ERROR_NOACCESS, "got error %lu\n", GetLastError() ); /* LMEM_FIXED block doesn't allow resize, though it succeeds with LMEM_MODIFY */ mem = LocalAlloc( LMEM_FIXED, 10 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/kernel32/tests/heap.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 309274a66bf..526a898209d 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -1813,6 +1813,23 @@ static void test_GlobalAlloc(void) mem = GlobalFree( mem ); ok( !mem, "GlobalFree failed, error %lu\n", GetLastError() ); } + + ptr = HeapAlloc( GetProcessHeap(), 0, 16 ); + ok( !!ptr, "HeapAlloc failed, error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + tmp_mem = GlobalHandle( ptr ); + ok( !!tmp_mem, "GlobalHandle failed, error %lu\n", GetLastError() ); + todo_wine + ok( tmp_mem == ptr, "GlobalHandle returned unexpected handle\n" ); + tmp_ptr = (void *)0xdeadbeef; + tmp_flags = 0xdeadbeef; + ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, ptr, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + todo_wine + ok( tmp_ptr == (void *)0xdeadbeef, "got user value %p\n", tmp_ptr ); + ok( tmp_flags == 0, "got user flags %#lx\n", tmp_flags ); + ret = HeapFree( GetProcessHeap(), 0, ptr ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); } static void test_LocalAlloc(void) @@ -1833,6 +1850,7 @@ static void test_LocalAlloc(void) HLOCAL locals[0x10000]; HLOCAL mem, tmp_mem; BYTE *ptr, *tmp_ptr; + ULONG tmp_flags; UINT i, flags; SIZE_T size; BOOL ret; @@ -2162,6 +2180,23 @@ static void test_LocalAlloc(void) mem = LocalFree( mem ); ok( !mem, "LocalFree failed, error %lu\n", GetLastError() ); } + + ptr = HeapAlloc( GetProcessHeap(), 0, 16 ); + ok( !!ptr, "HeapAlloc failed, error %lu\n", GetLastError() ); + SetLastError( 0xdeadbeef ); + tmp_mem = LocalHandle( ptr ); + ok( !!tmp_mem, "LocalHandle failed, error %lu\n", GetLastError() ); + todo_wine + ok( tmp_mem == ptr, "LocalHandle returned unexpected handle\n" ); + tmp_ptr = (void *)0xdeadbeef; + tmp_flags = 0xdeadbeef; + ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, ptr, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + todo_wine + ok( tmp_ptr == (void *)0xdeadbeef, "got user value %p\n", tmp_ptr ); + ok( tmp_flags == 0, "got user flags %#lx\n", tmp_flags ); + ret = HeapFree( GetProcessHeap(), 0, ptr ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); } static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags, DWORD alloc_flags ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> To clear and set block flags independently of the block size. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/ntdll/heap.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 8530bdf223f..3258dd2e4f4 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -256,12 +256,19 @@ static inline UINT block_get_size( const struct block *block ) return block_size * ALIGNMENT; } -static inline void block_set_size( struct block *block, UINT block_flags, UINT block_size ) +static inline void block_set_size( struct block *block, UINT block_size ) { block_size /= ALIGNMENT; - if (block_flags & BLOCK_FLAG_FREE) block->tail_size = block_size >> 16; + if (block_get_flags( block ) & BLOCK_FLAG_FREE) block->tail_size = block_size >> 16; block->block_size = block_size; - block->block_flags = block_flags; +} + +static inline void block_set_flags( struct block *block, BYTE clear, BYTE set ) +{ + UINT block_size = block_get_size( block ); + block->block_flags &= ~clear; + block->block_flags |= set; + block_set_size( block, block_size ); } static inline void *subheap_base( const SUBHEAP *subheap ) @@ -682,7 +689,8 @@ static void create_free_block( struct heap *heap, SUBHEAP *subheap, struct block valgrind_make_writable( block, sizeof(*entry) ); block_set_type( block, ARENA_FREE_MAGIC ); - block_set_size( block, BLOCK_FLAG_FREE, block_size ); + block_set_flags( block, ~0, BLOCK_FLAG_FREE ); + block_set_size( block, block_size ); /* If debugging, erase the freed block content */ @@ -695,14 +703,14 @@ static void create_free_block( struct heap *heap, SUBHEAP *subheap, struct block struct entry *next_entry = (struct entry *)next; list_remove( &next_entry->entry ); block_size += block_get_size( next ); - block_set_size( block, BLOCK_FLAG_FREE, block_size ); + 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 */ - block_set_size( next, BLOCK_FLAG_PREV_FREE, block_get_size( next ) ); + block_set_flags( next, 0, BLOCK_FLAG_PREV_FREE ); valgrind_make_writable( (struct block **)next - 1, sizeof(struct block *) ); *((struct block **)next - 1) = block; } @@ -760,21 +768,21 @@ static void free_used_block( struct heap *heap, SUBHEAP *subheap, struct block * } -static inline void shrink_used_block( struct heap *heap, SUBHEAP *subheap, struct block *block, UINT flags, +static inline void shrink_used_block( struct heap *heap, SUBHEAP *subheap, struct block *block, 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_set_size( block, block_size ); block->tail_size = block_size - sizeof(*block) - size; create_free_block( heap, subheap, next_block( subheap, block ), old_block_size - block_size ); } else { struct block *next; - block_set_size( block, flags, old_block_size ); + block_set_size( block, old_block_size ); block->tail_size = old_block_size - sizeof(*block) - size; - if ((next = next_block( subheap, block ))) next->block_flags &= ~BLOCK_FLAG_PREV_FREE; + if ((next = next_block( subheap, block ))) block_set_flags( next, BLOCK_FLAG_PREV_FREE, 0 ); } } @@ -804,7 +812,8 @@ static struct block *allocate_large_block( struct heap *heap, DWORD flags, SIZE_ arena->block_size = (char *)address + total_size - (char *)block; block_set_type( block, ARENA_LARGE_MAGIC ); - block_set_size( block, BLOCK_FLAG_LARGE, 0 ); + block_set_flags( block, ~0, BLOCK_FLAG_LARGE ); + block_set_size( block, 0 ); list_add_tail( &heap->large_list, &arena->entry ); valgrind_make_noaccess( (char *)block + sizeof(*block) + arena->data_size, arena->block_size - sizeof(*block) - arena->data_size ); @@ -968,7 +977,8 @@ static SUBHEAP *HEAP_CreateSubHeap( struct heap **heap_ptr, LPVOID address, DWOR list_init( &heap->free_lists[0].entry ); for (i = 0, pEntry = heap->free_lists; i < HEAP_NB_FREE_LISTS; i++, pEntry++) { - block_set_size( &pEntry->block, BLOCK_FLAG_FREE_LINK, 0 ); + block_set_flags( &pEntry->block, ~0, BLOCK_FLAG_FREE_LINK ); + block_set_size( &pEntry->block, 0 ); block_set_type( &pEntry->block, ARENA_FREE_MAGIC ); if (i) list_add_after( &pEntry[-1].entry, &pEntry->entry ); } @@ -1510,7 +1520,8 @@ 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( heap, subheap, block, 0, old_block_size, block_size, size ); + block_set_flags( block, ~0, 0 ); + shrink_used_block( heap, subheap, block, old_block_size, block_size, size ); initialize_block( block + 1, size, flags ); mark_block_tail( block, flags ); @@ -1632,7 +1643,7 @@ static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE } valgrind_notify_resize( block + 1, old_size, size ); - shrink_used_block( heap, subheap, block, block_get_flags( block ), old_block_size, block_size, size ); + shrink_used_block( heap, subheap, 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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/ntdll/heap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 3258dd2e4f4..f218ba12f3e 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -92,7 +92,10 @@ C_ASSERT( sizeof(struct block) == 8 ); #define BLOCK_FLAG_PREV_FREE 0x00000002 #define BLOCK_FLAG_FREE_LINK 0x00000003 #define BLOCK_FLAG_LARGE 0x00000004 +#define BLOCK_FLAG_USER_INFO 0x00000010 /* user flags up to 0xf0 */ +#define BLOCK_FLAG_USER_MASK 0x000000f0 +#define BLOCK_USER_FLAGS( heap_flags ) (((heap_flags) >> 4) & BLOCK_FLAG_USER_MASK) /* entry to link free blocks in free lists */ @@ -212,6 +215,7 @@ C_ASSERT( offsetof(struct heap, subheap) <= COMMIT_MASK ); /* some undocumented flags (names are made up) */ #define HEAP_PRIVATE 0x00001000 #define HEAP_ADD_USER_INFO 0x00000100 +#define HEAP_USER_FLAGS_MASK 0x00000f00 #define HEAP_PAGE_ALLOCS 0x01000000 #define HEAP_VALIDATE 0x10000000 #define HEAP_VALIDATE_ALL 0x20000000 @@ -469,7 +473,7 @@ static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug = 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; + flags &= HEAP_GENERATE_EXCEPTIONS | HEAP_NO_SERIALIZE | HEAP_ZERO_MEMORY | HEAP_REALLOC_IN_PLACE_ONLY | HEAP_CHECKING_ENABLED | HEAP_USER_FLAGS_MASK; return heap->flags | flags; } @@ -812,7 +816,7 @@ static struct block *allocate_large_block( struct heap *heap, DWORD flags, SIZE_ arena->block_size = (char *)address + total_size - (char *)block; block_set_type( block, ARENA_LARGE_MAGIC ); - block_set_flags( block, ~0, BLOCK_FLAG_LARGE ); + block_set_flags( block, ~0, BLOCK_FLAG_LARGE | BLOCK_USER_FLAGS( flags ) ); block_set_size( block, 0 ); list_add_tail( &heap->large_list, &arena->entry ); valgrind_make_noaccess( (char *)block + sizeof(*block) + arena->data_size, @@ -896,7 +900,7 @@ static BOOL validate_large_block( const struct heap *heap, const struct block *b err = "invalid block alignment"; else if (block_get_size( block )) err = "invalid block size"; - else if (block_get_flags( block ) != BLOCK_FLAG_LARGE) + else if (!(block_get_flags( block ) & BLOCK_FLAG_LARGE)) err = "invalid block flags"; else if (block_get_type( block ) != ARENA_LARGE_MAGIC) err = "invalid block type"; @@ -1520,7 +1524,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 ); - block_set_flags( block, ~0, 0 ); + block_set_flags( block, ~0, BLOCK_USER_FLAGS( flags ) ); shrink_used_block( heap, subheap, block, old_block_size, block_size, size ); initialize_block( block + 1, size, flags ); mark_block_tail( block, flags ); @@ -1643,6 +1647,7 @@ static NTSTATUS heap_reallocate( struct heap *heap, ULONG flags, void *ptr, SIZE } valgrind_notify_resize( block + 1, old_size, size ); + block_set_flags( block, BLOCK_FLAG_USER_MASK, BLOCK_USER_FLAGS( flags ) ); shrink_used_block( heap, subheap, block, old_block_size, block_size, size ); if (size > old_size) initialize_block( (char *)(block + 1) + old_size, size - old_size, flags ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Adding the same user flags as native, for Global/Local allocs, and returning the pointer from Global/LocalHandle by default. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/kernel32/heap.c | 2 +- dlls/kernelbase/memory.c | 4 ++-- dlls/ntdll/heap.c | 9 +++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c index 1c1e6b05f25..5f7f950d23d 100644 --- a/dlls/kernel32/heap.c +++ b/dlls/kernel32/heap.c @@ -343,7 +343,7 @@ UINT WINAPI LocalFlags( HLOCAL handle ) */ HLOCAL WINAPI LocalHandle( const void *ptr ) { - HLOCAL handle; + HLOCAL handle = (HANDLE)ptr; ULONG flags; TRACE_(globalmem)( "ptr %p\n", ptr ); diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 19b1d0a3a1a..2a503587e93 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -835,7 +835,7 @@ HGLOBAL WINAPI DECLSPEC_HOTPATCH GlobalFree( HLOCAL handle ) */ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size ) { - DWORD heap_flags = HEAP_ADD_USER_INFO; + DWORD heap_flags = 0x200 | HEAP_ADD_USER_INFO; HANDLE heap = GetProcessHeap(); struct mem_entry *mem; HLOCAL handle; @@ -974,7 +974,7 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle ) */ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT flags ) { - DWORD heap_flags = HEAP_ADD_USER_INFO | HEAP_NO_SERIALIZE; + DWORD heap_flags = 0x200 | HEAP_ADD_USER_INFO | HEAP_NO_SERIALIZE; HANDLE heap = GetProcessHeap(); struct mem_entry *mem; HLOCAL ret = 0; diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index f218ba12f3e..e0af0e72925 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -2044,7 +2044,7 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void * BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value ) { struct block *block; - BOOLEAN ret = TRUE; + BOOLEAN ret = FALSE; struct heap *heap; SUBHEAP *subheap; char *tmp; @@ -2054,17 +2054,22 @@ BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void if (!(heap = unsafe_heap_from_handle( handle ))) return TRUE; heap_lock( heap, flags ); - if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) ret = FALSE; + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) + WARN( "Failed to find block %p in heap %p\n", ptr, handle ); + else if (!(block_get_flags( block ) & BLOCK_FLAG_USER_INFO)) + WARN( "Block %p wasn't allocated with user info\n", ptr ); else if (!subheap) { ARENA_LARGE *large = CONTAINING_RECORD( block, ARENA_LARGE, block ); large->user_value = user_value; + ret = TRUE; } else { tmp = (char *)block + block_get_size( block ) - block->tail_size + sizeof(void *); if ((heap_get_flags( heap, flags ) & HEAP_TAIL_CHECKING_ENABLED) || RUNNING_ON_VALGRIND) tmp += ALIGNMENT; *(void **)tmp = user_value; + ret = TRUE; } heap_unlock( heap, flags ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/kernel32/tests/heap.c | 14 -------------- dlls/ntdll/heap.c | 20 ++++++++++++++++---- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 526a898209d..7cecc4a319a 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -1411,7 +1411,6 @@ static void test_GlobalAlloc(void) ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, entry->ptr, (void **)&tmp_mem, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); ok( tmp_mem == mem, "got user ptr %p\n", tmp_mem ); - todo_wine ok( tmp_flags == 0x200, "got user flags %#lx\n", tmp_flags ); ret = pRtlSetUserValueHeap( GetProcessHeap(), 0, entry->ptr, invalid_mem ); @@ -1568,7 +1567,6 @@ static void test_GlobalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = GlobalHandle( mem ); ok( !tmp_mem, "GlobalHandle succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); } @@ -1603,13 +1601,10 @@ static void test_GlobalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = GlobalHandle( invalid_mem ); ok( !tmp_mem, "GlobalHandle succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); SetLastError( 0xdeadbeef ); ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, invalid_mem, (void **)&tmp_ptr, &tmp_flags ); - todo_wine ok( !ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); - todo_wine ok( GetLastError() == ERROR_INVALID_PARAMETER, "got error %lu\n", GetLastError() ); } @@ -1647,7 +1642,6 @@ static void test_GlobalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = GlobalHandle( invalid_ptr ); ok( !tmp_mem, "GlobalHandle succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); if (0) /* crashes */ { @@ -1819,13 +1813,11 @@ static void test_GlobalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = GlobalHandle( ptr ); ok( !!tmp_mem, "GlobalHandle failed, error %lu\n", GetLastError() ); - todo_wine ok( tmp_mem == ptr, "GlobalHandle returned unexpected handle\n" ); tmp_ptr = (void *)0xdeadbeef; tmp_flags = 0xdeadbeef; ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, ptr, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); - todo_wine ok( tmp_ptr == (void *)0xdeadbeef, "got user value %p\n", tmp_ptr ); ok( tmp_flags == 0, "got user flags %#lx\n", tmp_flags ); ret = HeapFree( GetProcessHeap(), 0, ptr ); @@ -1971,7 +1963,6 @@ static void test_LocalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = LocalHandle( mem ); ok( !tmp_mem, "LocalHandle succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); } @@ -2005,7 +1996,6 @@ static void test_LocalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = LocalHandle( invalid_mem ); ok( !tmp_mem, "LocalHandle succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_HANDLE, "got error %lu\n", GetLastError() ); } @@ -2186,13 +2176,11 @@ static void test_LocalAlloc(void) SetLastError( 0xdeadbeef ); tmp_mem = LocalHandle( ptr ); ok( !!tmp_mem, "LocalHandle failed, error %lu\n", GetLastError() ); - todo_wine ok( tmp_mem == ptr, "LocalHandle returned unexpected handle\n" ); tmp_ptr = (void *)0xdeadbeef; tmp_flags = 0xdeadbeef; ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, ptr, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); - todo_wine ok( tmp_ptr == (void *)0xdeadbeef, "got user value %p\n", tmp_ptr ); ok( tmp_flags == 0, "got user flags %#lx\n", tmp_flags ); ret = HeapFree( GetProcessHeap(), 0, ptr ); @@ -2359,7 +2347,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ret = pRtlGetUserInfoHeap( heap, 0, ptr0, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); ok( tmp_ptr == NULL, "got ptr %p\n", tmp_ptr ); - todo_wine ok( tmp_flags == 0xc00, "got flags %#lx\n", tmp_flags ); tmp_ptr = (void *)0xdeadbeef; @@ -2367,7 +2354,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ret = pRtlGetUserInfoHeap( heap, 0, ptr1, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); ok( tmp_ptr == NULL, "got ptr %p\n", tmp_ptr ); - todo_wine ok( tmp_flags == 0x200, "got flags %#lx\n", tmp_flags ); ret = pRtlSetUserValueHeap( heap, 0, ptr0, (void *)0xdeadbeef ); diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index e0af0e72925..4b8f45f82cd 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -96,6 +96,7 @@ C_ASSERT( sizeof(struct block) == 8 ); #define BLOCK_FLAG_USER_MASK 0x000000f0 #define BLOCK_USER_FLAGS( heap_flags ) (((heap_flags) >> 4) & BLOCK_FLAG_USER_MASK) +#define HEAP_USER_FLAGS( block_flags ) (((block_flags) & BLOCK_FLAG_USER_MASK) << 4) /* entry to link free blocks in free lists */ @@ -2008,6 +2009,7 @@ NTSTATUS WINAPI RtlSetHeapInformation( HANDLE handle, HEAP_INFORMATION_CLASS inf */ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void **user_value, ULONG *user_flags ) { + NTSTATUS status = STATUS_SUCCESS; struct block *block; struct heap *heap; SUBHEAP *subheap; @@ -2016,26 +2018,36 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void * TRACE( "handle %p, flags %#x, ptr %p, user_value %p, user_flags %p semi-stub!\n", handle, flags, ptr, user_value, user_flags ); - *user_value = 0; *user_flags = 0; if (!(heap = unsafe_heap_from_handle( handle ))) return TRUE; heap_lock( heap, flags ); - if ((block = unsafe_block_from_ptr( heap, ptr, &subheap )) && !subheap) + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) + { + WARN( "Failed to find block %p in heap %p\n", ptr, handle ); + status = STATUS_INVALID_PARAMETER; + *user_value = 0; + } + else if (!(*user_flags = HEAP_USER_FLAGS(block_get_flags( block )))) + WARN( "Block %p wasn't allocated with user info\n", ptr ); + else if (!subheap) { const ARENA_LARGE *large = CONTAINING_RECORD( block, ARENA_LARGE, block ); + *user_flags = *user_flags & ~HEAP_ADD_USER_INFO; *user_value = large->user_value; } - else if (block) + else { tmp = (char *)block + block_get_size( block ) - block->tail_size + sizeof(void *); if ((heap_get_flags( heap, flags ) & HEAP_TAIL_CHECKING_ENABLED) || RUNNING_ON_VALGRIND) tmp += ALIGNMENT; + *user_flags = *user_flags & ~HEAP_ADD_USER_INFO; *user_value = *(void **)tmp; } heap_unlock( heap, flags ); - return TRUE; + heap_set_status( heap, flags, status ); + return !status; } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53741 --- dlls/kernel32/tests/heap.c | 5 ----- dlls/ntdll/heap.c | 30 ++++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 7cecc4a319a..46256493913 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -2361,15 +2361,12 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags SetLastError( 0xdeadbeef ); ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0, 0x1000 ); ok( !ret, "RtlSetUserFlagsHeap succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_PARAMETER, "got error %lu\n", GetLastError() ); SetLastError( 0xdeadbeef ); ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0x100, 0 ); ok( !ret, "RtlSetUserFlagsHeap succeeded\n" ); - todo_wine ok( GetLastError() == ERROR_INVALID_PARAMETER, "got error %lu\n", GetLastError() ); ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0x400, 0x200 ); - todo_wine ok( ret, "RtlSetUserFlagsHeap failed, error %lu\n", GetLastError() ); tmp_ptr = NULL; @@ -2377,7 +2374,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ret = pRtlGetUserInfoHeap( heap, 0, ptr0, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); ok( tmp_ptr == (void *)0xdeadbeef, "got ptr %p\n", tmp_ptr ); - todo_wine ok( tmp_flags == 0xa00 || broken(tmp_flags == 0xc00) /* w1064v1507 */, "got flags %#lx\n", tmp_flags ); @@ -2391,7 +2387,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ret = pRtlGetUserInfoHeap( heap, 0, ptr0, (void **)&tmp_ptr, &tmp_flags ); ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); ok( tmp_ptr == (void *)0xdeadbee0, "got ptr %p\n", tmp_ptr ); - todo_wine ok( tmp_flags == 0xa00 || broken(tmp_flags == 0xc00) /* w1064v1507 */, "got flags %#lx\n", tmp_flags ); diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 4b8f45f82cd..3fe601feb2e 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -2093,6 +2093,32 @@ BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void */ BOOLEAN WINAPI RtlSetUserFlagsHeap( HANDLE handle, ULONG flags, void *ptr, ULONG clear, ULONG set ) { - FIXME( "handle %p, flags %#x, ptr %p, clear %#x, set %#x stub!\n", handle, flags, ptr, clear, set ); - return FALSE; + struct block *block; + BOOLEAN ret = FALSE; + struct heap *heap; + SUBHEAP *subheap; + + TRACE( "handle %p, flags %#x, ptr %p, clear %#x, set %#x.\n", handle, flags, ptr, clear, set ); + + if ((clear | set) & ~(0xe00)) + { + SetLastError( ERROR_INVALID_PARAMETER ); + return FALSE; + } + + if (!(heap = unsafe_heap_from_handle( handle ))) return TRUE; + + heap_lock( heap, flags ); + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) + WARN( "Failed to find block %p in heap %p\n", ptr, handle ); + else if (!(block_get_flags( block ) & BLOCK_FLAG_USER_INFO)) + WARN( "Block %p wasn't allocated with user info\n", ptr ); + else + { + block_set_flags( block, BLOCK_USER_FLAGS( clear ), BLOCK_USER_FLAGS( set ) ); + ret = TRUE; + } + heap_unlock( heap, flags ); + + return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1312
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=126046 Your paranoid android. === debian11 (32 bit report) === d3drm: d3drm.c:4600: Test failed: Cannot create IDirect3DRMDevice interface, hr 0x8007000e. d3drm.c:4602: Test failed: expected ref2 > ref1, got ref1 = 1 , ref2 = 1. d3drm.c:4604: Test failed: Expected surface_ref2 > surface_ref1, got surface_ref1 = 1, surface_ref2 = 1. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x0040e52b). === debian11 (build log) === 0370:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this.
v2: Added more tests and checked invalid heap pointer (as with the dsound test) behavior. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1312#note_13943
participants (2)
-
Marvin -
Rémi Bernon