[newlib-cygwin] Simplify stack allocation code in child after fork

[Corinna Vinschen <corinna at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=29a12632278368f197f3270532e2f6c19642512e

commit 29a12632278368f197f3270532e2f6c19642512e
Author: Corinna Vinschen <corinna@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 <corinna@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  <corinna@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  <corinna@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