Re: A patch for linuxthreads

["H . J . Lu" <hjl@valinux.com>]

On Thu, Dec 16, 1999 at 04:33:18PM -0800, Geoff Keating wrote:
> > Date: Thu, 16 Dec 1999 16:12:43 -0800
> > From: "H . J . Lu" <hjl@valinux.com>
> > 
> > Hi,
> > 
> > While working on the ia64 port for linuxthreads, I found
> > this patch was necessary. Any comments?
> 
> Yes.
> 
> > -- 
> > H.J. Lu (hjl@gnu.org)
> > ---
> > Thu Dec 16 15:59:58 1999  H.J. Lu  <hjl@gnu.org>
> > 
> > 	* manager.c (pthread_allocate_stack): Correct the calculation
> > 	of "new_thread_bottom".
> > 
> > Index: manager.c
> > ===================================================================
> > RCS file: /work/cvs/gnu/glibc-2.1/linuxthreads/manager.c,v
> > retrieving revision 1.1.1.22
> > diff -u -p -r1.1.1.22 manager.c
> > --- manager.c	1999/11/27 18:44:03	1.1.1.22
> > +++ manager.c	1999/12/17 00:10:18
> > @@ -291,7 +291,7 @@ static int pthread_allocate_stack(const 
> >      {
> >        /* Allocate space for stack and thread descriptor at default address */
> >        new_thread = default_new_thread;
> > -      new_thread_bottom = (char *) new_thread - STACK_SIZE;
> > +      new_thread_bottom = (char *) (new_thread + 1) - STACK_SIZE;
> >        if (mmap((caddr_t)((char *)(new_thread + 1) - INITIAL_STACK_SIZE),
> >                 INITIAL_STACK_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC,
> >                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED | MAP_GROWSDOWN,
> > 
> 
> 1. When you submit a patch, it is helpful if you explain what bug you
>    are fixing or what new feature you are adding.  This saves us from
>    having to deduce it by inspection of the code.  This is even more
>    important when you are implementing on hardware that is not readily
>    available.

I am not sure how much I am allowed to say. Let me just say we
need to access new_thread_bottom. If I understand the code
correctly, the stack top is (char *)(new_thread + 1) since we do

        if (mmap((caddr_t)((char *)(new_thread + 1) - INITIAL_STACK_SIZE),
                 INITIAL_STACK_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC,
                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED | MAP_GROWSDOWN,

That means we mmap the top INITIAL_STACK_SIZE bytes from the stack
top and grow the stack downwards. If it is true, the stack bottom should
be 

(char *)(new_thread + 1) - STACK_SIZE

We didn't see any bug reports since not many archs need to access
the stack bottom. In fact, if you compute it,

(char *) new_thread - STACK_SIZE

usually is not page aligned.

> 
> 2. If you need to change this, don't you need to change the similar
>    calculation in the previous {} group a few lines above?

It is because the user should provide the correct stack top and
stack size.


-- 
H.J. Lu (hjl@gnu.org)