From mboxrd@z Thu Jan 1 00:00:00 1970 From: "H . J . Lu" To: Geoff Keating Cc: libc-hacker@sourceware.cygnus.com Subject: Re: A patch for linuxthreads Date: Thu, 16 Dec 1999 16:43:00 -0000 Message-id: <19991216164305.A11873@valinux.com> References: <19991216161243.A13548@valinux.com> <199912170033.QAA01416@localhost.cygnus.com> X-SW-Source: 1999-12/msg00066.html 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" > > > > 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 > > > > * 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)