[PATCH v2 0/5] MR201: ntdll: Implement RtlSetUserValueHeap.
And prepare HLOCAL allocation to use it. -- v2: kernelbase: Use RtlSetUserValueHeap to store HLOCAL. ntdll: Implement RtlSetUserValueHeap. kernelbase: Use HEAP_ADD_USER_INFO for HLOCAL allocations. ntdll: Add block padding when HEAP_ADD_USER_INFO flag is set. ole32/tests: Update IStream_SetSize with large size test result. https://gitlab.winehq.org/wine/wine/-/merge_requests/201
From: Rémi Bernon <rbernon(a)codeweavers.com> The high part is ignored, and OOM error is only returned if there's not enough memory available, not because of GlobalReAlloc specific behavior. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ole32/tests/hglobalstream.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/ole32/tests/hglobalstream.c b/dlls/ole32/tests/hglobalstream.c index 237bcca9251..8310800a5c7 100644 --- a/dlls/ole32/tests/hglobalstream.c +++ b/dlls/ole32/tests/hglobalstream.c @@ -294,10 +294,9 @@ static void test_streamonhglobal(void) /* test OOM condition */ ull.u.HighPart = -1; - ull.u.LowPart = -1; + ull.u.LowPart = 0; hr = IStream_SetSize(pStream, ull); - ok(hr == E_OUTOFMEMORY || broken(hr == S_OK), /* win9x */ - "IStream_SetSize with large size should have returned E_OUTOFMEMORY instead of 0x%08lx\n", hr); + ok(hr == S_OK, "IStream_SetSize with large size should have returned S_OK instead of 0x%08lx\n", hr); IStream_Release(pStream); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/201
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernel32/tests/heap.c | 3 +-- dlls/ntdll/heap.c | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 91f582f36de..97f4d9c9fe3 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -2277,14 +2277,13 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags expect_size = max( alloc_size, 2 * sizeof(void *) ); expect_size = ALIGN_BLOCK_SIZE( expect_size + extra_size + 2 * sizeof(void *) ); diff = min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ); - todo_wine ok( diff == expect_size, "got diff %#Ix\n", diff ); ok( !memcmp( ptr0 + alloc_size, tail_buf, tail_size ), "missing block tail\n" ); ok( !memcmp( ptr1 + alloc_size, tail_buf, tail_size ), "missing block tail\n" ); ok( !memcmp( ptr2 + alloc_size, tail_buf, tail_size ), "missing block tail\n" ); - todo_wine + todo_wine_if( global_flags & FLG_HEAP_ENABLE_FREE_CHECK ) ok( !memcmp( ptr0 + alloc_size + tail_size, padd_buf, 2 * sizeof(void *) ), "unexpected padding\n" ); tmp_ptr = (void *)0xdeadbeef; diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 0ecff6f6cba..6d9f949b6a9 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -212,6 +212,7 @@ C_ASSERT( offsetof(HEAP, subheap) <= COMMIT_MASK ); /* some undocumented flags (names are made up) */ #define HEAP_PRIVATE 0x00001000 +#define HEAP_ADD_USER_INFO 0x00000100 #define HEAP_PAGE_ALLOCS 0x01000000 #define HEAP_VALIDATE 0x10000000 #define HEAP_VALIDATE_ALL 0x20000000 @@ -451,7 +452,7 @@ static RTL_CRITICAL_SECTION_DEBUG process_heap_cs_debug = static inline ULONG heap_get_flags( const 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; + 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; } @@ -1476,7 +1477,7 @@ HANDLE WINAPI RtlDestroyHeap( HANDLE heap ) static SIZE_T heap_get_block_size( HEAP *heap, ULONG flags, SIZE_T size ) { - static const ULONG padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS; + 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; SIZE_T overhead; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/201
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernelbase/memory.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 736d3642995..776f0aafa18 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -713,6 +713,9 @@ BOOL WINAPI DECLSPEC_HOTPATCH HeapWalk( HANDLE heap, PROCESS_HEAP_ENTRY *entry ) * Global/local heap functions ***********************************************************************/ +/* some undocumented flags (names are made up) */ +#define HEAP_ADD_USER_INFO 0x00000100 + /* not compatible with windows */ struct kernelbase_global_data { @@ -826,15 +829,15 @@ HGLOBAL WINAPI DECLSPEC_HOTPATCH GlobalFree( HLOCAL handle ) */ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size ) { + DWORD heap_flags = HEAP_ADD_USER_INFO; HANDLE heap = GetProcessHeap(); struct mem_entry *mem; - DWORD heap_flags = 0; HLOCAL handle; void *ptr; TRACE_(globalmem)( "flags %#x, size %#Ix\n", flags, size ); - if (flags & LMEM_ZEROINIT) heap_flags = HEAP_ZERO_MEMORY; + if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY; if (!(flags & LMEM_MOVEABLE)) /* pointer */ { @@ -964,13 +967,15 @@ 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; struct mem_entry *mem; HLOCAL ret = 0; - DWORD heap_flags = (flags & LMEM_ZEROINIT) ? HEAP_ZERO_MEMORY : 0; void *ptr; TRACE_(globalmem)( "handle %p, size %#Ix, flags %#x\n", handle, size, flags ); + if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY; + RtlLockHeap( GetProcessHeap() ); if (flags & LMEM_MODIFY) /* modify flags */ { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/201
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernel32/tests/heap.c | 13 +++---------- dlls/ntdll/heap.c | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 97f4d9c9fe3..4221fd39c0f 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -354,9 +354,7 @@ static void test_HeapCreate(void) ok( !!ptrs[i], "HeapAlloc failed, error %lu\n", GetLastError() ); align |= (UINT_PTR)ptrs[i]; } - todo_wine_if( sizeof(void *) == 8 ) ok( !(align & (8 * sizeof(void *) - 1)), "got wrong alignment\n" ); - todo_wine_if( sizeof(void *) == 8 ) ok( align & (8 * sizeof(void *)), "got wrong alignment\n" ); for (i = 0; i < ARRAY_SIZE(ptrs); ++i) { @@ -2228,7 +2226,7 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ok( !!ptr2, "HeapAlloc failed, error %lu\n", GetLastError() ); align = (UINT_PTR)ptr0 | (UINT_PTR)ptr1 | (UINT_PTR)ptr2; - todo_wine_if( sizeof(void *) == 8 || alloc_size == 0x7efe9 ) + todo_wine_if( alloc_size == 0x7efe9 ) ok( !(align & (8 * sizeof(void *) - 1)), "wrong align\n" ); expect_size = max( alloc_size, 2 * sizeof(void *) ); @@ -2303,7 +2301,6 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ok( tmp_flags == 0x200, "got flags %#lx\n", tmp_flags ); ret = pRtlSetUserValueHeap( heap, 0, ptr0, (void *)0xdeadbeef ); - todo_wine ok( ret, "RtlSetUserValueHeap failed, error %lu\n", GetLastError() ); SetLastError( 0xdeadbeef ); ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0, 0x1000 ); @@ -2330,13 +2327,9 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags "got flags %#lx\n", tmp_flags ); user_ptr = (void **)(ptr0 + alloc_size + tail_size); - todo_wine ok( user_ptr[1] == (void *)0xdeadbeef, "unexpected user value\n" ); - if (user_ptr[1] == (void *)0xdeadbeef) - { - user_ptr[0] = (void *)0xdeadbeef; - user_ptr[1] = (void *)0xdeadbee0; - } + user_ptr[0] = (void *)0xdeadbeef; + user_ptr[1] = (void *)0xdeadbee0; tmp_ptr = NULL; tmp_flags = 0; diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 6d9f949b6a9..64d4f6644af 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -105,10 +105,11 @@ C_ASSERT( sizeof(struct entry) == 2 * ALIGNMENT ); typedef struct { + SIZE_T __pad[sizeof(SIZE_T) / sizeof(DWORD)]; struct list entry; /* entry in heap large blocks list */ SIZE_T data_size; /* size of user data */ SIZE_T block_size; /* total size of virtual memory block */ - DWORD pad[2]; /* padding to ensure 16-byte alignment of data */ + void *user_value; DWORD size; /* fields for compatibility with normal arenas */ DWORD magic; /* these must remain at the end of the structure */ } ARENA_LARGE; @@ -362,6 +363,12 @@ static inline void mark_block_tail( struct block *block, DWORD flags ) memset( tail, ARENA_TAIL_FILLER, ALIGNMENT ); } valgrind_make_noaccess( tail, ALIGNMENT ); + if (flags & HEAP_ADD_USER_INFO) + { + if (flags & HEAP_TAIL_CHECKING_ENABLED || RUNNING_ON_VALGRIND) tail += ALIGNMENT; + valgrind_make_writable( tail + sizeof(void *), sizeof(void *) ); + memset( tail + sizeof(void *), 0, sizeof(void *) ); + } } /* initialize contents of a newly created block of memory */ @@ -2001,10 +2008,31 @@ BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE heap, ULONG flags, void *ptr, void **u /*********************************************************************** * RtlSetUserValueHeap (NTDLL.@) */ -BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE heap, ULONG flags, void *ptr, void *user_value ) +BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value ) { - FIXME( "heap %p, flags %#x, ptr %p, user_value %p stub!\n", heap, flags, ptr, user_value ); - return FALSE; + ARENA_LARGE *large = (ARENA_LARGE *)ptr - 1; + struct block *block; + BOOLEAN ret = TRUE; + SUBHEAP *subheap; + HEAP *heap; + char *tmp; + + TRACE( "handle %p, flags %#x, ptr %p, user_value %p.\n", handle, flags, ptr, user_value ); + + if (!(heap = HEAP_GetPtr( handle ))) return TRUE; + + heap_lock( heap, flags ); + if (!(block = unsafe_block_from_ptr( heap, ptr, &subheap ))) ret = FALSE; + else if (!subheap) large->user_value = user_value; + 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; + } + heap_unlock( heap, flags ); + + return ret; } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/201
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/kernelbase/memory.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 776f0aafa18..409b35089e3 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -44,6 +44,9 @@ WINE_DECLARE_DEBUG_CHANNEL(virtual); WINE_DECLARE_DEBUG_CHANNEL(globalmem); +BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value ); + + /*********************************************************************** * Virtual memory functions ***********************************************************************/ @@ -842,6 +845,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size ) if (!(flags & LMEM_MOVEABLE)) /* pointer */ { ptr = HeapAlloc( heap, heap_flags, size ); + if (ptr) RtlSetUserValueHeap( heap, heap_flags, ptr, ptr ); TRACE_(globalmem)( "return %p\n", ptr ); return ptr; } @@ -869,6 +873,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalAlloc( UINT flags, SIZE_T size ) else { if (!(ptr = HeapAlloc( heap, heap_flags, size + HLOCAL_STORAGE ))) goto failed; + RtlSetUserValueHeap( heap, heap_flags, ptr, handle ); *(HLOCAL *)ptr = handle; mem->ptr = (char *)ptr + HLOCAL_STORAGE; } @@ -1014,6 +1019,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f /* reallocate fixed memory */ if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY; ret = HeapReAlloc( GetProcessHeap(), heap_flags, ptr, size ); + if (ret) RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ret, ret ); } else if ((mem = unsafe_mem_from_HLOCAL( handle ))) { @@ -1027,6 +1033,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f if ((ptr = HeapReAlloc( GetProcessHeap(), heap_flags, (char *)mem->ptr - HLOCAL_STORAGE, size + HLOCAL_STORAGE ))) { + RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ptr, handle ); mem->ptr = (char *)ptr + HLOCAL_STORAGE; ret = handle; } @@ -1035,6 +1042,7 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f { if ((ptr = HeapAlloc( GetProcessHeap(), heap_flags, size + HLOCAL_STORAGE ))) { + RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ptr, handle ); *(HLOCAL *)ptr = handle; mem->ptr = (char *)ptr + HLOCAL_STORAGE; ret = handle; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/201
v2: Leave LocalReAlloc cleanup for later, it'll be easier after we got rid of the pointer offsets. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/201#note_1752
participants (1)
-
Rémi Bernon