Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 53cff284c6f..a131a7f56e0 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -328,10 +328,12 @@ static void test_HeapCreate(void) ok( !!heap, "HeapCreate failed, error %lu\n", GetLastError() ); ok( !((ULONG_PTR)heap & 0xffff), "wrong heap alignment\n" );
- ptr = HeapAlloc( heap, 0, alloc_size - (0x400 + 0x80 * sizeof(void *)) ); + /* theshold between failure and success varies, and w7pro64 has a much larger overhead. */ + + ptr = HeapAlloc( heap, 0, alloc_size - (0x400 + 0x100 * sizeof(void *)) ); ok( !!ptr, "HeapAlloc failed, error %lu\n", GetLastError() ); size = HeapSize( heap, 0, ptr ); - ok( size == alloc_size - (0x400 + 0x80 * sizeof(void *)), + ok( size == alloc_size - (0x400 + 0x100 * sizeof(void *)), "HeapSize returned %#Ix, error %lu\n", size, GetLastError() ); ret = HeapFree( heap, 0, ptr ); ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); @@ -438,7 +440,7 @@ static void test_HeapCreate(void) "got Region.dwCommittedSize %#lx\n", entries[0].Region.dwCommittedSize ); todo_wine ok( entries[0].Region.dwUnCommittedSize == 0x10000 - entries[0].Region.dwCommittedSize || - entries[0].Region.dwUnCommittedSize == 0x40000 - entries[0].Region.dwCommittedSize /* win7 */, + entries[0].Region.dwUnCommittedSize == 0x10000 * sizeof(void *) - entries[0].Region.dwCommittedSize /* win7 */, "got Region.dwUnCommittedSize %#lx\n", entries[0].Region.dwUnCommittedSize ); todo_wine ok( (BYTE *)entries[0].Region.lpFirstBlock == (BYTE *)entries[0].lpData + entries[0].cbData + 2 * sizeof(void *) || @@ -493,7 +495,7 @@ static void test_HeapCreate(void) ok( entries[3].lpData == ptr, "got lpData %p\n", entries[3].lpData ); todo_wine ok( entries[3].cbData == 5 * alloc_size, "got cbData %#lx\n", entries[3].cbData ); - ok( entries[3].cbOverhead == 0 || entries[3].cbOverhead == 0x20 /* win7 */, + ok( entries[3].cbOverhead == 0 || entries[3].cbOverhead == 8 * sizeof(void *) /* win7 */, "got cbOverhead %#x\n", entries[3].cbOverhead ); todo_wine ok( entries[3].iRegionIndex == 64, "got iRegionIndex %d\n", entries[3].iRegionIndex ); @@ -520,7 +522,7 @@ static void test_HeapCreate(void) ok( entries[4].lpData == ptr1, "got lpData %p\n", entries[4].lpData ); todo_wine ok( entries[4].cbData == 5 * alloc_size, "got cbData %#lx\n", entries[4].cbData ); - ok( entries[4].cbOverhead == 0 || entries[4].cbOverhead == 0x20 /* win7 */, + ok( entries[4].cbOverhead == 0 || entries[4].cbOverhead == 8 * sizeof(void *) /* win7 */, "got cbOverhead %#x\n", entries[4].cbOverhead ); todo_wine ok( entries[4].iRegionIndex == 64, "got iRegionIndex %d\n", entries[4].iRegionIndex );
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index a131a7f56e0..e1cda94f78c 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -998,7 +998,7 @@ static void test_GlobalAlloc(void) mem = GlobalFree( mem ); ok( !mem, "GlobalFree failed, error %lu\n", GetLastError() );
- for (alloc_size = 1; alloc_size < 0x10000000; alloc_size <<= 1) + for (alloc_size = 1; alloc_size < 0x10000000; alloc_size <<= 5) { winetest_push_context( "size %#Ix", alloc_size );
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=112851
Your paranoid android.
=== w1064v1507 (64 bit report) ===
kernel32: heap.c:2150: Test failed: got ullAvailPhys 0x8117e000 heap.c:2152: Test failed: got ullAvailPageFile 0x863e9000
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 346bcf3c10c..01364016271 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -153,6 +153,7 @@ typedef struct tagHEAP /* For Vista through 10, 'flags' is at offset 0x40 (x86) / 0x70 (x64) */ DWORD flags; /* Heap flags */ DWORD force_flags; /* Forced heap flags for debugging */ + BOOL shared; /* System shared heap */ SUBHEAP subheap; /* First sub-heap */ struct list entry; /* Entry in process heap list */ struct list subheap_list; /* Sub-heap list */ @@ -687,7 +688,7 @@ static void HEAP_MakeInUseBlockFree( SUBHEAP *subheap, ARENA_INUSE *pArena )
/* Decommit the end of the heap */
- if (!(subheap->heap->flags & HEAP_SHARED)) HEAP_Decommit( subheap, pFree + 1 ); + if (!(subheap->heap->shared)) HEAP_Decommit( subheap, pFree + 1 ); }
@@ -922,6 +923,7 @@ static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags,
heap = address; heap->flags = flags; + heap->shared = (flags & HEAP_SHARED) != 0; heap->magic = HEAP_MAGIC; heap->grow_size = max( HEAP_DEF_SIZE, totalSize ); list_init( &heap->subheap_list ); @@ -967,7 +969,7 @@ static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags, heap->critSection.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": HEAP.critSection"); }
- if (flags & HEAP_SHARED) + if (heap->shared) { /* let's assume that only one thread at a time will try to do this */ HANDLE sem = heap->critSection.LockSemaphore;
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=112852
Your paranoid android.
=== debian11 (32 bit WoW report) ===
ntdll: virtual: Timeout
=== debian11 (64 bit WoW report) ===
ntdll: env.c:461: Test failed: wrong end ptr 000000000003197C/0000000000031980
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 3 -- dlls/ntdll/heap.c | 72 +++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 32 deletions(-)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index e1cda94f78c..104eced3bf4 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -1974,9 +1974,7 @@ static void test_heap_layout( HANDLE handle, DWORD global_flag, DWORD heap_flags if (global_flag & FLG_HEAP_ENABLE_TAGGING) heap_flags |= HEAP_SHARED; if (!(global_flag & FLG_HEAP_PAGE_ALLOCS)) force_flags &= ~(HEAP_GROWABLE|HEAP_PRIVATE);
- todo_wine_if( force_flags & (HEAP_PRIVATE|HEAP_NO_SERIALIZE) ) ok( heap->force_flags == force_flags, "got force_flags %#x\n", heap->force_flags ); - todo_wine_if( heap_flags & (HEAP_VALIDATE_ALL|HEAP_VALIDATE_PARAMS|HEAP_SHARED|HEAP_PRIVATE) ) ok( heap->flags == heap_flags, "got flags %#x\n", heap->flags );
if (heap->flags & HEAP_PAGE_ALLOCS) @@ -1990,7 +1988,6 @@ static void test_heap_layout( HANDLE handle, DWORD global_flag, DWORD heap_flags } else { - todo_wine ok( heap->ffeeffee == 0xffeeffee, "got ffeeffee %#x\n", heap->ffeeffee ); ok( heap->auto_flags == (heap_flags & HEAP_GROWABLE) || !heap->auto_flags, "got auto_flags %#x\n", heap->auto_flags ); diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 01364016271..298c39f24ff 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -142,17 +142,17 @@ typedef struct tagSUBHEAP #define SUBHEAP_MAGIC ((DWORD)('S' | ('U'<<8) | ('B'<<16) | ('H'<<24)))
typedef struct tagHEAP -{ - DWORD_PTR unknown1[2]; - DWORD unknown2[2]; - DWORD_PTR unknown3[4]; - DWORD unknown4; - DWORD_PTR unknown5[2]; - DWORD unknown6[3]; - DWORD_PTR unknown7[2]; - /* For Vista through 10, 'flags' is at offset 0x40 (x86) / 0x70 (x64) */ - DWORD flags; /* Heap flags */ - DWORD force_flags; /* Forced heap flags for debugging */ +{ /* win32/win64 */ + DWORD_PTR unknown1[2]; /* 0000/0000 */ + DWORD ffeeffee; /* 0008/0010 */ + DWORD auto_flags; /* 000c/0014 */ + DWORD_PTR unknown2[7]; /* 0010/0018 */ + DWORD unknown3[2]; /* 002c/0050 */ + DWORD_PTR unknown4[3]; /* 0034/0058 */ + DWORD flags; /* 0040/0070 */ + DWORD force_flags; /* 0044/0074 */ + /* end of the Windows 10 compatible struct layout */ + BOOL shared; /* System shared heap */ SUBHEAP subheap; /* First sub-heap */ struct list entry; /* Entry in process heap list */ @@ -173,6 +173,7 @@ typedef struct tagHEAP #define MAX_FREE_PENDING 1024 /* max number of free requests to delay */
/* some undocumented flags (names are made up) */ +#define HEAP_PRIVATE 0x00001000 #define HEAP_PAGE_ALLOCS 0x01000000 #define HEAP_VALIDATE 0x10000000 #define HEAP_VALIDATE_ALL 0x20000000 @@ -922,7 +923,9 @@ static SUBHEAP *HEAP_CreateSubHeap( HEAP *heap, LPVOID address, DWORD flags, /* If this is a primary subheap, initialize main heap */
heap = address; - heap->flags = flags; + heap->ffeeffee = 0xffeeffee; + heap->auto_flags = (flags & HEAP_GROWABLE); + heap->flags = (flags & ~HEAP_SHARED); heap->shared = (flags & HEAP_SHARED) != 0; heap->magic = HEAP_MAGIC; heap->grow_size = max( HEAP_DEF_SIZE, totalSize ); @@ -1438,6 +1441,25 @@ static BOOL validate_block_pointer( HEAP *heap, SUBHEAP **ret_subheap, const ARE return ret; }
+static DWORD heap_flags_from_global_flag( DWORD flag ) +{ + DWORD ret = 0; + + if (flag & FLG_HEAP_ENABLE_TAIL_CHECK) + ret |= HEAP_TAIL_CHECKING_ENABLED; + if (flag & FLG_HEAP_ENABLE_FREE_CHECK) + ret |= HEAP_FREE_CHECKING_ENABLED; + if (flag & FLG_HEAP_VALIDATE_PARAMETERS) + ret |= HEAP_VALIDATE_PARAMS | HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED; + if (flag & FLG_HEAP_VALIDATE_ALL) + ret |= HEAP_VALIDATE_ALL | HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED; + if (flag & FLG_HEAP_DISABLE_COALESCING) + ret |= HEAP_DISABLE_COALESCE_ON_FREE; + if (flag & FLG_HEAP_PAGE_ALLOCS) + ret |= HEAP_PAGE_ALLOCS; + + return ret; +}
/*********************************************************************** * heap_set_debug_flags @@ -1446,27 +1468,21 @@ static void heap_set_debug_flags( HANDLE handle ) { HEAP *heap = HEAP_GetPtr( handle ); ULONG global_flags = RtlGetNtGlobalFlags(); - ULONG flags = 0; + DWORD flags, force_flags;
if (TRACE_ON(heap)) global_flags |= FLG_HEAP_VALIDATE_ALL; if (WARN_ON(heap)) global_flags |= FLG_HEAP_VALIDATE_PARAMETERS;
- if (global_flags & FLG_HEAP_ENABLE_TAIL_CHECK) flags |= HEAP_TAIL_CHECKING_ENABLED; - if (global_flags & FLG_HEAP_ENABLE_FREE_CHECK) flags |= HEAP_FREE_CHECKING_ENABLED; - if (global_flags & FLG_HEAP_DISABLE_COALESCING) flags |= HEAP_DISABLE_COALESCE_ON_FREE; - if (global_flags & FLG_HEAP_PAGE_ALLOCS) flags |= HEAP_PAGE_ALLOCS | HEAP_GROWABLE; + flags = heap_flags_from_global_flag( global_flags ); + force_flags = (heap->flags | flags) & ~(HEAP_SHARED|HEAP_DISABLE_COALESCE_ON_FREE);
- if (global_flags & FLG_HEAP_VALIDATE_PARAMETERS) - flags |= HEAP_VALIDATE | HEAP_VALIDATE_PARAMS | - HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED; - if (global_flags & FLG_HEAP_VALIDATE_ALL) - flags |= HEAP_VALIDATE | HEAP_VALIDATE_ALL | - HEAP_TAIL_CHECKING_ENABLED | HEAP_FREE_CHECKING_ENABLED; + if (global_flags & FLG_HEAP_ENABLE_TAGGING) flags |= HEAP_SHARED; + if (!(global_flags & FLG_HEAP_PAGE_ALLOCS)) force_flags &= ~(HEAP_GROWABLE|HEAP_PRIVATE);
if (RUNNING_ON_VALGRIND) flags = 0; /* no sense in validating since Valgrind catches accesses */
heap->flags |= flags; - heap->force_flags |= flags & ~(HEAP_VALIDATE | HEAP_DISABLE_COALESCE_ON_FREE); + heap->force_flags |= force_flags;
if (flags & (HEAP_FREE_CHECKING_ENABLED | HEAP_TAIL_CHECKING_ENABLED)) /* fix existing blocks */ { @@ -1541,11 +1557,9 @@ HANDLE WINAPI RtlCreateHeap( ULONG flags, PVOID addr, SIZE_T totalSize, SIZE_T c
/* Allocate the heap block */
- if (!totalSize) - { - totalSize = HEAP_DEF_SIZE; - flags |= HEAP_GROWABLE; - } + if (processHeap) flags |= HEAP_PRIVATE; + if (!processHeap || !totalSize || (flags & HEAP_SHARED)) flags |= HEAP_GROWABLE; + if (!totalSize) totalSize = HEAP_DEF_SIZE;
if (!(subheap = HEAP_CreateSubHeap( NULL, addr, flags, commitSize, totalSize ))) return 0;
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=112853
Your paranoid android.
=== debian11 (64 bit WoW report) ===
ntdll: env.c:461: Test failed: wrong end ptr 000000000003197C/0000000000031980
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 126 +++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 104eced3bf4..7195907ff03 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -37,6 +37,9 @@ #define HEAP_VALIDATE_ALL 0x20000000 #define HEAP_VALIDATE_PARAMS 0x40000000
+#define BLOCK_ALIGN (2 * sizeof(void *) - 1) +#define ALIGN_BLOCK_SIZE(x) (((x) + BLOCK_ALIGN) & ~BLOCK_ALIGN) + /* use function pointers to avoid warnings for invalid parameter tests */ static LPVOID (WINAPI *pHeapAlloc)(HANDLE,DWORD,SIZE_T); static LPVOID (WINAPI *pHeapReAlloc)(HANDLE,DWORD,LPVOID,SIZE_T); @@ -1729,6 +1732,128 @@ static void test_LocalAlloc(void) } }
+static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ) +{ + DWORD padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS; + SIZE_T expect_size, alloc_size, tail_size, extra_size; + unsigned char *ptr0, *ptr1, *ptr2; + char tail_buf[64], padd_buf[64]; + BOOL ret; + + if (global_flags & (FLG_HEAP_DISABLE_COALESCING|FLG_HEAP_PAGE_ALLOCS|FLG_POOL_ENABLE_TAGGING| + FLG_HEAP_ENABLE_TAGGING|FLG_HEAP_ENABLE_TAG_BY_DLL)) + { + skip( "skipping not yet implemented block layout tests\n" ); + return; + } + + if (!global_flags) extra_size = 8; + else extra_size = 2 * sizeof(void *); + if (heap_flags & HEAP_TAIL_CHECKING_ENABLED) extra_size += 2 * sizeof(void *); + if (heap_flags & padd_flags) extra_size += 2 * sizeof(void *); + + if (!(heap_flags & HEAP_TAIL_CHECKING_ENABLED)) tail_size = 0; + else tail_size = 2 * sizeof(void *); + + memset(padd_buf, 0, sizeof(padd_buf)); + memset(tail_buf, 0xab, sizeof(tail_buf)); + + for (alloc_size = 0x20000 * sizeof(void *) - 0x3000 - extra_size; alloc_size > 0; alloc_size >>= 1) + { + winetest_push_context( "size %#Ix", alloc_size ); + + ptr0 = pHeapAlloc( heap, HEAP_ZERO_MEMORY, alloc_size ); + ok( !!ptr0, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr1 = HeapAlloc( heap, HEAP_ZERO_MEMORY, alloc_size ); + ok( !!ptr1, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr2 = HeapAlloc( heap, HEAP_ZERO_MEMORY, alloc_size ); + ok( !!ptr2, "HeapAlloc failed, error %lu\n", GetLastError() ); + + ok( !((UINT_PTR)ptr0 & (2 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + ok( !((UINT_PTR)ptr1 & (2 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + ok( !((UINT_PTR)ptr2 & (2 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + + expect_size = max( alloc_size, 2 * sizeof(void *) ); + expect_size = ALIGN_BLOCK_SIZE( expect_size + extra_size ); + todo_wine_if( heap_flags & (HEAP_VALIDATE_PARAMS|HEAP_VALIDATE_ALL) || + min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ) != expect_size ) + ok( min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ) == expect_size, "got diff %#I64x\n", + min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ) ); + + 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" ); + + ret = HeapFree( heap, 0, ptr2 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr1 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr0 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + + winetest_pop_context(); + } + + + /* between the two thesholds, tail may still be set but block position is inconsistent */ + + alloc_size = 0x20000 * sizeof(void *) - 0x2000 - extra_size; + winetest_push_context( "size %#Ix", alloc_size ); + + ptr0 = pHeapAlloc( heap, HEAP_ZERO_MEMORY, alloc_size ); + ok( !!ptr0, "HeapAlloc failed, error %lu\n", GetLastError() ); + ok( !((UINT_PTR)ptr0 & (2 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + + ok( !memcmp( ptr0 + alloc_size, tail_buf, tail_size ), "missing block tail\n" ); + + ret = HeapFree( heap, 0, ptr0 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + + winetest_pop_context(); + + + for (alloc_size = 0x20000 * sizeof(void *) - 0x1000 - extra_size + 1; alloc_size < 0x800000; alloc_size <<= 1) + { + winetest_push_context( "size %#Ix", alloc_size ); + + ptr0 = pHeapAlloc( heap, HEAP_ZERO_MEMORY, alloc_size ); + ok( !!ptr0, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr1 = HeapAlloc( heap, 0, alloc_size ); + ok( !!ptr1, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr2 = HeapAlloc( heap, 0, alloc_size ); + ok( !!ptr2, "HeapAlloc failed, error %lu\n", GetLastError() ); + + todo_wine_if( sizeof(void *) == 8 || alloc_size == 0x7efe9 ) + ok( !((UINT_PTR)ptr0 & (8 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + todo_wine_if( sizeof(void *) == 8 || alloc_size == 0x7efe9 ) + ok( !((UINT_PTR)ptr1 & (8 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + todo_wine_if( sizeof(void *) == 8 || alloc_size == 0x7efe9 ) + ok( !((UINT_PTR)ptr2 & (8 * sizeof(void *) - 1)), "got unexpected ptr align\n" ); + + expect_size = max( alloc_size, 2 * sizeof(void *) ); + expect_size = ALIGN_BLOCK_SIZE( expect_size + extra_size ); + todo_wine_if( alloc_size == 0x7efe9 ) + ok( min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ) > expect_size, "got diff %#I64x\n", + min( llabs( ptr2 - ptr1 ), llabs( ptr1 - ptr0 ) ) ); + + todo_wine_if( heap_flags & HEAP_TAIL_CHECKING_ENABLED ) + ok( !ptr0[alloc_size], "got tail\n" ); + todo_wine_if( heap_flags & HEAP_TAIL_CHECKING_ENABLED ) + ok( !ptr1[alloc_size], "got tail\n" ); + todo_wine_if( heap_flags & HEAP_TAIL_CHECKING_ENABLED ) + ok( !ptr2[alloc_size], "got tail\n" ); + + ret = HeapFree( heap, 0, ptr2 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr1 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr0 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + + winetest_pop_context(); + } +} + static void test_heap_checks( DWORD flags ) { BYTE old, *p, *p2; @@ -2043,6 +2168,7 @@ static void test_child_heap( const char *arg ) heap = HeapCreate( HEAP_NO_SERIALIZE, 0, 0 ); ok( heap != GetProcessHeap(), "got unexpected heap\n" ); test_heap_layout( heap, global_flags, heap_flags|HEAP_NO_SERIALIZE|HEAP_GROWABLE|HEAP_PRIVATE ); + test_block_layout( heap, global_flags, heap_flags|HEAP_NO_SERIALIZE|HEAP_GROWABLE|HEAP_PRIVATE ); ret = HeapDestroy( heap ); ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() );
On 4/19/22 19:33, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/kernel32/tests/heap.c | 126 +++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 104eced3bf4..7195907ff03 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -37,6 +37,9 @@ #define HEAP_VALIDATE_ALL 0x20000000 #define HEAP_VALIDATE_PARAMS 0x40000000
+#define BLOCK_ALIGN (2 * sizeof(void *) - 1) +#define ALIGN_BLOCK_SIZE(x) (((x) + BLOCK_ALIGN) & ~BLOCK_ALIGN)
- /* use function pointers to avoid warnings for invalid parameter tests */ static LPVOID (WINAPI *pHeapAlloc)(HANDLE,DWORD,SIZE_T); static LPVOID (WINAPI *pHeapReAlloc)(HANDLE,DWORD,LPVOID,SIZE_T);
@@ -1729,6 +1732,128 @@ static void test_LocalAlloc(void) } }
+static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ) +{
- DWORD padd_flags = HEAP_VALIDATE | HEAP_VALIDATE_ALL | HEAP_VALIDATE_PARAMS;
- SIZE_T expect_size, alloc_size, tail_size, extra_size;
- unsigned char *ptr0, *ptr1, *ptr2;
- char tail_buf[64], padd_buf[64];
- BOOL ret;
- if (global_flags & (FLG_HEAP_DISABLE_COALESCING|FLG_HEAP_PAGE_ALLOCS|FLG_POOL_ENABLE_TAGGING|
FLG_HEAP_ENABLE_TAGGING|FLG_HEAP_ENABLE_TAG_BY_DLL))- {
skip( "skipping not yet implemented block layout tests\n" );return;- }
- if (!global_flags) extra_size = 8;
- else extra_size = 2 * sizeof(void *);
- if (heap_flags & HEAP_TAIL_CHECKING_ENABLED) extra_size += 2 * sizeof(void *);
- if (heap_flags & padd_flags) extra_size += 2 * sizeof(void *);
- if (!(heap_flags & HEAP_TAIL_CHECKING_ENABLED)) tail_size = 0;
- else tail_size = 2 * sizeof(void *);
- memset(padd_buf, 0, sizeof(padd_buf));
- memset(tail_buf, 0xab, sizeof(tail_buf));
Hmm, padd_buf is only really used in PATCH 7. Doesn't seem to cause any warning but I can re-split and resend 5-7 later if it's better.
Returning TRUE from RtlGetUserInfoHeap as tests in next patch suggest it always does.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/heap.c | 30 ++++++++++++++++++++++++++++++ dlls/ntdll/ntdll.spec | 6 +++--- 2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index 298c39f24ff..566513d2251 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -2273,3 +2273,33 @@ NTSTATUS WINAPI RtlSetHeapInformation( HANDLE heap, HEAP_INFORMATION_CLASS info_ FIXME("%p %d %p %ld stub\n", heap, info_class, info, size); return STATUS_SUCCESS; } + +/*********************************************************************** + * RtlGetUserInfoHeap (NTDLL.@) + */ +BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE heap, ULONG flags, void *ptr, void **user_value, ULONG *user_flags ) +{ + FIXME( "heap %p, flags %#x, ptr %p, user_value %p, user_flags %p semi-stub!\n", + heap, flags, ptr, user_value, user_flags ); + *user_value = NULL; + *user_flags = 0; + return TRUE; +} + +/*********************************************************************** + * RtlSetUserValueHeap (NTDLL.@) + */ +BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE heap, 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; +} + +/*********************************************************************** + * RtlSetUserFlagsHeap (NTDLL.@) + */ +BOOLEAN WINAPI RtlSetUserFlagsHeap( HANDLE heap, ULONG flags, void *ptr, ULONG user_flags ) +{ + FIXME( "heap %p, flags %#x, ptr %p, user_flags %#x stub!\n", heap, flags, ptr, user_flags ); + return FALSE; +} diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 311f99be054..6b00de53940 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -748,7 +748,7 @@ @ stdcall RtlGetThreadPreferredUILanguages(long ptr ptr ptr) @ stdcall RtlGetUnloadEventTrace() @ stdcall RtlGetUnloadEventTraceEx(ptr ptr ptr) -@ stub RtlGetUserInfoHeap +@ stdcall RtlGetUserInfoHeap(ptr long ptr ptr ptr) @ stdcall RtlGetUserPreferredUILanguages(long long ptr ptr ptr) @ stdcall RtlGetVersion(ptr) @ stdcall -arch=arm,arm64,x86_64 RtlGrowFunctionTable(ptr long) @@ -1001,8 +1001,8 @@ # @ stub RtlSetTimer @ stdcall RtlSetUnhandledExceptionFilter(ptr) @ stub RtlSetUnicodeCallouts -@ stub RtlSetUserFlagsHeap -@ stub RtlSetUserValueHeap +@ stdcall RtlSetUserFlagsHeap(ptr long ptr long) +@ stdcall RtlSetUserValueHeap(ptr long ptr ptr) @ stdcall RtlSizeHeap(long long ptr) @ stdcall RtlSleepConditionVariableCS(ptr ptr ptr) @ stdcall RtlSleepConditionVariableSRW(ptr ptr ptr long)
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=112855
Your paranoid android.
=== debian11 (64 bit WoW report) ===
ntdll: env.c:461: Test failed: wrong end ptr 000000000003197C/0000000000031980
And corresponding RtlSetUserValueHeap and RtlSetUserFlagsHeap tests.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/kernel32/tests/heap.c | 103 +++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+)
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index 7195907ff03..a16084160d7 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -31,6 +31,7 @@ #include "wine/test.h"
/* some undocumented flags (names are made up) */ +#define HEAP_ADD_USER_INFO 0x00000100 #define HEAP_PRIVATE 0x00001000 #define HEAP_PAGE_ALLOCS 0x01000000 #define HEAP_VALIDATE 0x10000000 @@ -44,6 +45,9 @@ static LPVOID (WINAPI *pHeapAlloc)(HANDLE,DWORD,SIZE_T); static LPVOID (WINAPI *pHeapReAlloc)(HANDLE,DWORD,LPVOID,SIZE_T); static BOOL (WINAPI *pGetPhysicallyInstalledSystemMemory)( ULONGLONG * ); +static BOOLEAN (WINAPI *pRtlGetUserInfoHeap)(HANDLE,ULONG,void*,void**,ULONG*); +static BOOLEAN (WINAPI *pRtlSetUserValueHeap)(HANDLE,ULONG,void*,void*); +static BOOLEAN (WINAPI *pRtlSetUserFlagsHeap)(HANDLE,ULONG,void*,ULONG);
#define MAKE_FUNC(f) static typeof(f) *p ## f MAKE_FUNC( HeapQueryInformation ); @@ -65,6 +69,9 @@ static void load_functions(void) LOAD_FUNC( kernel32, GetPhysicallyInstalledSystemMemory ); LOAD_FUNC( kernel32, GlobalFlags ); LOAD_FUNC( ntdll, RtlGetNtGlobalFlags ); + LOAD_FUNC( ntdll, RtlGetUserInfoHeap ); + LOAD_FUNC( ntdll, RtlSetUserValueHeap ); + LOAD_FUNC( ntdll, RtlSetUserFlagsHeap ); #undef LOAD_FUNC }
@@ -910,6 +917,7 @@ static void test_GlobalAlloc(void) SIZE_T size, alloc_size; HGLOBAL mem, tmp_mem; BYTE *ptr, *tmp_ptr; + ULONG tmp_flags; UINT i, flags; BOOL ret;
@@ -1029,6 +1037,25 @@ static void test_GlobalAlloc(void) todo_wine ok( size == alloc_size, "HeapSize returned %Iu\n", size );
+ tmp_mem = invalid_mem; + tmp_flags = 0xdeadbeef; + ret = pRtlGetUserInfoHeap( GetProcessHeap(), 0, entry->ptr, (void **)&tmp_mem, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + todo_wine + 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 ); + todo_wine + ok( ret, "RtlSetUserValueHeap failed, error %lu\n", GetLastError() ); + tmp_mem = GlobalHandle( entry->ptr ); + todo_wine + ok( tmp_mem == invalid_mem, "GlobalHandle returned unexpected handle\n" ); + ret = pRtlSetUserValueHeap( GetProcessHeap(), 0, entry->ptr, mem ); + todo_wine + ok( ret, "RtlSetUserValueHeap failed, error %lu\n", GetLastError() ); + ptr = GlobalLock( mem ); ok( !!ptr, "GlobalLock failed, error %lu\n", GetLastError() ); ok( ptr != mem, "got unexpected ptr %p\n", ptr ); @@ -1738,6 +1765,8 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags SIZE_T expect_size, alloc_size, tail_size, extra_size; unsigned char *ptr0, *ptr1, *ptr2; char tail_buf[64], padd_buf[64]; + ULONG tmp_flags; + void *tmp_ptr; BOOL ret;
if (global_flags & (FLG_HEAP_DISABLE_COALESCING|FLG_HEAP_PAGE_ALLOCS|FLG_POOL_ENABLE_TAGGING| @@ -1849,7 +1878,81 @@ static void test_block_layout( HANDLE heap, DWORD global_flags, DWORD heap_flags ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); ret = HeapFree( heap, 0, ptr0 ); ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + winetest_pop_context(); + }
+ /* Undocumented HEAP_ADD_USER_INFO flag can be used to force an additional padding + * on small block sizes. It's possibly where the user info is stored, although it + * doesn't seem to be consistent and maybe has been moved elsewhere. Larger block + * seem to store the user flags and info in their header. + * + * RtlGetUserInfoHeap also requires the flag to work consistently, and otherwise + * causes crashes when heap flags are used, or on 32-bit. + */ + if (!(heap_flags & padd_flags)) + { + alloc_size = 0x1000; + winetest_push_context( "size %#Ix", alloc_size ); + ptr0 = pHeapAlloc( heap, 0xe00|HEAP_ADD_USER_INFO, alloc_size ); + ok( !!ptr0, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr1 = HeapAlloc( heap, 0x200|HEAP_ADD_USER_INFO, alloc_size ); + ok( !!ptr1, "HeapAlloc failed, error %lu\n", GetLastError() ); + ptr2 = HeapAlloc( heap, HEAP_ADD_USER_INFO, alloc_size ); + ok( !!ptr2, "HeapAlloc failed, error %lu\n", GetLastError() ); + + expect_size = max( alloc_size, 2 * sizeof(void *) ); + expect_size = ALIGN_BLOCK_SIZE( expect_size + extra_size + 2 * sizeof(void *) ); + todo_wine_if( heap_flags & (HEAP_VALIDATE_PARAMS|HEAP_VALIDATE_ALL) || + (ptr2 - ptr1 != expect_size && ptr1 - ptr0 != expect_size) ) + ok( ptr2 - ptr1 == expect_size || ptr1 - ptr0 == expect_size, + "got diff %#Ix %#Ix\n", ptr2 - ptr1, ptr1 - ptr0 ); + + 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 + ok( !memcmp( ptr0 + alloc_size + tail_size, padd_buf, 2 * sizeof(void *) ), "unexpected padding\n" ); + + tmp_ptr = (void *)0xdeadbeef; + tmp_flags = 0xdeadbeef; + ret = pRtlGetUserInfoHeap( heap, 0, ptr0, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + ok( tmp_ptr == (void *)NULL, "got ptr %p\n", tmp_ptr ); + todo_wine + ok( tmp_flags == 0xe00, "got flags %#lx\n", tmp_flags ); + + tmp_ptr = (void *)0xdeadbeef; + tmp_flags = 0xdeadbeef; + ret = pRtlGetUserInfoHeap( heap, 0, ptr1, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + ok( tmp_ptr == (void *)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 ); + todo_wine + ok( ret, "RtlSetUserValueHeap failed, error %lu\n", GetLastError() ); + ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0xdeadbeef ); + ok( !ret, "RtlSetUserFlagsHeap failed, error %lu\n", GetLastError() ); + ret = pRtlSetUserFlagsHeap( heap, 0, ptr0, 0x200 ); + ok( !ret, "RtlSetUserFlagsHeap failed, error %lu\n", GetLastError() ); + + tmp_ptr = NULL; + tmp_flags = 0; + ret = pRtlGetUserInfoHeap( heap, 0, ptr0, (void **)&tmp_ptr, &tmp_flags ); + ok( ret, "RtlGetUserInfoHeap failed, error %lu\n", GetLastError() ); + todo_wine + ok( tmp_ptr == (void *)0xdeadbeef, "got ptr %p\n", tmp_ptr ); + todo_wine + ok( tmp_flags == 0xe00, "got flags %#lx\n", tmp_flags ); + + ret = HeapFree( heap, 0, ptr2 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr1 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); + ret = HeapFree( heap, 0, ptr0 ); + ok( ret, "HeapFree failed, error %lu\n", GetLastError() ); winetest_pop_context(); } }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=112856
Your paranoid android.
=== debian11 (32 bit Arabic:Morocco report) ===
kernel32: heap.c:2376: Test failed: got ullAvailPhys 0x9ff8a000 heap.c:2378: Test failed: got ullAvailPageFile 0xcf60d000
-
Marvin -
Rémi Bernon