Re: [PATCH v2] add powerpc64 port

[Bobby Bingham <koorogi@koorogi.info>]

On Wed, Apr 27, 2016 at 10:07:51PM -0400, Rich Felker wrote:
> On Wed, Apr 27, 2016 at 08:38:19PM -0500, Bobby Bingham wrote:
> > > > @@ -141 +141 @@
> > > > -mcontext_t: mcontext_t, mcontext_t*, size (*) [1272], align (*) [8]
> > > > +mcontext_t: sigcontext, sigcontext*, size (*) [1528], align (*) [8]
> > > 
> > > IIRC on other archs we made an effort to make the tag here match ABI
> > > (duplicating the struct def if needed). Not sure if it matters.
> > 
> > I can duplicate the structure if you want.  But it looks like glibc used
> > to do 'typedef struct sigcontext mcontext_t' as well:
> > 
> > https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h;h=a499a80ef9994e541b866202ee8440843b004fdd;hp=b75e25a3c84c852b22e1690ff530d3ddb8dff257;hb=5ef6ae4bdb;hpb=39b04aa39823faf1cc414e7f3eca4f43e01426e4
> 
> Wow, the glibc stuff is an utter mess and grossly non-conforming. It
> doesn't even have a uc_mcontext member of type mcontext_t. At least it

I went back and looked at some of the kernel history here.

For ppc32:
When this was added in 1998, struct ucontext didn't contain a uc_mcontext
member at all.  One was added in 2000, before the sigmask member, but it
difn't include registers directly, just a pointer to them.  The structure
was reworked in 2003 to the current layout, and hasn't changed since.

For ppc64:
When it was originally added in 2002, the uc_mcontext and uc_sigmask
members were swapped.  They were moved to their current positions in
2003, and haven't moved since.

> looks like all modern kernels have the mcontext_t at the end where
> it's extensible and doesn't break access to uc_sigmask if it changes.
> We should probably check and make sure both the current 32-bit powerpc
> ucontext_t and your proposed powerpc64 one have the mcontext_t members
> (at least the program counter) accessible at the expected locations.
> We should probably also let the kernel people know that mcontext_t
> (and its location) are ABI whether they like it or not. This is
> imposed by POSIX and the glibc hack of making it a pointer is
> non-conforming.

I don't think I got across the point I was trying to make, or got the
answer I was looking for.

The point was that until the commit linked above, glibc's mcontext_t
was a structure with tag 'sigcontext'.  That commit changed it.  I can
duplicate the structure definition if you want to match the tag with
current glibc, but since glibc changed the tag without worrying about
it, I'm not sure it really matters.

> 
> > > > @@ -195 +195 @@
> > > > -sem_t: sem_t, sem_t*, size (*) [32], align (*) [8]
> > > > +sem_t: sem_t, sem_t*, size (*) [32], align (*) [4]
> > > 
> > > > @@ -229,2 +229,2 @@
> > > > -cmsghdr: cmsghdr, cmsghdr*, size (*) [16], align (*) [8]
> > > > +cmsghdr: cmsghdr, cmsghdr*, size (*) [16], align (*) [4]
> > > 
> > > This is likely going to hit the same issue we're trying to debug on
> > > mips64.
> > 
> > The mips64 issue ended up not being alignment related.  Do you still
> > want me to do something about this?  And if so, do you have a suggestion?
> 
> I'm not entirely sure, but I think you can leave it alone for now. If
> it needs fixing here, then it already does on multiple existing archs,
> and they should be handled together.

Ok.

> 
> > > > @@ -416 +417 @@
> > > > -ucontext_t: ucontext, ucontext*, size (*) [1440], align (*) [8]
> > > > +ucontext_t: ucontext, ucontext*, size (*) [1696], align (*) [8]
> > > 
> > > This may be a real problem. ucontext_t is ABI between kernel and
> > > userspace and if it's wrong cancellation won't work right.
> > 
> > Kernel commit ce48b2100785 expanded the vmx_reserve member of mcontext_t
> > by 256 bytes.  The glibc headers haven't been updated for this expansion.
> 
> Ah. As noted above, uc_mcontext is at the end so it shouldn't break
> anything.

Right.  I was just explaining the size difference in this structure
between glibc and my patch.

> 
> Rich