Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
I'm not sure at all about the layout of things in PATCH 2, or the safe stack size to clear, but the idea is to avoid touching all the pages.
Dragon Quest Builder 2 uses a default stack reserve size of 2G, and it quickly exhaust memory, especially with all the internal threads being created.
I also have some draft implementation of growing stacks on page fault instead, but I figured it was going to be a pain for debugging with all the additional signals being received.
Also is seems like that some unix libraries are reserving stack pages very far away from the current stack pointer, and it overflows the usual number of guard pages. We would then have to grow stack on uncommitted page faults too, or to mark all reserved pages as page guard (to the contrary to what the tests are showing).
dlls/ntdll/tests/virtual.c | 264 +++++++++++++++++++++++++++++++++++++ 1 file changed, 264 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 546a7d83bbd..112be80c01e 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -237,14 +237,266 @@ static void test_NtAllocateVirtualMemory(void) ok(status == STATUS_INVALID_PARAMETER, "NtAllocateVirtualMemoryEx returned %08x\n", status); }
+struct test_stack_size_thread_args +{ + DWORD expect_committed; + DWORD expect_reserved; +}; + +static void force_stack_grow(void) +{ + volatile int buffer[0x2000]; + buffer[0] = 0xdeadbeef; + (void)buffer[0]; +} + +#ifdef _WIN64 +static void force_stack_grow_small(void) +{ + volatile int buffer[0x400]; + buffer[0] = 0xdeadbeef; + (void)buffer[0]; +} +#endif + +static DWORD WINAPI test_stack_size_thread(void *ptr) +{ + struct test_stack_size_thread_args *args = ptr; + MEMORY_BASIC_INFORMATION mbi; + NTSTATUS status; + SIZE_T size, guard_size; + DWORD committed, reserved; + void *addr; +#ifdef _WIN64 + DWORD prot; + void *tmp; +#endif + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + reserved = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->DeallocationStack; + todo_wine ok( committed == args->expect_committed || broken(committed == 0x1000), "unexpected stack committed size %x, expected %x\n", committed, args->expect_committed ); + ok( reserved == args->expect_reserved, "unexpected stack reserved size %x, expected %x\n", reserved, args->expect_reserved ); + + addr = (char *)NtCurrentTeb()->DeallocationStack; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); + ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#x, 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 %#x, expected %#x\n", mbi.State, MEM_RESERVE ); + todo_wine ok( mbi.Protect == 0, "unexpected Protect %#x, expected %#x\n", mbi.Protect, 0 ); + ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#x, expected %#x\n", mbi.Type, MEM_PRIVATE ); + + + force_stack_grow(); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + reserved = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->DeallocationStack; + todo_wine ok( committed == 0x9000, "unexpected stack committed size %x, expected 9000\n", committed ); + ok( reserved == args->expect_reserved, "unexpected stack reserved size %x, expected %x\n", reserved, args->expect_reserved ); + + + /* reserved area shrinks whenever stack grows */ + + addr = (char *)NtCurrentTeb()->DeallocationStack; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); + ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#x, 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 %#x, expected %#x\n", mbi.State, MEM_RESERVE ); + todo_wine ok( mbi.Protect == 0, "unexpected Protect %#x, expected %#x\n", mbi.Protect, 0 ); + ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#x, expected %#x\n", mbi.Type, MEM_PRIVATE ); + + guard_size = reserved - committed - mbi.RegionSize; + ok( guard_size == 0x1000 || guard_size == 0x2000 || guard_size == 0x3000, "unexpected guard_size %I64x, expected 1000, 2000 or 3000\n", (UINT64)guard_size ); + + /* the commit area is initially preceded by guard pages */ + + addr = (char *)NtCurrentTeb()->DeallocationStack + mbi.RegionSize; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); + ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#x, expected %#x\n", mbi.AllocationProtect, PAGE_READWRITE ); + ok( mbi.BaseAddress == addr, "unexpected BaseAddress %p, expected %p\n", mbi.BaseAddress, addr ); + ok( mbi.RegionSize == guard_size, "unexpected RegionSize %I64x, expected 3000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == (PAGE_READWRITE|PAGE_GUARD), "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE|PAGE_GUARD ); + ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#x, expected %#x\n", mbi.Type, MEM_PRIVATE ); + + addr = (char *)NtCurrentTeb()->Tib.StackLimit; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.AllocationBase == NtCurrentTeb()->DeallocationStack, "unexpected AllocationBase %p, expected %p\n", mbi.AllocationBase, NtCurrentTeb()->DeallocationStack ); + ok( mbi.AllocationProtect == PAGE_READWRITE, "unexpected AllocationProtect %#x, expected %#x\n", mbi.AllocationProtect, PAGE_READWRITE ); + ok( mbi.BaseAddress == addr, "unexpected BaseAddress %p, expected %p\n", mbi.BaseAddress, addr ); + ok( mbi.RegionSize == committed, "unexpected RegionSize %I64x, expected %I64x\n", (UINT64)mbi.RegionSize, (UINT64)committed ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + ok( mbi.Type == MEM_PRIVATE, "unexpected Type %#x, expected %#x\n", mbi.Type, MEM_PRIVATE ); + + +#ifdef _WIN64 + /* setting a guard page shrinks stack automatically */ + + addr = (char *)NtCurrentTeb()->Tib.StackLimit + 0x2000; + size = 0x1000; + status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE | PAGE_GUARD ); + ok( !status, "NtAllocateVirtualMemory returned %08x\n", status ); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x6000, "unexpected stack committed size %x, expected 6000\n", committed ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), (char *)addr - 0x2000, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.RegionSize == 0x2000, "unexpected RegionSize %I64x, expected 2000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.RegionSize == 0x1000, "unexpected RegionSize %I64x, expected 1000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == (PAGE_READWRITE|PAGE_GUARD), "unexpected Protect %#x, expected %#x\n", mbi.Protect, (PAGE_READWRITE|PAGE_GUARD) ); + + addr = (char *)NtCurrentTeb()->Tib.StackLimit; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + todo_wine ok( mbi.RegionSize == 0x6000, "unexpected RegionSize %I64x, expected 6000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + + /* guard pages are restored as the stack grows back */ + + addr = (char *)NtCurrentTeb()->Tib.StackLimit + 0x4000; + tmp = (char *)addr - guard_size - 0x1000; + size = 0x1000; + status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE | PAGE_GUARD ); + ok( !status, "NtAllocateVirtualMemory returned %08x\n", status ); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x1000, "unexpected stack committed size %x, expected 1000\n", committed ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), tmp, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + todo_wine ok( mbi.RegionSize == guard_size + 0x1000, "unexpected RegionSize %I64x, expected %I64x\n", (UINT64)mbi.RegionSize, (UINT64)(guard_size + 0x1000) ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + force_stack_grow_small(); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x2000, "unexpected stack committed size %x, expected 2000\n", committed ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), tmp, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.RegionSize == 0x1000, "unexpected RegionSize %I64x, expected 1000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), (char *)tmp + 0x1000, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.RegionSize == guard_size, "unexpected RegionSize %I64x, expected %I64x\n", (UINT64)mbi.RegionSize, (UINT64)guard_size ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == (PAGE_READWRITE|PAGE_GUARD), "unexpected Protect %#x, expected %#x\n", mbi.Protect, (PAGE_READWRITE|PAGE_GUARD) ); + + + /* forcing stack limit over guard pages still shrinks the stack on page fault */ + + addr = (char *)tmp + guard_size + 0x1000; + size = 0x1000; + status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE | PAGE_GUARD ); + ok( !status, "NtAllocateVirtualMemory returned %08x\n", status ); + + NtCurrentTeb()->Tib.StackLimit = (char *)tmp; + + status = NtQueryVirtualMemory( NtCurrentProcess(), (char *)tmp + 0x1000, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + todo_wine ok( mbi.RegionSize == guard_size + 0x1000, "unexpected RegionSize %I64x, expected %I64x\n", (UINT64)mbi.RegionSize, (UINT64)(guard_size + 0x1000) ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == (PAGE_READWRITE|PAGE_GUARD), "unexpected Protect %#x, expected %#x\n", mbi.Protect, (PAGE_READWRITE|PAGE_GUARD) ); + + force_stack_grow_small(); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x2000, "unexpected stack committed size %x, expected 2000\n", committed ); + + + /* it works with NtProtectVirtualMemory as well */ + + force_stack_grow(); + + addr = (char *)NtCurrentTeb()->Tib.StackLimit + 0x2000; + size = 0x1000; + status = NtProtectVirtualMemory( NtCurrentProcess(), &addr, &size, PAGE_READWRITE | PAGE_GUARD, &prot ); + ok( !status, "NtProtectVirtualMemory returned %08x\n", status ); + todo_wine ok( prot == PAGE_READWRITE, "unexpected prot %#x, expected %#x\n", prot, PAGE_READWRITE ); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x6000, "unexpected stack committed size %x, expected 6000\n", committed ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), (char *)addr - 0x2000, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + todo_wine ok( mbi.RegionSize == 0x2000, "unexpected RegionSize %I64x, expected 2000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + ok( mbi.RegionSize == 0x1000, "unexpected RegionSize %I64x, expected 1000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + ok( mbi.Protect == (PAGE_READWRITE|PAGE_GUARD), "unexpected Protect %#x, expected %#x\n", mbi.Protect, (PAGE_READWRITE|PAGE_GUARD) ); + + addr = (char *)NtCurrentTeb()->Tib.StackLimit; + status = NtQueryVirtualMemory( NtCurrentProcess(), addr, MemoryBasicInformation, &mbi, sizeof(mbi), &size ); + ok( !status, "NtQueryVirtualMemory returned %08x\n", status ); + todo_wine ok( mbi.RegionSize == 0x6000, "unexpected RegionSize %I64x, expected 6000\n", (UINT64)mbi.RegionSize ); + ok( mbi.State == MEM_COMMIT, "unexpected State %#x, expected %#x\n", mbi.State, MEM_COMMIT ); + todo_wine ok( mbi.Protect == PAGE_READWRITE, "unexpected Protect %#x, expected %#x\n", mbi.Protect, PAGE_READWRITE ); + + + /* clearing the guard pages doesn't change StackLimit back */ + + force_stack_grow(); + + addr = (char *)NtCurrentTeb()->Tib.StackLimit + 0x2000; + size = 0x1000; + status = NtProtectVirtualMemory( NtCurrentProcess(), &addr, &size, PAGE_READWRITE | PAGE_GUARD, &prot ); + ok( !status, "NtProtectVirtualMemory returned %08x\n", status ); + todo_wine ok( prot == PAGE_READWRITE, "unexpected prot %#x, expected %#x\n", prot, PAGE_READWRITE ); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x6000, "unexpected stack committed size %x, expected 6000\n", committed ); + + status = NtProtectVirtualMemory( NtCurrentProcess(), &addr, &size, PAGE_READWRITE, &prot ); + ok( !status, "NtProtectVirtualMemory returned %08x\n", status ); + ok( prot == (PAGE_READWRITE | PAGE_GUARD), "unexpected prot %#x, expected %#x\n", prot, (PAGE_READWRITE | PAGE_GUARD) ); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x6000, "unexpected stack committed size %x, expected 6000\n", committed ); + + /* and as we messed with it and it now doesn't fault, it doesn't grow back either */ + + force_stack_grow(); + + committed = (char *)NtCurrentTeb()->Tib.StackBase - (char *)NtCurrentTeb()->Tib.StackLimit; + todo_wine ok( committed == 0x6000, "unexpected stack committed size %x, expected 6000\n", committed ); +#endif + + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); + struct test_stack_size_thread_args args; SIZE_T default_commit = nt->OptionalHeader.SizeOfStackCommit; SIZE_T default_reserve = nt->OptionalHeader.SizeOfStackReserve; INITIAL_TEB stack = {0}; unsigned int i; NTSTATUS ret; + HANDLE thread;
struct { @@ -298,6 +550,18 @@ static void test_RtlCreateUserStack(void)
ret = pRtlCreateUserStack(0x11000, 0x110000, 0, 0, 1, &stack); ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret); + + args.expect_committed = 0x4000; + args.expect_reserved = default_reserve; + thread = CreateThread(NULL, 0x3f00, test_stack_size_thread, &args, 0, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + args.expect_committed = default_commit < 0x2000 ? 0x2000 : default_commit; + args.expect_reserved = 0x400000; + thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); }
static void test_NtMapViewOfSection(void)
Instead of the whole stack, except for the last one or two pages.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/signal_i386.c | 5 ++--- dlls/ntdll/signal_x86_64.c | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c index 635b8f4de81..cb04206343a 100644 --- a/dlls/ntdll/signal_i386.c +++ b/dlls/ntdll/signal_i386.c @@ -507,9 +507,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "leal -12(%esi),%ecx\n\t" /* clear the thread stack */ "andl $~0xfff,%ecx\n\t" /* round down to page size */ - "movl %fs:8,%edi\n\t" /* NtCurrentTeb()->Tib.StackLimit */ - "addl $0x1000,%edi\n\t" - "movl %edi,%esp\n\t" + "movl %ecx,%edi\n\t" + "subl $0x2000,%edi\n\t" "subl %edi,%ecx\n\t" "xorl %eax,%eax\n\t" "shrl $2,%ecx\n\t" diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index b5be4f35758..418b3dbba25 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -1540,10 +1540,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movq %rcx,%rbx\n\t" /* context */ /* clear the thread stack */ "andq $~0xfff,%rcx\n\t" /* round down to page size */ - "movq %gs:0x30,%rax\n\t" - "movq 0x10(%rax),%rdi\n\t" /* NtCurrentTeb()->Tib.StackLimit */ - "addq $0x2000,%rdi\n\t" - "movq %rdi,%rsp\n\t" + "movq %rcx,%rdi\n\t" + "subq $0x2000,%rdi\n\t" "subq %rdi,%rcx\n\t" "xorl %eax,%eax\n\t" "shrq $3,%rcx\n\t"
On 3/30/21 7:24 PM, Rémi Bernon wrote:
Instead of the whole stack, except for the last one or two pages.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/signal_i386.c | 5 ++--- dlls/ntdll/signal_x86_64.c | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c index 635b8f4de81..cb04206343a 100644 --- a/dlls/ntdll/signal_i386.c +++ b/dlls/ntdll/signal_i386.c @@ -507,9 +507,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "leal -12(%esi),%ecx\n\t" /* clear the thread stack */ "andl $~0xfff,%ecx\n\t" /* round down to page size */
"movl %fs:8,%edi\n\t" /* NtCurrentTeb()->Tib.StackLimit */"addl $0x1000,%edi\n\t""movl %edi,%esp\n\t"
"movl %ecx,%edi\n\t""subl $0x2000,%edi\n\t" "subl %edi,%ecx\n\t" "xorl %eax,%eax\n\t" "shrl $2,%ecx\n\t"diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index b5be4f35758..418b3dbba25 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -1540,10 +1540,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movq %rcx,%rbx\n\t" /* context */ /* clear the thread stack */ "andq $~0xfff,%rcx\n\t" /* round down to page size */
"movq %gs:0x30,%rax\n\t""movq 0x10(%rax),%rdi\n\t" /* NtCurrentTeb()->Tib.StackLimit */"addq $0x2000,%rdi\n\t""movq %rdi,%rsp\n\t"
"movq %rcx,%rdi\n\t""subq $0x2000,%rdi\n\t" "subq %rdi,%rcx\n\t" "xorl %eax,%eax\n\t" "shrq $3,%rcx\n\t"
Also out of curiosity, what was the reason not to clear the last (last two on x86_64) stack page(s) here?
I also removed the rsp stores, assuming it wasn't used for rep movs anyway but I'm now thinking that maybe all this was here for the case we get signaled while clearing the pages?
Rémi Bernon rbernon@codeweavers.com writes:
@@ -1540,10 +1540,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movq %rcx,%rbx\n\t" /* context */ /* clear the thread stack */ "andq $~0xfff,%rcx\n\t" /* round down to page size */
"movq %gs:0x30,%rax\n\t""movq 0x10(%rax),%rdi\n\t" /* NtCurrentTeb()->Tib.StackLimit */"addq $0x2000,%rdi\n\t""movq %rdi,%rsp\n\t"
"movq %rcx,%rdi\n\t""subq $0x2000,%rdi\n\t" "subq %rdi,%rcx\n\t" "xorl %eax,%eax\n\t" "shrq $3,%rcx\n\t"Also out of curiosity, what was the reason not to clear the last (last two on x86_64) stack page(s) here?
There's already data there, like the initial context.
I also removed the rsp stores, assuming it wasn't used for rep movs anyway but I'm now thinking that maybe all this was here for the case we get signaled while clearing the pages?
Yes, in general touching the stack below the stack pointer is not a good idea.
On 3/30/21 8:33 PM, Alexandre Julliard wrote:
Rémi Bernon rbernon@codeweavers.com writes:
@@ -1540,10 +1540,8 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movq %rcx,%rbx\n\t" /* context */ /* clear the thread stack */ "andq $~0xfff,%rcx\n\t" /* round down to page size */
"movq %gs:0x30,%rax\n\t""movq 0x10(%rax),%rdi\n\t" /* NtCurrentTeb()->Tib.StackLimit */"addq $0x2000,%rdi\n\t""movq %rdi,%rsp\n\t"
"movq %rcx,%rdi\n\t""subq $0x2000,%rdi\n\t" "subq %rdi,%rcx\n\t" "xorl %eax,%eax\n\t" "shrq $3,%rcx\n\t"Also out of curiosity, what was the reason not to clear the last (last two on x86_64) stack page(s) here?
There's already data there, like the initial context.
I meant on the other side? But I guess the next part answers it, as I understand we have to keep a bit of space for anything that would need a stack while we clear the pages.
I also removed the rsp stores, assuming it wasn't used for rep movs anyway but I'm now thinking that maybe all this was here for the case we get signaled while clearing the pages?
Yes, in general touching the stack below the stack pointer is not a good idea.
-
Alexandre Julliard -
Rémi Bernon