Bug 21016 - pthread_cond support is broken on hppa
Summary: pthread_cond support is broken on hppa
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.26
: P2 normal
Target Milestone: 2.26
Assignee: Carlos O'Donell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-02 16:57 UTC by John David Anglin
Modified: 2017-08-12 17:05 UTC (History)
6 users (show)

See Also:
Host: hppa-unknown-linux-gnu
Target: hppa-unknown-linux-gnu
Build: hppa-unknown-linux-gnu
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John David Anglin 2017-01-02 16:57:37 UTC
Trunk build fails here:

gcc ../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c -c -std=gnu11 -fgnu89-in
line  -O2 -Wall -Werror -Wundef -Wwrite-strings -fmerge-all-constants -fno-stack
-protector -frounding-math -g -Wstrict-prototypes -Wold-style-definition     -ft
ls-model=initial-exec      -I../include -I/home/dave/gnu/glibc/objdir/nptl  -I/home/dave/gnu/glibc/objdir  -I../sysdeps/unix/sysv/linux/hppa  -I../sysdeps/hppa/
nptl  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux  -I../sy
sdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu  -I../sysdeps/unix/inet  -I..
/sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix  -I../sysdeps/hppa/hpp
a1.1  -I../sysdeps/wordsize-32  -I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee75
4/dbl-64  -I../sysdeps/hppa/fpu  -I../sysdeps/hppa  -I../sysdeps/ieee754  -I../s
ysdeps/generic  -I.. -I../libio -I. -nostdinc -isystem /usr/lib/gcc/hppa-linux-g
nu/6/include -isystem /usr/lib/gcc/hppa-linux-gnu/6/include-fixed -isystem /usr/
include  -D_LIBC_REENTRANT -include /home/dave/gnu/glibc/objdir/libc-modules.h -
DMODULE_NAME=libpthread -include ../include/libc-symbols.h       -o /home/dave/g
nu/glibc/objdir/nptl/pthread_cond_init.o -MD -MP -MF /home/dave/gnu/glibc/objdir/nptl/pthread_cond_init.o.dt -MT /home/dave/gnu/glibc/objdir/nptl/pthread_cond_i
nit.o
In file included from ./pthreadP.h:30:0,
                 from ../sysdeps/unix/sysv/linux/hppa/pthreadP.h:1,                 from ../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:24,
                 from ../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:21:
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c: In function ‘__pthread_cond_init’:
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:51:14: error: ‘struct <anonymous>’ has no member named ‘__wseq’
   var->__data.__wseq = 0;      \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:52:14: error: ‘struct <anonymous>’ has no member named ‘__signals_sent’; did you mean ‘__total_seq’?
   var->__data.__signals_sent = 0;     \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:53:14: error: ‘struct <anonymous>’ has no member named ‘__confirmed’
   var->__data.__confirmed = 0;      \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:54:14: error: ‘struct <anonymous>’ has no member named ‘__generation’
   var->__data.__generation = 0;      \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:56:14: error: ‘struct <anonymous>’ has no member named ‘__quiescence_waiters’; did you mean ‘__nwaiters’?
   var->__data.__quiescence_waiters = 0;     \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:57:14: error: ‘struct <anonymous>’ has no member named ‘__clockid’; did you mean ‘__lock’?
   var->__data.__clockid = 0;      \
              ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/hppa/internaltypes.h:50:7: error: unused variable ‘tmp’ [-Werror=unused-variable]
   int tmp = 0;        \
       ^
../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:30:3: note: in expansion of macro ‘cond_compat_clear’
   cond_compat_clear (cond);
   ^~~~~~~~~~~~~~~~~
In file included from ../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:39:0,
                 from ../sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c:21:
./pthread_cond_init.c: In function ‘__pthread_cond_init_internal’:
./pthread_cond_init.c:38:17: error: ‘struct <anonymous>’ has no member named ‘__wrefs’
     cond->__data.__wrefs |= __PTHREAD_COND_SHARED_MASK;
                 ^
./pthread_cond_init.c:44:17: error: ‘struct <anonymous>’ has no member named ‘__wrefs’
     cond->__data.__wrefs |= __PTHREAD_COND_CLOCK_MONOTONIC_MASK;
                 ^
cc1: all warnings being treated as errors
/home/dave/gnu/glibc/objdir/sysd-rules:9: recipe for target '/home/dave/gnu/glibc/objdir/nptl/pthread_cond_init.o' failed
make[2]: *** [/home/dave/gnu/glibc/objdir/nptl/pthread_cond_init.o] Error 1
make[2]: Leaving directory '/home/dave/gnu/glibc/glibc/nptl'
Makefile:215: recipe for target 'nptl/subdir_lib' failed
make[1]: *** [nptl/subdir_lib] Error 2
make[1]: Leaving directory '/home/dave/gnu/glibc/glibc'
Makefile:9: recipe for target 'all' failed
make: *** [all] Error 2
Comment 1 John David Anglin 2017-01-02 16:59:10 UTC
I believe this was introduced by the following patch:
https://sourceware.org/ml/libc-alpha/2016-05/msg00605.html
Comment 2 Torvald Riegel 2017-01-02 20:21:47 UTC
Carlos has said he would take care of it.  I'm aware of two issues blocking this: atomics don't work in all cases (stores should use the kernel helper too), and the additional compatibility requirements for old initializers (IIRC).  To get the condvar files to compile, just copy over the new pthread_cond_t members from, say, x86 pthreadtypes.h to hppa's pthreadtypes.h.
Comment 3 dave.anglin 2017-01-04 21:02:17 UTC
On 2017-01-02, at 3:21 PM, triegel at redhat dot com wrote:

> Carlos has said he would take care of it.  I'm aware of two issues blocking
> this: atomics don't work in all cases (stores should use the kernel helper
> too), and the additional compatibility requirements for old initializers
> (IIRC).  To get the condvar files to compile, just copy over the new
> pthread_cond_t members from, say, x86 pthreadtypes.h to hppa's pthreadtypes.h.


I had a successful build by copying over the pthread_cond_t initial member from x86.
I kept the old hppa member for the moment to get the same current alignment.  Fortunately,
the sizes are the same (48).  I updated PTHREAD_COND_INITIALIZER to default.

I removed the LinuxThreads compatibility code although this is still a work in progress:

	modified:   sysdeps/hppa/nptl/bits/pthreadtypes.h
	deleted:    sysdeps/unix/sysv/linux/hppa/internaltypes.h
	modified:   sysdeps/unix/sysv/linux/hppa/pthread.h
	deleted:    sysdeps/unix/sysv/linux/hppa/pthread_cond_broadcast.c
	deleted:    sysdeps/unix/sysv/linux/hppa/pthread_cond_destroy.c
	modified:   sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c
	deleted:    sysdeps/unix/sysv/linux/hppa/pthread_cond_signal.c
	deleted:    sysdeps/unix/sysv/linux/hppa/pthread_cond_wait.c

Would like to get rid of pthread_cond_init.c and use default.

There is essentially zero code still using LinuxThreads in Debian, so I can't see any reason to
try and retain compatibility with it.  With new union, I don't see how to reliably check for the
LinuxThreads initializer.

In gcc, we use the kernel helper for stores so hopefully implementing this shouldn't be hard.

--
John David Anglin	dave.anglin@bell.net
Comment 4 Richard Henderson 2017-03-12 01:24:16 UTC
Any movement on this?  It is now more than two months without
the hppa port being buildable at all.
Comment 5 dave.anglin 2017-03-12 15:03:39 UTC
On 2017-03-11, at 8:24 PM, rth at gcc dot gnu.org wrote:

> Any movement on this?  It is now more than two months without
> the hppa port being buildable at all.

I submitted a patch to fix the build here:
https://sourceware.org/ml/libc-alpha/2017-03/msg00181.html

--
John David Anglin	dave.anglin@bell.net
Comment 6 Carlos O'Donell 2017-03-13 23:10:49 UTC
(In reply to Richard Henderson from comment #4)
> Any movement on this?  It is now more than two months without
> the hppa port being buildable at all.

Yes, I have a build mostly fixed, the problem is we ran out of space in pthread_cond_t to implement the new algorithm _and_ support old-style Linuxthreads initializers.

So I guess a good question for you and Dave is:

* Should we stop supporting compat with old style static linuxthreads initializers?

* Should we continue to support compat old style static linuxthreads initializers, stealing 1-bit from the group size structure in the new algorithm which would have a barely noticeable extra load on overflow.

Thoughts?
Comment 7 Richard Henderson 2017-03-13 23:32:41 UTC
(In reply to Carlos O'Donell from comment #6)
> * Should we stop supporting compat with old style static linuxthreads
> initializers?

I don't see any compelling reason to do so.

I think it would be preferable to minimize the amount of code
specific to hppa.  That, in the long run, is more helpful for
keeping minor platforms running.

As David has said, LinuxThreads has been gone for at least a decade.
I don't find it credible that there are binaries in any distribution
that haven't been recompiled since then.
Comment 8 Carlos O'Donell 2017-03-14 00:02:43 UTC
(In reply to Richard Henderson from comment #7)
> (In reply to Carlos O'Donell from comment #6)
> > * Should we stop supporting compat with old style static linuxthreads
> > initializers?
> 
> I don't see any compelling reason to do so.
> 
> I think it would be preferable to minimize the amount of code
> specific to hppa.  That, in the long run, is more helpful for
> keeping minor platforms running.
> 
> As David has said, LinuxThreads has been gone for at least a decade.
> I don't find it credible that there are binaries in any distribution
> that haven't been recompiled since then.

Given that Dave has already weighed in on the topic by posting a patch to kill the compat code I'll install that on Thursday unless someone beats me to it.
Comment 9 dave.anglin 2017-03-14 00:04:12 UTC
On 2017-03-13, at 7:32 PM, rth at gcc dot gnu.org wrote:

> As David has said, LinuxThreads has been gone for at least a decade.
> I don't find it credible that there are binaries in any distribution
> that haven't been recompiled since then.

Agree.

--
John David Anglin	dave.anglin@bell.net
Comment 10 John David Anglin 2017-07-09 19:24:50 UTC
Fixed by commit c5f70682a53ae118a1541fe5a8142ca1008098ba.
Comment 11 Sourceware Commits 2017-08-12 17:05:59 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.25/master has been updated
       via  241af18538d25e020428c66a50884f7cc37f0385 (commit)
      from  f827b1cec3e87c282727f8f90b9d4a660eff8443 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=241af18538d25e020428c66a50884f7cc37f0385

commit 241af18538d25e020428c66a50884f7cc37f0385
Author: John David Anglin <danglin@gcc.gnu.org>
Date:   Sat Aug 12 13:02:52 2017 -0400

    Use generic pthread support on hppa.
    
            [BZ #21016]
            * sysdeps/hppa/nptl/bits/pthreadtypes.h: Update pthread_cond_t typedef.
            * sysdeps/unix/sysv/linux/hppa/pthread.h: Include
            bits/types/struct_timespec.h.
            (PTHREAD_MUTEX_INITIALIZER): Revise define.
            (PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP): Likewise.
            (PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP): Likewise.
            (PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP): Likewise.
            (PTHREAD_RWLOCK_INITIALIZER): Likewise.
            (PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP): Likewise.
            (PTHREAD_COND_INITIALIZER): Likewise.
            Remove old definitions.
            * sysdeps/unix/sysv/linux/hppa/internaltypes.h: Delete.
            * sysdeps/unix/sysv/linux/hppa/pthread_cond_broadcast.c: Delete.
            * sysdeps/unix/sysv/linux/hppa/pthread_cond_destroy.c: Delete.
            * sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c: Delete.
            * sysdeps/unix/sysv/linux/hppa/pthread_cond_signal.c: Delete.
            * sysdeps/unix/sysv/linux/hppa/pthread_cond_wait.c: Delete.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                          |   21 ++++
 sysdeps/hppa/nptl/bits/pthreadtypes.h              |   50 +++++-----
 sysdeps/unix/sysv/linux/hppa/internaltypes.h       |   84 ----------------
 sysdeps/unix/sysv/linux/hppa/pthread.h             |  103 ++++----------------
 .../unix/sysv/linux/hppa/pthread_cond_broadcast.c  |   40 --------
 .../unix/sysv/linux/hppa/pthread_cond_destroy.c    |   40 --------
 sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c   |   40 --------
 sysdeps/unix/sysv/linux/hppa/pthread_cond_signal.c |   40 --------
 sysdeps/unix/sysv/linux/hppa/pthread_cond_wait.c   |   53 ----------
 9 files changed, 64 insertions(+), 407 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/internaltypes.h
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/pthread_cond_broadcast.c
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/pthread_cond_destroy.c
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/pthread_cond_init.c
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/pthread_cond_signal.c
 delete mode 100644 sysdeps/unix/sysv/linux/hppa/pthread_cond_wait.c