_STACK_GROWS_UP fixes.

libc-alpha,

Fix stack grows up.
In a stack grows down scenario the stack pointer is between:

BOS <--- SP ---> h_descr

In the stack grows up scenarion we have:

h_descr,BOS <--- SP ---> STACK GUARD

TOS is not stored, and thus the stack guard start address is used.

This fixes a regression on HPPA where a cancellation signal would reach the child thread before the thread register had been setup. In that situation __pthread_self_stack was called to determine which thread was active, with the logic backwards, and no limit in place the while loop spun until it encountered garbage that caused the logical expression to evaluate to true. This garbage stack address was fatal to the proper execution of the program.

Cheers,
Carlos

===
===

2003-10-06 Carlos O'Donell <[email protected]>

* pthread.c (__pthread_self_stack): _STACK_GROWS_UP case added. (__pthread_find_self): Likewise.
* manager.c (thread_segment): _STACK_GROWS_UP case added.

diff -u -p -r1.131 pthread.c
@@ -893,7 +893,11 @@ pthread_descr __pthread_find_self(void)
   /* __pthread_handles[0] is the initial thread, __pthread_handles[1] is
      the manager threads handled specially in thread_self(), so start at 2 */
   h = __pthread_handles + 2;
+# ifdef _STACK_GROWS_UP
+  while (! (sp >= (char *) h->h_descr && sp < h->h_descr->p_guardaddr)) h++;
+# else
   while (! (sp <= (char *) h->h_descr && sp >= h->h_bottom)) h++;
+# endif
   return h->h_descr;
 }
 

@@ -908,11 +912,22 @@ pthread_descr __pthread_self_stack(void)
     return manager_thread;
   h = __pthread_handles + 2;
 # ifdef USE_TLS
+#  ifdef _STACK_GROWS_UP
+  while (h->h_descr == NULL
+	 || ! (sp >= h->h_descr->p_stackaddr && 
+	       sp < h->h_descr->p_guardaddr))
+#  else
   while (h->h_descr == NULL
-	 || ! (sp <= (char *) h->h_descr->p_stackaddr && sp >= h->h_bottom))
+	 || ! (sp <= h->h_descr->p_stackaddr && 
+	       sp >= h->h_bottom))
+#  endif
     h++;
 # else
+#  ifdef _STACK_GROWS_UP
+  while (! (sp >= (char *) h->h_descr && sp < h->h_descr->p_guardaddr))
+#  else
   while (! (sp <= (char *) h->h_descr && sp >= h->h_bottom))
+#  endif
     h++;
 # endif
   return h->h_descr;

diff -u -p -r1.95 manager.c
@@ -70,8 +70,13 @@ static pthread_descr manager_thread;
 #else
 static inline pthread_descr thread_segment(int seg)
 {
+# ifdef _STACK_GROWS_UP
+  return (pthread_descr)(THREAD_STACK_START_ADDRESS + (seg - 1) * STACK_SIZE)
+         + 1;
+# else
   return (pthread_descr)(THREAD_STACK_START_ADDRESS - (seg - 1) * STACK_SIZE)
          - 1;
+# endif
 }
 #endif
 

You need to look at linuxthreads/attr.c (__pthread_attr_getstack) too (the recent code which parses /proc/self/maps and finds stack VMA in there).

Jakub

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Applied after cleaning up.

- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/jOD22ijCOnn/RHQRAtSQAKCSAkx1YFOt3HSr2MBfKcB5OqK43ACgyHQK ch5Et0rXhEeQqKCszFrYGDY=
=UKA4
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Applied after cleaning up.

- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE/jOD22ijCOnn/RHQRAtSQAKCSAkx1YFOt3HSr2MBfKcB5OqK43ACgyHQK ch5Et0rXhEeQqKCszFrYGDY=
=UKA4
-----END PGP SIGNATURE-----

You need to look at linuxthreads/attr.c (__pthread_attr_getstack) too (the recent code which parses /proc/self/maps and finds stack VMA in there).

Jakub

That should hopefully explain the regression in tst-attr1, I hadn't tracked it down yet, but I knew that some other STACK_GROWS_UP problems might exist.

Thanks Jakub.

c.

That should hopefully explain the regression in tst-attr1, I hadn't tracked it down yet, but I knew that some other STACK_GROWS_UP problems might exist.

Thanks Jakub.

c.

Jakub,

What do you think of something like this? Would it not also be valid to do something like "to - from" for the _STACK_GROWS_DOWN case?

I'm building with this patch right now.

c.

---
2003-10-18 Carlos O'Donell <[email protected]>

* attr.c (pthread_getattr_np): Add _STACK_GROWS_UP case when converting /proc/self/maps entry to __stackaddr and __stacksize.

Index: linuxthreads/attr.c
===================================================================
RCS file: /cvs/glibc/libc/linuxthreads/attr.c,v
retrieving revision 1.33

diff -u -p -r1.33 attr.c
@@ -455,12 +455,19 @@ int pthread_getattr_np (pthread_t thread
 		{
 		  /* Found the entry.  Now we have the info we need.  */
 		  attr->__stacksize = rl.rlim_cur;
+#ifdef _STACK_GROWS_DOWN
 		  attr->__stackaddr = (void *) to;

Jakub,

What do you think of something like this? Would it not also be valid to do something like "to - from" for the _STACK_GROWS_DOWN case?

I'm building with this patch right now.

c.

---
2003-10-18 Carlos O'Donell <[email protected]>

* attr.c (pthread_getattr_np): Add _STACK_GROWS_UP case when converting /proc/self/maps entry to __stackaddr and __stacksize.

Index: linuxthreads/attr.c
===================================================================
RCS file: /cvs/glibc/libc/linuxthreads/attr.c,v
retrieving revision 1.33

diff -u -p -r1.33 attr.c
@@ -455,12 +455,19 @@ int pthread_getattr_np (pthread_t thread
 		{
 		  /* Found the entry.  Now we have the info we need.  */
 		  attr->__stacksize = rl.rlim_cur;
+#ifdef _STACK_GROWS_DOWN
 		  attr->__stackaddr = (void *) to;
-
+#else
+                  attr->__stackaddr = (void *) from;
+#endif
 		  /* The limit might be too high.  This is a bogus
 		     situation but try to avoid making it worse.  */
 		  if ((size_t) attr->__stacksize > (size_t) attr->__stackaddr)
+#ifdef _STACK_GROWS_DOWN
 		    attr->__stacksize = (size_t) attr->__stackaddr;
+#else
+                    attr->__stacksize = (size_t)(to - from);
+#endif
 
 		  /* We succeed and no need to look further.  */
 		  ret = 0;