From e753e4129ad0843859e97a4c56962b5395f390b6 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Mon, 7 Dec 2015 16:10:55 +0100 Subject: [PATCH] Always allocate main thread stack from pthread stack area on x86_64. * dcrt0.cc: Semi-revert commit 12743c2d5d2721f3a80b4d7671a349be03c1f520. (dll_crt0_0): Drop setting wow64_needs_stack_adjustment on 64 bit. (_dll_crt0): Split out 64 bit code again and always create new main thread stack, unless forked off from the non main thread in the parent. Call create_new_main_thread_stack with parent stack commitsize if started from the parent's main thread. Only call child_info_fork::alloc_stack for the latter case on 64 bit. Slightly rearrange moving rsp and rbp to new stack and document how. Revert 32 bit wow64 handling to its former self. * miscfunc.cc (create_new_main_thread_stack): Take a commitsize parameter and use it if it's not 0. Don't set _main_tls here, it's done in the caller _dll_crt0 anyway. Return stackbase - 16 bytes, rather than stacklimit (which was very wrong anyway). * miscfuncs.h (create_new_main_thread_stack): Accommodate declaration to aforementioned change. * wincap.h (wincaps::has_3264_stack_broken): Remove element. * wincap.cc: Ditto, throughout. * wow64.cc: Semi-revert to pre-12743c2d5d2721f3a80b4d7671a349be03c1f520 but keep architecture-agnostic type changes intact. Fix formatting. * wow64.h: Revert to pre-12743c2d5d2721f3a80b4d7671a349be03c1f520. Signed-off-by: Corinna Vinschen --- winsup/cygwin/ChangeLog | 23 +++++++++++ winsup/cygwin/dcrt0.cc | 84 ++++++++++++++++++++++---------------- winsup/cygwin/miscfuncs.cc | 12 +++--- winsup/cygwin/miscfuncs.h | 3 +- winsup/cygwin/wincap.cc | 8 ---- winsup/cygwin/wincap.h | 2 - winsup/cygwin/wow64.cc | 33 +++++---------- winsup/cygwin/wow64.h | 10 +++-- 8 files changed, 96 insertions(+), 79 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 2e10a382b..7213075f0 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,26 @@ +2015-12-07 Corinna Vinschen + + * dcrt0.cc: Semi-revert commit 12743c2d5d2721f3a80b4d7671a349be03c1f520. + (dll_crt0_0): Drop setting wow64_needs_stack_adjustment on 64 bit. + (_dll_crt0): Split out 64 bit code again and always create new main + thread stack, unless forked off from the non main thread in the parent. + Call create_new_main_thread_stack with parent stack commitsize if + started from the parent's main thread. + Only call child_info_fork::alloc_stack for the latter case on 64 bit. + Slightly rearrange moving rsp and rbp to new stack and document how. + Revert 32 bit wow64 handling to its former self. + * miscfunc.cc (create_new_main_thread_stack): Take a commitsize + parameter and use it if it's not 0. Don't set _main_tls here, it's + done in the caller _dll_crt0 anyway. Return stackbase - 16 bytes, + rather than stacklimit (which was very wrong anyway). + * miscfuncs.h (create_new_main_thread_stack): Accommodate declaration + to aforementioned change. + * wincap.h (wincaps::has_3264_stack_broken): Remove element. + * wincap.cc: Ditto, throughout. + * wow64.cc: Semi-revert to pre-12743c2d5d2721f3a80b4d7671a349be03c1f520 + but keep architecture-agnostic type changes intact. Fix formatting. + * wow64.h: Revert to pre-12743c2d5d2721f3a80b4d7671a349be03c1f520. + 2015-12-06 Corinna Vinschen * include/sys/cygwin.h (CCP_PROC_CYGDRIVE): New flag. diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc index 26b7ec3b4..27450940c 100644 --- a/winsup/cygwin/dcrt0.cc +++ b/winsup/cygwin/dcrt0.cc @@ -782,11 +782,6 @@ dll_crt0_0 () description in wow64_test_for_64bit_parent. */ if (wincap.wow64_has_secondary_stack ()) wow64_needs_stack_adjustment = wow64_test_for_64bit_parent (); -#else - /* Windows 10 1511 has a stack move when a 64 bit process is started from - a 32 bit process, just as it was vice versa in XP/2003. */ - if (wincap.has_3264_stack_broken ()) - wow64_needs_stack_adjustment = !wow64_test_for_64bit_parent (); #endif /* !__x86_64__ */ } else @@ -1068,59 +1063,76 @@ extern "C" void __stdcall _dll_crt0 () { #ifdef __x86_64__ - /* Handle 64 bit process on Windows 10 rel 1511 which has been started from - 32 bit WOW64 process. See comment in wow64_test_for_64bit_parent for a - problem description. Unfortunately the areas the stacks would have to - be moved to are both taken by "something else"(tm) in both, forker and - forkee, so we can't use the wow64_revert_to_original_stack method as in - the 32 bit case. Rather, we move the main thread stack to the stack area - reserved for pthread stacks for this process. */ -#define CREATE_STACK(a) create_new_main_thread_stack(a) -#define FIX_STACK(s) __asm__ ("\n" \ - "movq %[ADDR], %%rsp \n" \ - "movq %%rsp, %%rbp \n" \ - : : [ADDR] "r" (s)) + /* Starting with Windows 10 rel 1511, the main stack of an application is + not reproducible if a 64 bit process has been started from a 32 bit + process. Given that we have enough virtual address space on 64 bit + anyway, we now always move the main thread stack to the stack area + reserved for pthread stacks. This allows a reproducible stack space + under our own control and avoids collision with the OS. */ + if (!dynamically_loaded) + { + if (!in_forkee || fork_info->from_main) + { + /* Must be static since it's referenced after the stack and frame + pointer registers have been changed. */ + static PVOID allocationbase; + SIZE_T commitsize = in_forkee ? (PBYTE) fork_info->stackbase + - (PBYTE) fork_info->stacklimit + : 0; + PVOID stackaddr = create_new_main_thread_stack (allocationbase, + commitsize); + if (stackaddr) + { + /* Set stack pointer to new address. Set frame pointer to + stack pointer and subtract 32 bytes for shadow space. */ + __asm__ ("\n\ + movq %[ADDR], %%rsp \n\ + movq %%rsp, %%rbp \n\ + subq $32,%%rsp \n" + : : [ADDR] "r" (stackaddr)); + /* We're on the new stack now. Free up space taken by the former + main thread stack and set DeallocationStack correctly. */ + VirtualFree (NtCurrentTeb ()->DeallocationStack, 0, MEM_RELEASE); + NtCurrentTeb ()->DeallocationStack = allocationbase; + } + } + else + fork_info->alloc_stack (); + } #else /* Handle WOW64 process on XP/2K3 which has been started from native 64 bit process. See comment in wow64_test_for_64bit_parent for a full problem description. */ -#define CREATE_STACK(a) wow64_revert_to_original_stack(a) -#define FIX_STACK(s) __asm__ ("\n" \ - "movl %[ADDR], %%esp \n" \ - "xorl %%ebp, %%ebp \n" \ - : : [ADDR] "r" (s)) -#endif if (wow64_needs_stack_adjustment && !dynamically_loaded) { /* Must be static since it's referenced after the stack and frame pointer registers have been changed. */ static PVOID allocationbase; - PVOID stackaddr = CREATE_STACK (allocationbase); + PVOID stackaddr = wow64_revert_to_original_stack (allocationbase); if (stackaddr) { - /* 2nd half of the stack move. Set stack pointer to new address. - Set frame pointer to 0. */ - FIX_STACK (stackaddr); - /* Now we're back on the original stack. Free up space taken by the + /* Set stack pointer to new address. Set frame pointer to 0. */ + __asm__ ("\n\ + movl %[ADDR], %%esp \n\ + xorl %%ebp, %%ebp \n" + : : [ADDR] "r" (stackaddr)); + /* We're back on the original stack now. Free up space taken by the former main thread stack and set DeallocationStack correctly. */ VirtualFree (NtCurrentTeb ()->DeallocationStack, 0, MEM_RELEASE); NtCurrentTeb ()->DeallocationStack = allocationbase; } else /* Fall back to respawning if creating a new stack fails. */ - wow64_respawn_process (); + wow64_respawn_process(); } - _feinitialise (); -#ifndef __x86_64__ main_environ = user_data->envptr; -#endif if (in_forkee) - { - fork_info->alloc_stack (); - _main_tls = &_my_tls; - } + fork_info->alloc_stack (); +#endif + _feinitialise (); + _main_tls = &_my_tls; _main_tls->call ((DWORD (*) (void *, void *)) dll_crt0_1, NULL); } diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc index 320a3c2f3..796494132 100644 --- a/winsup/cygwin/miscfuncs.cc +++ b/winsup/cygwin/miscfuncs.cc @@ -765,13 +765,13 @@ thread_allocator thr_alloc NO_COPY; maintained by the thr_alloc class. See the description in the x86_64-only code in _dll_crt0 to understand why we have to do this. */ PVOID -create_new_main_thread_stack (PVOID &allocationbase) +create_new_main_thread_stack (PVOID &allocationbase, SIZE_T parent_commitsize) { PIMAGE_DOS_HEADER dosheader; PIMAGE_NT_HEADERS ntheader; SIZE_T stacksize; ULONG guardsize; - ULONG commitsize; + SIZE_T commitsize; PBYTE stacklimit; dosheader = (PIMAGE_DOS_HEADER) GetModuleHandle (NULL); @@ -783,7 +783,10 @@ create_new_main_thread_stack (PVOID &allocationbase) allocationbase = thr_alloc.alloc (ntheader->OptionalHeader.SizeOfStackReserve); guardsize = wincap.def_guard_page_size (); - commitsize = ntheader->OptionalHeader.SizeOfStackCommit; + if (parent_commitsize) + commitsize = (SIZE_T) parent_commitsize; + else + commitsize = ntheader->OptionalHeader.SizeOfStackCommit; commitsize = roundup2 (commitsize, wincap.page_size ()); if (commitsize > stacksize - guardsize - wincap.page_size ()) commitsize = stacksize - guardsize - wincap.page_size (); @@ -798,8 +801,7 @@ create_new_main_thread_stack (PVOID &allocationbase) return NULL; NtCurrentTeb()->Tib.StackBase = ((PBYTE) allocationbase + stacksize); NtCurrentTeb()->Tib.StackLimit = stacklimit; - _main_tls = &_my_tls; - return stacklimit - 64; + return ((PBYTE) allocationbase + stacksize - 16); } #endif diff --git a/winsup/cygwin/miscfuncs.h b/winsup/cygwin/miscfuncs.h index 8ff85d970..82b413f3a 100644 --- a/winsup/cygwin/miscfuncs.h +++ b/winsup/cygwin/miscfuncs.h @@ -71,7 +71,8 @@ ssize_t __reg3 check_iovec (const struct iovec *, int, bool); #define check_iovec_for_write(a, b) check_iovec ((a), (b), true) #ifdef __x86_64__ -extern PVOID create_new_main_thread_stack (PVOID &allocationbase); +extern PVOID create_new_main_thread_stack (PVOID &allocationbase, + SIZE_T parent_commitsize); #endif extern "C" DWORD WINAPI pthread_wrapper (PVOID arg); diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc index 6da4b0b5a..201bd2594 100644 --- a/winsup/cygwin/wincap.cc +++ b/winsup/cygwin/wincap.cc @@ -52,7 +52,6 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:false, has_broken_prefetchvm:false, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -87,7 +86,6 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:false, has_broken_prefetchvm:false, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -122,7 +120,6 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:false, has_broken_prefetchvm:false, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -157,7 +154,6 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:true, has_broken_prefetchvm:false, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_8 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -192,7 +188,6 @@ wincaps wincap_8 __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:true, has_broken_prefetchvm:false, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_10 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -227,7 +222,6 @@ wincaps wincap_10 __attribute__((section (".cygwin_dll_common"), shared)) = { has_processor_groups:true, has_broken_prefetchvm:true, has_new_pebteb_region:false, - has_3264_stack_broken:false, }; wincaps wincap_10_1511 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -262,7 +256,6 @@ wincaps wincap_10_1511 __attribute__((section (".cygwin_dll_common"), shared)) = has_processor_groups:true, has_broken_prefetchvm:false, has_new_pebteb_region:true, - has_3264_stack_broken:true, }; wincapc wincap __attribute__((section (".cygwin_dll_common"), shared)); @@ -330,7 +323,6 @@ wincapc::init () /* Windows 10 1511 has a stack move when a 64 bit process is started from a 32 bit process, just as it was vice versa in XP/2003. Reset the flag here for 32 bit. */ - ((wincaps *)caps)->has_3264_stack_broken = false; if (NT_SUCCESS (NtQueryInformationProcess (NtCurrentProcess (), ProcessWow64Information, &wow64, sizeof wow64, NULL)) diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h index 05a73f925..4508974f5 100644 --- a/winsup/cygwin/wincap.h +++ b/winsup/cygwin/wincap.h @@ -45,7 +45,6 @@ struct wincaps unsigned has_processor_groups : 1; unsigned has_broken_prefetchvm : 1; unsigned has_new_pebteb_region : 1; - unsigned has_3264_stack_broken : 1; }; class wincapc @@ -105,7 +104,6 @@ public: bool IMPLEMENT (has_processor_groups) bool IMPLEMENT (has_broken_prefetchvm) bool IMPLEMENT (has_new_pebteb_region) - bool IMPLEMENT (has_3264_stack_broken) #undef IMPLEMENT }; diff --git a/winsup/cygwin/wow64.cc b/winsup/cygwin/wow64.cc index f91a6d855..fcfa58ec2 100644 --- a/winsup/cygwin/wow64.cc +++ b/winsup/cygwin/wow64.cc @@ -8,6 +8,10 @@ This software is a copyrighted work licensed under the terms of the Cygwin license. Please consult the file "CYGWIN_LICENSE" for details. */ +#ifndef __x86_64__ +/* WOW64 only plays a role in the 32 bit version. Don't use any of this + in the 64 bit version. */ + #include "winsup.h" #include "cygtls.h" #include "ntdll.h" @@ -37,15 +41,8 @@ wow64_eval_expected_main_stack (PVOID &allocbase, PVOID &stackbase) stack address on Vista/2008 64 bit is 0x80000 and on W7/2K8R2 64 bit it is 0x90000. However, this is no problem because the system sticks to that address for all WOW64 processes, not only for the first one - started from a 64 bit parent. - - On 64 bit W10 1511 the stack starts at 0x400000 by default. See comment - in wow64_test_for_64bit_parent. */ -#ifdef __x86_64__ - allocbase = (PVOID) 0x400000; -#else + started from a 64 bit parent. */ allocbase = (PVOID) 0x30000; -#endif /* Stack size. The OS always rounds the size up to allocation granularity and it never allocates less than 256K. */ size = roundup2 (ntheader->OptionalHeader.SizeOfStackReserve, @@ -79,14 +76,6 @@ wow64_test_for_64bit_parent () process here. If so, we note this fact in wow64_needs_stack_adjustment so we can workaround the stack problem in _dll_crt0. See there for how we go along. */ - - /* Amazing but true: Starting with Windows 10 1511 this problem has been - reintroduced, just in the opposite direction: If a 64 bit process is - created from a 32 bit WOW64 process, the main thread stack in the 64 - bit child gets moved to another location than the default. In the - forked child, the stack is back where it usually is when started from - another 64 bit process. Therefore we have to be able to recognize - this scenarion now on 64 bit as well. We I don't believe it... */ NTSTATUS ret; PROCESS_BASIC_INFORMATION pbi; HANDLE parent; @@ -118,8 +107,6 @@ wow64_test_for_64bit_parent () return !wow64; } -#ifndef __x86_64__ - PVOID wow64_revert_to_original_stack (PVOID &allocationbase) { @@ -184,14 +171,12 @@ wow64_revert_to_original_stack (PVOID &allocationbase) accordingly, and return the new, 16 byte aligned address for the stack pointer. The second half of the stack move is done by the caller _dll_crt0. */ - NtCurrentTeb()->Tib.StackBase = (char *) newbase; - NtCurrentTeb()->Tib.StackLimit = (char *) newtop; + NtCurrentTeb ()->Tib.StackBase = (char *) newbase; + NtCurrentTeb ()->Tib.StackLimit = (char *) newtop; _main_tls = &_my_tls; - return PTR_ADD (NtCurrentTeb()->Tib.StackBase, -16); + return PTR_ADD (NtCurrentTeb ()->Tib.StackBase, -16); } -#endif /* !__x86_64__ */ - /* Respawn WOW64 process. This is only called if we can't reuse the original stack. See comment in wow64_revert_to_original_stack for details. See _dll_crt0 for the call of this function. @@ -226,3 +211,5 @@ wow64_respawn_process () TerminateProcess (GetCurrentProcess (), ret); ExitProcess (ret); } + +#endif /* !__x86_64__ */ diff --git a/winsup/cygwin/wow64.h b/winsup/cygwin/wow64.h index 150561c8c..2b46c9872 100644 --- a/winsup/cygwin/wow64.h +++ b/winsup/cygwin/wow64.h @@ -8,12 +8,14 @@ This software is a copyrighted work licensed under the terms of the Cygwin license. Please consult the file "CYGWIN_LICENSE" for details. */ -extern bool NO_COPY wow64_needs_stack_adjustment; -extern bool wow64_test_for_64bit_parent (); -extern void wow64_respawn_process () __attribute__ ((noreturn)); - #ifndef __x86_64__ +/* WOW64 only plays a role in the 32 bit version. Don't use any of this + in the 64 bit version. */ +extern bool NO_COPY wow64_needs_stack_adjustment; + +extern bool wow64_test_for_64bit_parent (); extern PVOID wow64_revert_to_original_stack (PVOID &allocationbase); +extern void wow64_respawn_process () __attribute__ ((noreturn)); #endif /* !__x86_64__ */ -- 2.43.5