[PATCH 0/1] MR7790: ntdll: Do not mark first stack guard page as committed.
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 29 +++++++++++++++++++++++++---- dlls/ntdll/thread.c | 3 +-- dlls/ntdll/unix/virtual.c | 4 ++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 207f0dca43d..366122dbe0c 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -1014,8 +1014,8 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#lx, expected %#x\n", mbi.AllocationProtect, PAGE_READWRITE ); ok( mbi.BaseAddress == addr, "unexpected BaseAddress %p, expected %p\n", mbi.BaseAddress, addr ); - todo_wine ok( mbi.State == MEM_RESERVE, "unexpected State %#lx, expected %#x\n", mbi.State, MEM_RESERVE ); - todo_wine ok( mbi.Protect == 0, "unexpected Protect %#lx, expected %#x\n", mbi.Protect, 0 ); + ok( mbi.State == MEM_RESERVE, "unexpected State %#lx, expected %#x\n", mbi.State, MEM_RESERVE ); + ok( mbi.Protect == 0, "unexpected Protect %#lx, expected %#x\n", mbi.Protect, 0 ); ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#lx, expected %#x\n", mbi.Type, MEM_PRIVATE ); @@ -1035,8 +1035,8 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#lx, expected %#x\n", mbi.AllocationProtect, PAGE_READWRITE ); ok( mbi.BaseAddress == addr, "unexpected BaseAddress %p, expected %p\n", mbi.BaseAddress, addr ); - todo_wine ok( mbi.State == MEM_RESERVE, "unexpected State %#lx, expected %#x\n", mbi.State, MEM_RESERVE ); - todo_wine ok( mbi.Protect == 0, "unexpected Protect %#lx, expected %#x\n", mbi.Protect, 0 ); + ok( mbi.State == MEM_RESERVE, "unexpected State %#lx, expected %#x\n", mbi.State, MEM_RESERVE ); + ok( mbi.Protect == 0, "unexpected Protect %#lx, expected %#x\n", mbi.Protect, 0 ); ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#lx, expected %#x\n", mbi.Type, MEM_PRIVATE ); guard_size = reserved - committed - mbi.RegionSize; @@ -1249,11 +1249,13 @@ static void test_RtlCreateUserStack(void) struct test_stack_size_thread_args args; SIZE_T default_commit = nt->OptionalHeader.SizeOfStackCommit; SIZE_T default_reserve = nt->OptionalHeader.SizeOfStackReserve; + MEMORY_BASIC_INFORMATION mbi; INITIAL_TEB stack = {0}; unsigned int i; NTSTATUS ret; HANDLE thread; CLIENT_ID id; + SIZE_T szret; struct { @@ -1267,6 +1269,7 @@ static void test_RtlCreateUserStack(void) { 0, 0x200000, 1, 1, default_commit, 0x200000}, { 0x4000, 0x200000, 1, 1, 0x4000, 0x200000}, {0x100000, 0x100000, 1, 1, 0x100000, 0x100000}, + { 0xff000, 0x100000, 1, 1, 0xff000, 0x100000}, { 0x20000, 0x20000, 1, 1, 0x20000, 0x100000}, { 0, 0x110000, 1, 1, default_commit, 0x110000}, @@ -1299,6 +1302,24 @@ static void test_RtlCreateUserStack(void) "%u: got reserve %#Ix\n", i, (ULONG_PTR)stack.StackBase - (ULONG_PTR)stack.DeallocationStack); todo_wine ok((ULONG_PTR)stack.StackBase - (ULONG_PTR)stack.StackLimit == tests[i].expect_commit, "%u: got commit %#Ix\n", i, (ULONG_PTR)stack.StackBase - (ULONG_PTR)stack.StackLimit); + szret = VirtualQuery(stack.DeallocationStack, &mbi, sizeof(mbi)); + ok(szret == sizeof(mbi), "got %Iu.\n", szret); + ok(mbi.AllocationBase == stack.DeallocationStack, "got %p, %p.\n", mbi.AllocationBase, stack.DeallocationStack); + if (tests[i].commit + 2 * page_size <= max( tests[i].reserve, 0x100000)) + { + ok(mbi.State == MEM_RESERVE, "%u: got %#lx.\n", i, mbi.State); + ok(!mbi.Protect, "%u: got %#lx.\n", i, mbi.Protect); + } + else if (tests[i].commit + page_size <= max( tests[i].reserve, 0x100000)) + { + todo_wine ok(mbi.State == MEM_COMMIT, "%u: got %#lx.\n", i, mbi.State); + todo_wine ok(mbi.Protect == (PAGE_READWRITE | PAGE_GUARD), "%u: got %#lx.\n", i, mbi.Protect); + } + else + { + todo_wine ok(mbi.State == MEM_COMMIT, "%u: got %#lx.\n", i, mbi.State); + todo_wine ok(mbi.Protect == PAGE_READWRITE, "%u: got %#lx.\n", i, mbi.Protect); + } pRtlFreeUserStack(stack.DeallocationStack); } diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c index 1bb4c266bd3..ee784fb2db2 100644 --- a/dlls/ntdll/thread.c +++ b/dlls/ntdll/thread.c @@ -333,10 +333,9 @@ NTSTATUS WINAPI RtlCreateUserStack( SIZE_T commit, SIZE_T reserve, ULONG zero_bi &alloc, sizeof(alloc) ); if (!status) { - void *addr = alloc.StackBase; + void *addr; SIZE_T size = page_size; - NtAllocateVirtualMemory( GetCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_NOACCESS ); addr = (char *)alloc.StackBase + page_size; NtAllocateVirtualMemory( GetCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE | PAGE_GUARD ); addr = (char *)alloc.StackBase + 2 * page_size; diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 2838472e7ba..ca5d50b0fe6 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -4053,7 +4053,7 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, ULONG_PTR limit_low, UL /* setup no access guard page */ if (guard_page) { - set_page_vprot( view->base, host_page_size, VPROT_COMMITTED ); + set_page_vprot( view->base, host_page_size, 0 ); set_page_vprot( (char *)view->base + host_page_size, host_page_size, VPROT_READ | VPROT_WRITE | VPROT_COMMITTED | VPROT_GUARD ); mprotect_range( view->base, 2 * host_page_size , 0, 0 ); @@ -4143,7 +4143,7 @@ static NTSTATUS grow_thread_stack( char *page, struct thread_stack_info *stack_i { NTSTATUS ret = 0; - set_page_vprot_bits( page, host_page_size, 0, VPROT_GUARD ); + set_page_vprot_bits( page, host_page_size, VPROT_COMMITTED, VPROT_GUARD ); mprotect_range( page, host_page_size, 0, 0 ); if (page >= stack_info->start + host_page_size + stack_info->guaranteed) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7790
This helps Champions of Anteria to start. The game zeros the data below the current stack pointer in its threads for some reason and established the bottom of the stack roughly like that: ``` VirtualQuery(esp, &d); stack_bottom = d.AllocationBase; while (1) { VirtualQuery(stack_bottom, &d); stack_bottom += d.RegionSize; if (d.State & MEM_RESERVE) continue; if (!(d.State & MEM_COMMIT)) continue; if (d.Protect & PAGE_GUARD) continue; break; } stack_bottom -= d.RegionSize; ``` That depends on the first page(s) at the bottom of the stack to be reserved but uncommitted (which is the case on Windows and basically covered by the existing tests). On Windows also the initially uncommitted stack part all goes as reserved followed by guard page and committed stack area; on Wine the stack currently looks like fully reserved but we still have guard page pattern in the bottom which I think we can mark the same way even if not currently moving stack bottom dynamically. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7790#note_100608
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)