hppa: Add _STACK_GROWS_* cases to pthread_attr_[sg]etstack.

Message ID 5316B1E5.5070100@redhat.com
State Committed
Headers show

Commit Message

Carlos O'Donell March 5, 2014, 5:11 a.m. UTC
This is one of a several NPTL patches to build glibc on hppa.

The pthread_attr_[sg]etstack functions are defined by POSIX as
taking a stackaddr that is the lowest addressable byte of the
storage used for the stack. However, the internal iattr variable
of the same name in NPTL is actually the final stack address
as usable in the stack pointer for the machine. Therefore the
NPTL implementation must add and subtract stacksize for
_STACK_GROWS_DOWN architectures. HPPA is a _STACK_GROWS_UP
architecture and doesn't need to add or subtract anything,
the stack address *is* the lowest addressable byte of the
storage.

Tested on hppa-linux-gnu, with no regressions.

Can't impact any other targets because of the conditionals.

If nobody objects I'll check this in at the end of the week.

I can't see there being any objections to this patch except
that it introduces more code to maintain for an old architecture
(perhaps we'll get another _S_G_U target in the future?).

2014-03-05  Carlos O'Donell  <carlos@redhat.com>
                                                                                                                                                                                           
        * nptl/pthread_attr_setstack.c (__pthread_attr_setstack)
        [!_STACK_GROWS_DOWN]: Don't add stacksize to stackaddr.
        (__old_pthread_attr_setstack): Likewise.
        * nptl/pthread_attr_getstack.c (__pthread_attr_getstack)
        [!_STACK_GROWS_DOWN]: Likewise.

diff --git a/nptl/pthread_attr_setstack.c b/nptl/pthread_attr_setstack.c
index 19a5b54..4785501 100644
--- a/nptl/pthread_attr_setstack.cDon't add stacksize to stackaddr.

+++ b/nptl/pthread_attr_setstack.c
@@ -48,7 +48,11 @@ __pthread_attr_setstack (attr, stackaddr, stacksize)
 #endif
 
   iattr->stacksize = stacksize;
+#if _STACK_GROWS_DOWN
   iattr->stackaddr = (char *) stackaddr + stacksize;
+#else
+  iattr->stackaddr = (char *) stackaddr;
+#endif
   iattr->flags |= ATTR_FLAG_STACKADDR;
 
   return 0;
@@ -81,7 +85,11 @@ __old_pthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
 #  endif
 
   iattr->stacksize = stacksize;
+#if _STACK_GROWS_DOWN
   iattr->stackaddr = (char *) stackaddr + stacksize;
+#else
+  iattr->stackaddr = (char *) stackaddr;
+#endif
   iattr->flags |= ATTR_FLAG_STACKADDR;
 
   return 0;
---

Cheers,
Carlos.

Comments

Mike Frysinger March 11, 2014, 6:06 a.m. UTC | #1
On Wed 05 Mar 2014 00:11:01 Carlos O'Donell wrote:
> This is one of a several NPTL patches to build glibc on hppa.
> 
> The pthread_attr_[sg]etstack functions are defined by POSIX as
> taking a stackaddr that is the lowest addressable byte of the
> storage used for the stack. However, the internal iattr variable
> of the same name in NPTL is actually the final stack address
> as usable in the stack pointer for the machine. Therefore the
> NPTL implementation must add and subtract stacksize for
> _STACK_GROWS_DOWN architectures. HPPA is a _STACK_GROWS_UP
> architecture and doesn't need to add or subtract anything,
> the stack address *is* the lowest addressable byte of the
> storage.
> 
> Tested on hppa-linux-gnu, with no regressions.
> 
> Can't impact any other targets because of the conditionals.
> 
> If nobody objects I'll check this in at the end of the week.
> 
> I can't see there being any objections to this patch except
> that it introduces more code to maintain for an old architecture
> (perhaps we'll get another _S_G_U target in the future?).

other than the #if vs #ifdef weirdness, LGTM.  Gentoo has been shipping this 
for hppa since at least glibc-2.11, and has been using it on all arches since 
glibc-2.17 (which is our current stable).  no reports of weirdness yet.
-mike
Roland McGrath March 11, 2014, 6:16 p.m. UTC | #2
Needs to be consistently #if, not #ifdef.  OK otherwise.
Mike Frysinger March 14, 2014, 12:41 a.m. UTC | #3
On Wed 05 Mar 2014 00:11:01 Carlos O'Donell wrote:
> This is one of a several NPTL patches to build glibc on hppa.
> 
> The pthread_attr_[sg]etstack functions are defined by POSIX as
> taking a stackaddr that is the lowest addressable byte of the
> storage used for the stack. However, the internal iattr variable
> of the same name in NPTL is actually the final stack address
> as usable in the stack pointer for the machine. Therefore the
> NPTL implementation must add and subtract stacksize for
> _STACK_GROWS_DOWN architectures. HPPA is a _STACK_GROWS_UP
> architecture and doesn't need to add or subtract anything,
> the stack address *is* the lowest addressable byte of the
> storage.
> 
> Tested on hppa-linux-gnu, with no regressions.
> 
> Can't impact any other targets because of the conditionals.
> 
> If nobody objects I'll check this in at the end of the week.

sorry Carlos, i got impatient and fixed the #if/#ifdef and pushed it ;)

> I can't see there being any objections to this patch except
> that it introduces more code to maintain for an old architecture
> (perhaps we'll get another _S_G_U target in the future?).

if the metag arch ever gets merged, it's _S_G_U iiuc
-mike
Carlos O'Donell March 14, 2014, 1:34 a.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/13/2014 08:41 PM, Mike Frysinger wrote:
> On Wed 05 Mar 2014 00:11:01 Carlos O'Donell wrote:
>> This is one of a several NPTL patches to build glibc on hppa.
>>
>> The pthread_attr_[sg]etstack functions are defined by POSIX as
>> taking a stackaddr that is the lowest addressable byte of the
>> storage used for the stack. However, the internal iattr variable
>> of the same name in NPTL is actually the final stack address
>> as usable in the stack pointer for the machine. Therefore the
>> NPTL implementation must add and subtract stacksize for
>> _STACK_GROWS_DOWN architectures. HPPA is a _STACK_GROWS_UP
>> architecture and doesn't need to add or subtract anything,
>> the stack address *is* the lowest addressable byte of the
>> storage.
>>
>> Tested on hppa-linux-gnu, with no regressions.
>>
>> Can't impact any other targets because of the conditionals.
>>
>> If nobody objects I'll check this in at the end of the week.
> 
> sorry Carlos, i got impatient and fixed the #if/#ifdef and pushed it ;)

No worries. Thanks!

I have more patches queued up which I'll push directly
as long as they don't change code-gen for x86*

Cheers,
Carlos.

 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTIlyTAAoJECXvCkNsKkr/dssIAKRp+9kt9NcO5qqgaLFeN7Sv
MC9b5rXynx1ZMspN0dvCHWbRX6wuRbSJLy8OJKdDfmcGYNAXe3ZMDZGqpEUwnrhX
ddLh6E/KwQOpUfykzTmEj31zSspi8dNqL527LAM2OfSHT4iDKYCssrGExmRh2bjk
jqVHJVQ9vTE+WjTSMZFfuZAZO7l5W/GOUYlPaH7ZimB+DMiFD4Hgwza2HmElqzq7
KbdmvZamQrJE6bEVVf9kIlt67dV0wk6+tJiZIACx99D4z0VjSA8iQotsl9WIryoK
puKVy5x/brODffioxZr/b3EuaHiQzTeUciAovK650103t/4FAWI+PhUH0Qt50aI=
=Kf7z
-----END PGP SIGNATURE-----

Patch

diff --git a/nptl/pthread_attr_getstack.c b/nptl/pthread_attr_getstack.c
index 3f4fd8d..4c1a098 100644
--- a/nptl/pthread_attr_getstack.c
+++ b/nptl/pthread_attr_getstack.c
@@ -32,7 +32,11 @@  __pthread_attr_getstack (attr, stackaddr, stacksize)
   iattr = (struct pthread_attr *) attr;
 
   /* Store the result.  */
+#ifdef _STACK_GROWS_DOWN
   *stackaddr = (char *) iattr->stackaddr - iattr->stacksize;
+#else
+  *stackaddr = (char *) iattr->stackaddr;
+#endif
   *stacksize = iattr->stacksize;
 
   return 0;