Subject: Re: pthread stack problem
To: None <tech-userlevel@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-userlevel
Date: 08/15/2004 08:00:58
--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Aug 05, 2004 at 08:05:36AM -0700, Chuck Silvers wrote:
> I've thought of a couple possible ways to fix this:
> 
>  (1) Change the caller of main() to record its current stack pointer in a
>      global variable and let pthread__initmain() use that to initialize
>      the stack bounds for the initial thread.

it turns out that this option isn't so easy.  pthread_init() (as a shared-
library "constructor") is actually called by ld.elf_so before any code in
the base executable has a chance to run.  so to make this kind of approach
work, ld.elf_so would actually have to poke the value into a global variable
similar to what it does for "__progname", etc.  yuck.


>  (2) Change the kernel to make the usable part of the stack (ie. not including
>      the argv and environment) aligned such that the (base + size) address
>      that pthread__initmain() computes is valid and not in the middle of
>      the argv or environment.

and this approach wouldn't work with PT_FIXEDSTACKSIZE_LG, since then
the kernel wouldn't know what stack size libpthread is going to use.



upon further consideration, I've concluded that my previous fix for PR 26392
was just wrong and that the problem is really with this bit of code:

        /* Set up an initial ucontext pointer to a "safe" area */
        t->pt_uc = (ucontext_t *)
                STACK_ALIGN(STACK_GROW(sp, pagesize / 2), ~_UC_UCONTEXT_ALIGN);

since libpthread can't know how far from the end of the initial stack
it has to go before it's "safe", it can really only consider the other end
"safe".  so pthread__initmain() should override the choice of "safe" location
for the initial stack.  (we'd like to continue using the existing "safe" value
for other stacks, since it's ok for them and we'd like to avoid touching
pages that usually won't ever be used again.)

the attached patches (for -current and the 2.0 branch) undo the previous
change and apply the new one.  if no one objects, I'll commit this
(and request a pullup for the 2.0-branch version) tomorrow.

-Chuck

--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.pthread.initmain3.current"

Index: src/lib/libpthread/pthread_stack.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread_stack.c,v
retrieving revision 1.15
diff -u -p -r1.15 pthread_stack.c
--- src/lib/libpthread/pthread_stack.c	25 Jul 2004 23:22:43 -0000	1.15
+++ src/lib/libpthread/pthread_stack.c	15 Aug 2004 14:41:28 -0000
@@ -106,6 +106,7 @@ pthread__stackalloc(pthread_t *newt)
 void
 pthread__initmain(pthread_t *newt)
 {
+	pthread_t t;
 	void *base;
 	size_t size;
 
@@ -142,24 +143,28 @@ pthread__initmain(pthread_t *newt)
 	pthread_stackmask = pthread_stacksize - 1;
 #endif /* PT_FIXEDSTACKSIZE_LG */
 
+	base = (void *)(pthread__sp() & ~PT_STACKMASK);
+	size = PT_STACKSIZE;
+
+	t = pthread__stackid_setup(base, size);
+
 	/*
-	 * The "initial" thread stack can be smaller than requested
+	 * The "safe" area chosen below isn't safe for the initial thread stack
 	 * because we don't control the end of the stack.
 	 * For example, on i386 the stack usually ends at 0xbfc00000,
-	 * so for requested sizes >=8MB, we get a 4MB smaller stack.
+	 * so for requested sizes >=8MB, the last 4MB of stack isn't available.
 	 * Also, we don't want to clobber the argv, environment, etc.
-	 * Just trim off the part of the stack that we can't use.
+	 * Reset the initial pt_uc pointer to be safe for the initial thread.
 	 */
+
 #ifdef __MACHINE_STACK_GROWS_UP
-	/* XXX this case isn't right */
-	base = (void *) (pthread__sp() & ~PT_STACKMASK);
-	size = PT_STACKSIZE;
+	t->pt_uc = (ucontext_t *)
+		((char *)t->pt_stack.ss_sp + t->pt_stack.ss_size -
+		 sizeof (ucontext_t));
 #else
-	base = (void *) (pthread__sp() & ~PT_STACKMASK);
-	size = ((char *)pthread__sp() - (char *)base) & ~(pagesize - 1);
+	t->pt_uc = (ucontext_t *)t->pt_stack.ss_sp;
 #endif
-
-	*newt = pthread__stackid_setup(base, size);
+	*newt = t;
 }
 
 static pthread_t

--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.pthread.initmain3.20"

Index: src/lib/libpthread/pthread_stack.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread_stack.c,v
retrieving revision 1.12.2.1
diff -u -p -r1.12.2.1 pthread_stack.c
--- src/lib/libpthread/pthread_stack.c	26 Jul 2004 07:09:38 -0000	1.12.2.1
+++ src/lib/libpthread/pthread_stack.c	15 Aug 2004 14:41:24 -0000
@@ -104,6 +104,7 @@ pthread__stackalloc(pthread_t *newt)
 void
 pthread__initmain(pthread_t *newt)
 {
+	pthread_t t;
 	void *base;
 	size_t size;
 
@@ -140,18 +141,22 @@ pthread__initmain(pthread_t *newt)
 	pthread_stackmask = pthread_stacksize - 1;
 #endif /* PT_FIXEDSTACKSIZE_LG */
 
+	base = (void *)(pthread__sp() & ~PT_STACKMASK);
+	size = PT_STACKSIZE;
+
+	t = pthread__stackid_setup(base, size);
+
 	/*
-	 * The "initial" thread stack can be smaller than requested
+	 * The "safe" area chosen below isn't safe for the initial thread stack
 	 * because we don't control the end of the stack.
 	 * For example, on i386 the stack usually ends at 0xbfc00000,
-	 * so for requested sizes >=8MB, we get a 4MB smaller stack.
+	 * so for requested sizes >=8MB, the last 4MB of stack isn't available.
 	 * Also, we don't want to clobber the argv, environment, etc.
-	 * Just trim off the part of the stack that we can't use.
+	 * Reset the initial pt_uc pointer to be safe for the initial thread.
 	 */
-	base = (void *) (pthread__sp() & ~PT_STACKMASK);
-	size = ((char *)pthread__sp() - (char *)base) & ~(pagesize - 1);
 
-	*newt = pthread__stackid_setup(base, size);
+	t->pt_uc = (ucontext_t *)t->pt_stack.ss_sp;
+	*newt = t;
 }
 
 static pthread_t

--2fHTh5uZTiUOsy+g--