This is the mail archive of the cygwin-cvs@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[newlib-cygwin] Always allocate main thread stack from pthread stack area on x86_64.


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

commit e753e4129ad0843859e97a4c56962b5395f390b6
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Mon Dec 7 16:10:55 2015 +0100

    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 <corinna@vinschen.de>

Diff:
---
 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 2e10a38..7213075 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,26 @@
+2015-12-07  Corinna Vinschen  <corinna@vinschen.de>
+
+	* 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  <corinna@vinschen.de>
 
 	* include/sys/cygwin.h (CCP_PROC_CYGDRIVE): New flag.
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 26b7ec3..2745094 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 320a3c2..7964941 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 8ff85d9..82b413f 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 6da4b0b..201bd25 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 05a73f9..4508974 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 f91a6d8..fcfa58e 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 150561c..2b46c98 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__ */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]