[PATCH v4 0/1] MR8852: ntdll: Zero aligned size in initialize_block().
-- v4: ntdll: Zero aligned size in initialize_block(). https://gitlab.winehq.org/wine/wine/-/merge_requests/8852
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/heap.c | 62 ++++++++++++++++++++++++++++++++++++++ dlls/ntdll/heap.c | 8 +++-- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index f88a140a115..2991c5563d5 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -41,6 +41,8 @@ #define BLOCK_ALIGN (2 * sizeof(void *) - 1) #define ALIGN_BLOCK_SIZE(x) (((x) + BLOCK_ALIGN) & ~BLOCK_ALIGN) +#define ROUND_SIZE(size, mask) ((((SIZE_T)(size) + (mask)) & ~(SIZE_T)(mask))) + /* 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); @@ -82,6 +84,21 @@ static void load_functions(void) #undef LOAD_FUNC } +static BOOL check_win_version(int min_major, int min_minor) +{ + HMODULE hntdll = GetModuleHandleA("ntdll.dll"); + NTSTATUS (WINAPI *pRtlGetVersion)(RTL_OSVERSIONINFOEXW *); + RTL_OSVERSIONINFOEXW rtlver; + + rtlver.dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW); + pRtlGetVersion = (void *)GetProcAddress(hntdll, "RtlGetVersion"); + pRtlGetVersion(&rtlver); + return rtlver.dwMajorVersion > min_major || + (rtlver.dwMajorVersion == min_major && + rtlver.dwMinorVersion >= min_minor); +} +#define is_win8_plus() check_win_version(6, 2) + struct heap { UINT_PTR unknown1[2]; @@ -3456,6 +3473,49 @@ static void test_heap_layout( HANDLE handle, DWORD global_flag, DWORD heap_flags } } +static void test_heap_tail_zeroing( DWORD heap_flags ) +{ + static const ULONG_PTR large_block_min_size = 65536 * (2 * sizeof(void *)); + BOOL before_win8 = !is_win8_plus(); + HANDLE heap = GetProcessHeap(); + size_t size, size_aligned; + ULONG_PTR v, expected; + char *p1, *p2; + + if (heap_flags & HEAP_PAGE_ALLOCS) + { + /* This behaves differently, no support yet. */ + skip( "Skipping test with HEAP_PAGE_ALLOCS.\n" ); + return; + } + + for (size = 1; size <= 1048576 * 2; size *= 2) + { + winetest_push_context( "heap_flags %#lx, size %Iu", heap_flags, size ); + p1 = pHeapAlloc( heap, 0, size + 1 ); + ok( !!p1, "got NULL.\n" ); + size_aligned = ROUND_SIZE(size + 1, sizeof(void *) - 1); + /* This and read access below is going to make valgrind or ASAN unhappy but the purpose of this test is + * to specifically check what happens with the tail bytes following the allocation. */ + if (!(heap_flags & (HEAP_VALIDATE_PARAMS | HEAP_VALIDATE_ALL))) + memset( p1, 0xcc, size_aligned ); + pHeapFree( heap, 0, p1 ); + + /* We are not guarenteed to get the same pointer here but that often happens, especially when the test + * is run first, and spoling the data before that adds certainity to the results. */ + p2 = pHeapAlloc( heap, HEAP_ZERO_MEMORY, size + 1 ); + ok( !!p2, "got NULL.\n" ); + v = 0; + memcpy( &v, p2 + size, size_aligned - size ); + expected = 0; + if (size_aligned - size > 1 && size + 1 < large_block_min_size && heap_flags & HEAP_TAIL_CHECKING_ENABLED) + memset( (char *)&expected + 1, 0xab, size_aligned - size - 1 ); + ok( v == expected || broken( before_win8 && !expected ), "got %#Ix, expected %#Ix.\n", v, expected ); + pHeapFree( heap, 0, p2 ); + winetest_pop_context(); + } +} + static void test_child_heap( const char *arg ) { char buffer[32]; @@ -3535,6 +3595,7 @@ static void test_child_heap( const char *arg ) ok( ret, "HeapDestroy failed, error %lu\n", GetLastError() ); test_heap_checks( heap_flags ); + test_heap_tail_zeroing( heap_flags ); } static void test_GetPhysicallyInstalledSystemMemory(void) @@ -3808,6 +3869,7 @@ START_TEST(heap) test_GetPhysicallyInstalledSystemMemory(); test_GlobalMemoryStatus(); test_HeapSummary(); + test_heap_tail_zeroing( 0 ); if (pRtlGetNtGlobalFlags) { diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index e761268095e..c68c449a524 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -503,14 +503,16 @@ static inline void mark_block_tail( struct block *block, DWORD flags ) static inline void initialize_block( struct block *block, SIZE_T old_size, SIZE_T size, DWORD flags ) { char *data = (char *)(block + 1); - SIZE_T i; + SIZE_T i, aligned_size; if (size <= old_size) return; if (flags & HEAP_ZERO_MEMORY) { - valgrind_make_writable( data + old_size, size - old_size ); - memset( data + old_size, 0, size - old_size ); + aligned_size = ROUND_SIZE( size, sizeof(void *) - 1 ); + valgrind_make_writable( data + old_size, aligned_size - old_size ); + memset( data + old_size, 0, aligned_size - old_size ); + valgrind_make_noaccess( data + size, aligned_size - size ); } else if (flags & HEAP_FREE_CHECKING_ENABLED) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8852
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8852
v4: - define and use ROUND_SIZE() macro in test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8852#note_114427
Alexandre Julliard (@julliard) commented about dlls/kernel32/tests/heap.c:
}
+static BOOL check_win_version(int min_major, int min_minor) +{ + HMODULE hntdll = GetModuleHandleA("ntdll.dll"); + NTSTATUS (WINAPI *pRtlGetVersion)(RTL_OSVERSIONINFOEXW *); + RTL_OSVERSIONINFOEXW rtlver; + + rtlver.dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW); + pRtlGetVersion = (void *)GetProcAddress(hntdll, "RtlGetVersion"); + pRtlGetVersion(&rtlver); + return rtlver.dwMajorVersion > min_major || + (rtlver.dwMajorVersion == min_major && + rtlver.dwMinorVersion >= min_minor); +} +#define is_win8_plus() check_win_version(6, 2) Please avoid version checks in tests. If absolutely necessary, check some feature of that platform instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8852#note_114436
On Mon Sep 1 17:58:39 2025 +0000, Alexandre Julliard wrote:
Please avoid version checks in tests. If absolutely necessary, check some feature of that platform instead. The problem here is that exactly this zeroing size is different on Windows 7, while I so far I don't know about any other clear heap relayout differences between Windows 7 and later.
@rbernon Do you maybe remember any heap info / flags or whatever heap related difference between heaps on Win7 and Win8? If not I will try harder to find some, while not sure upfront if that exists. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8852#note_114437
On Mon Sep 1 18:02:56 2025 +0000, Paul Gofman wrote:
The problem here is that exactly this zeroing size is different on Windows 7, while I so far I don't know about any other clear heap relayout differences between Windows 7 and later. @rbernon Do you maybe remember any heap info / flags or whatever heap related difference between heaps on Win7 and Win8? If not I will try harder to find some, while not sure upfront if that exists. Or we could simply stop worrying about making the tests pass on Windows 7, I'm not sure it brings much value these days...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8852#note_114438
participants (4)
-
Alexandre Julliard (＠julliard) -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Rémi Bernon