https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=29a12632278368f197f3270532e2f6c19642512e
commit 29a12632278368f197f3270532e2f6c19642512e Author: Corinna Vinschen <cori...@vinschen.de> Date: Tue Jul 7 17:24:37 2015 +0200 Simplify stack allocation code in child after fork * child_info.h (CURR_CHILD_INFO_MAGIC): Update. (child_info_fork::alloc_stack_hard_way): Drop declaration. * dcrt0.cc (child_info_fork::alloc_stack_hard_way): Fold into child_info_fork::alloc_stack. (getstack): Remove. (child_info_fork::alloc_stack): Simplify check for application-provided stack in "hard way" code. Don't call getstack for each page, just reallocate stack immediately as required. Signed-off-by: Corinna Vinschen <cori...@vinschen.de> Diff: --- winsup/cygwin/ChangeLog | 11 ++++ winsup/cygwin/child_info.h | 3 +- winsup/cygwin/dcrt0.cc | 123 +++++++++++++++++++++------------------------ 3 files changed, 68 insertions(+), 69 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 141307e..a5e82d6 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,5 +1,16 @@ 2015-07-07 Corinna Vinschen <cori...@vinschen.de> + * child_info.h (CURR_CHILD_INFO_MAGIC): Update. + (child_info_fork::alloc_stack_hard_way): Drop declaration. + * dcrt0.cc (child_info_fork::alloc_stack_hard_way): Fold into + child_info_fork::alloc_stack. + (getstack): Remove. + (child_info_fork::alloc_stack): Simplify check for application-provided + stack in "hard way" code. Don't call getstack for each page, just + reallocate stack immediately as required. + +2015-07-07 Corinna Vinschen <cori...@vinschen.de> + * fork.cc (frok::parent): Simplify code propagating stack setup to child process. Tweak comments. diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index d946727..9a0a2aa 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -39,7 +39,7 @@ enum child_status #define EXEC_MAGIC_SIZE sizeof(child_info) /* Change this value if you get a message indicating that it is out-of-sync. */ -#define CURR_CHILD_INFO_MAGIC 0x93737edaU +#define CURR_CHILD_INFO_MAGIC 0x4a91a908U #define NPROCS 256 @@ -113,7 +113,6 @@ public: void __reg1 handle_fork (); bool abort (const char *fmt = NULL, ...); void alloc_stack (); - void alloc_stack_hard_way (volatile char *); }; class fhandler_base; diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc index 48301ea..f80ee36 100644 --- a/winsup/cygwin/dcrt0.cc +++ b/winsup/cygwin/dcrt0.cc @@ -405,39 +405,49 @@ check_sanity_and_sync (per_process *p) child_info NO_COPY *child_proc_info; +/* Extend the stack prior to fork longjmp. */ void -child_info_fork::alloc_stack_hard_way (volatile char *b) +child_info_fork::alloc_stack () { - void *stack_ptr; - SIZE_T stacksize; - - /* First check if the requested stack area is part of the user heap - or part of a mmapped region. If so, we have been started from a - pthread with an application-provided stack, and the stack has just - to be used as is. */ - if ((stacktop >= cygheap->user_heap.base - && stackbottom <= cygheap->user_heap.max) - || is_mmapped_region ((caddr_t) stacktop, (caddr_t) stackbottom)) - return; - /* First, try to reserve the entire stack. */ - stacksize = (PBYTE) stackbottom - (PBYTE) stackaddr; - if (!VirtualAlloc (stackaddr, stacksize, MEM_RESERVE, PAGE_NOACCESS)) - { - PTEB teb = NtCurrentTeb (); - api_fatal ("fork: can't reserve memory for parent stack " - "%p - %p, (child has %p - %p), %E", - stackaddr, stackbottom, teb->DeallocationStack, _tlsbase); - } - stacksize = (PBYTE) stackbottom - (PBYTE) stacktop; - stack_ptr = VirtualAlloc (stacktop, stacksize, MEM_COMMIT, PAGE_READWRITE); - if (!stack_ptr) - abort ("can't commit memory for stack %p(%ly), %E", stacktop, stacksize); - if (guardsize != (size_t) -1) + /* Make sure not to try a hard allocation if we have been forked off from + the main thread of a Cygwin process which has been started from a 64 bit + parent. In that case the _tlsbase of the forked child is not the same + as the _tlsbase of the parent (== stackbottom), but only because the + stack of the parent has been slightly rearranged. See comment in + wow64_revert_to_original_stack for details. We check here if the + parent stack fits into the child stack. */ + if (_tlsbase != stackbottom + && (!wincap.is_wow64 () + || stacktop < NtCurrentTeb ()->DeallocationStack + || stackbottom > _tlsbase)) { + void *stack_ptr; + size_t stacksize; + + /* If guardsize is -1, we have been started from a pthread with an + application-provided stack, and the stack has just to be used as is. */ + if (guardsize == (size_t) -1) + return; + /* Reserve entire stack. */ + stacksize = (PBYTE) stackbottom - (PBYTE) stackaddr; + if (!VirtualAlloc (stackaddr, stacksize, MEM_RESERVE, PAGE_NOACCESS)) + { + PTEB teb = NtCurrentTeb (); + api_fatal ("fork: can't reserve memory for parent stack " + "%p - %p, (child has %p - %p), %E", + stackaddr, stackbottom, teb->DeallocationStack, _tlsbase); + } + /* Commit the area commited in parent. */ + stacksize = (PBYTE) stackbottom - (PBYTE) stacktop; + stack_ptr = VirtualAlloc (stacktop, stacksize, MEM_COMMIT, + PAGE_READWRITE); + if (!stack_ptr) + api_fatal ("can't commit memory for stack %p(%ly), %E", + stacktop, stacksize); + /* Set up guardpages. */ ULONG real_guardsize = guardsize ? roundup2 (guardsize, wincap.page_size ()) : wincap.def_guard_page_size (); - /* Allocate PAGE_GUARD page if it still fits. */ if (stack_ptr > stackaddr) { stack_ptr = (void *) ((PBYTE) stack_ptr - real_guardsize); @@ -446,8 +456,9 @@ child_info_fork::alloc_stack_hard_way (volatile char *b) api_fatal ("fork: couldn't allocate new stack guard page %p, %E", stack_ptr); } - /* On post-XP systems, set thread stack guarantee matching the guardsize. - Note that the guardsize is one page bigger than the guarantee. */ + /* On post-XP systems, set thread stack guarantee matching the + guardsize. Note that the guardsize is one page bigger than + the guarantee. */ if (wincap.has_set_thread_stack_guarantee () && real_guardsize > wincap.def_guard_page_size ()) { @@ -455,46 +466,24 @@ child_info_fork::alloc_stack_hard_way (volatile char *b) SetThreadStackGuarantee (&real_guardsize); } } - b[0] = '\0'; -} - -void *getstack (void *) __attribute__ ((noinline)); -volatile char * -getstack (volatile char * volatile p) -{ - *p ^= 1; - *p ^= 1; - return p - 4096; -} - -/* extend the stack prior to fork longjmp */ - -void -child_info_fork::alloc_stack () -{ - volatile char * volatile stackp; -#ifdef __x86_64__ - __asm__ volatile ("movq %%rsp,%0": "=r" (stackp)); -#else - __asm__ volatile ("movl %%esp,%0": "=r" (stackp)); -#endif - /* Make sure not to try a hard allocation if we have been forked off from - the main thread of a Cygwin process which has been started from a 64 bit - parent. In that case the _tlsbase of the forked child is not the same - as the _tlsbase of the parent (== stackbottom), but only because the - stack of the parent has been slightly rearranged. See comment in - wow64_revert_to_original_stack for details. We check here if the - parent stack fits into the child stack. */ - if (_tlsbase != stackbottom - && (!wincap.is_wow64 () - || stacktop < (char *) NtCurrentTeb ()->DeallocationStack - || stackbottom > _tlsbase)) - alloc_stack_hard_way (stackp); else { - char *st = (char *) stacktop; - while (_tlstop > st) - stackp = getstack (stackp); + /* Fork has been called from main thread. Simply commit the region + of the stack commited in the parent but not yet commited in the + child and create new guardpages. */ + if (_tlstop > stacktop) + { + SIZE_T commitsize = (PBYTE) _tlstop - (PBYTE) stacktop; + if (!VirtualAlloc (stacktop, commitsize, MEM_COMMIT, PAGE_READWRITE)) + api_fatal ("can't commit child memory for stack %p(%ly), %E", + stacktop, commitsize); + PVOID guardpage = (PBYTE) stacktop - wincap.def_guard_page_size (); + if (!VirtualAlloc (guardpage, wincap.def_guard_page_size (), + MEM_COMMIT, PAGE_READWRITE | PAGE_GUARD)) + api_fatal ("fork: couldn't allocate new stack guard page %p, %E", + guardpage); + _tlstop = stacktop; + } stackaddr = 0; /* This only affects forked children of a process started from a native 64 bit process, but it doesn't hurt to do it unconditionally. Fix