[PATCH] powerpc64: use a type for mcontext_t regs field

[PATCH] powerpc64: use a type for mcontext_t regs field

[A. Wilcox]

GCC Go dereferences `regs` for `nip`.  Without this change, compilation
fails with the following message:

../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
  ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
                                                        ^~
../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
---
 arch/powerpc64/bits/signal.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
index 34693a68..6736c69a 100644
--- a/arch/powerpc64/bits/signal.h
+++ b/arch/powerpc64/bits/signal.h
@@ -8,6 +8,8 @@
 
 #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 
+#include <bits/user.h>
+
 typedef unsigned long greg_t, gregset_t[48];
 
 typedef struct {
@@ -29,7 +31,7 @@ typedef struct sigcontext {
 	int _pad0;
 	unsigned long handler;
 	unsigned long oldmask;
-	void *regs;
+	struct pt_regs *regs;
 	gregset_t gp_regs;
 	fpregset_t fp_regs;
 	vrregset_t *v_regs;
-- 
2.19.2

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[Rich Felker]

On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> fails with the following message:
> 
> .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
>   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
>                                                         ^~
> .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> ---
>  arch/powerpc64/bits/signal.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> index 34693a68..6736c69a 100644
> --- a/arch/powerpc64/bits/signal.h
> +++ b/arch/powerpc64/bits/signal.h
> @@ -8,6 +8,8 @@
>  
>  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
>  
> +#include <bits/user.h>
> +

This is almost surely not right. You can never use a bits/* header
except from the corresponding public header that includes it. They do
not have multiple-inclusion guards and are not designed to be usable
independently. And including sys/user.h is also almost surely wrong
since it's a problematic header you don't want getting included
(conflicts with linux/*.h stuff).

>  typedef unsigned long greg_t, gregset_t[48];
>  
>  typedef struct {
> @@ -29,7 +31,7 @@ typedef struct sigcontext {
>  	int _pad0;
>  	unsigned long handler;
>  	unsigned long oldmask;
> -	void *regs;
> +	struct pt_regs *regs;
>  	gregset_t gp_regs;
>  	fpregset_t fp_regs;
>  	vrregset_t *v_regs;
> -- 
> 2.19.2

Do you know if there's a reason Go is trying to use this regs member
rather than gp_regs? I think that's the real issue here.

Rich

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[David Edelsohn]

On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > fails with the following message:
> >
> > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> >                                                         ^~
> > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > ---
> >  arch/powerpc64/bits/signal.h | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > index 34693a68..6736c69a 100644
> > --- a/arch/powerpc64/bits/signal.h
> > +++ b/arch/powerpc64/bits/signal.h
> > @@ -8,6 +8,8 @@
> >
> >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> >
> > +#include <bits/user.h>
> > +
>
> This is almost surely not right. You can never use a bits/* header
> except from the corresponding public header that includes it. They do
> not have multiple-inclusion guards and are not designed to be usable
> independently. And including sys/user.h is also almost surely wrong
> since it's a problematic header you don't want getting included
> (conflicts with linux/*.h stuff).
>
> >  typedef unsigned long greg_t, gregset_t[48];
> >
> >  typedef struct {
> > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> >       int _pad0;
> >       unsigned long handler;
> >       unsigned long oldmask;
> > -     void *regs;
> > +     struct pt_regs *regs;
> >       gregset_t gp_regs;
> >       fpregset_t fp_regs;
> >       vrregset_t *v_regs;
> > --
> > 2.19.2
>
> Do you know if there's a reason Go is trying to use this regs member
> rather than gp_regs? I think that's the real issue here.

Apparently GCCGo runtime has dependencies on GLibc and has not been
ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
a signal.

#ifdef __PPC__
 #ifdef __linux__
       ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
 #endif
 #ifdef _AIX
       ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
 #endif
#endif

The program counter is not part of gp_regs.

I presume that someone is trying to use GCCGo with Alpine Linux.  I
believe that Golang works with Alpine Linux.

Thanks, David

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[Rich Felker]

On Tue, Feb 12, 2019 at 12:05:17PM -0500, David Edelsohn wrote:
> On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
> >
> > On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > > fails with the following message:
> > >
> > > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> > >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > >                                                         ^~
> > > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > > ---
> > >  arch/powerpc64/bits/signal.h | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > > index 34693a68..6736c69a 100644
> > > --- a/arch/powerpc64/bits/signal.h
> > > +++ b/arch/powerpc64/bits/signal.h
> > > @@ -8,6 +8,8 @@
> > >
> > >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > >
> > > +#include <bits/user.h>
> > > +
> >
> > This is almost surely not right. You can never use a bits/* header
> > except from the corresponding public header that includes it. They do
> > not have multiple-inclusion guards and are not designed to be usable
> > independently. And including sys/user.h is also almost surely wrong
> > since it's a problematic header you don't want getting included
> > (conflicts with linux/*.h stuff).
> >
> > >  typedef unsigned long greg_t, gregset_t[48];
> > >
> > >  typedef struct {
> > > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> > >       int _pad0;
> > >       unsigned long handler;
> > >       unsigned long oldmask;
> > > -     void *regs;
> > > +     struct pt_regs *regs;
> > >       gregset_t gp_regs;
> > >       fpregset_t fp_regs;
> > >       vrregset_t *v_regs;
> > > --
> > > 2.19.2
> >
> > Do you know if there's a reason Go is trying to use this regs member
> > rather than gp_regs? I think that's the real issue here.
> 
> Apparently GCCGo runtime has dependencies on GLibc and has not been
> ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
> a signal.
> 
> #ifdef __PPC__
>  #ifdef __linux__
>        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
>  #endif
>  #ifdef _AIX
>        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
>  #endif
> #endif
> 
> The program counter is not part of gp_regs.

Yes it is, as musl already depends on having access to the program
counter for thread cancellation. See arch/powerpc64/pthread_arch.h:

// the kernel calls the ip "nip", it's the first saved value after the 32
// GPRs.
#define MC_PC gp_regs[32]

Rich

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[David Edelsohn]

On Tue, Feb 12, 2019 at 12:23 PM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Feb 12, 2019 at 12:05:17PM -0500, David Edelsohn wrote:
> > On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > > > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > > > fails with the following message:
> > > >
> > > > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > > > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> > > >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > >                                                         ^~
> > > > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > > > ---
> > > >  arch/powerpc64/bits/signal.h | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > > > index 34693a68..6736c69a 100644
> > > > --- a/arch/powerpc64/bits/signal.h
> > > > +++ b/arch/powerpc64/bits/signal.h
> > > > @@ -8,6 +8,8 @@
> > > >
> > > >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > > >
> > > > +#include <bits/user.h>
> > > > +
> > >
> > > This is almost surely not right. You can never use a bits/* header
> > > except from the corresponding public header that includes it. They do
> > > not have multiple-inclusion guards and are not designed to be usable
> > > independently. And including sys/user.h is also almost surely wrong
> > > since it's a problematic header you don't want getting included
> > > (conflicts with linux/*.h stuff).
> > >
> > > >  typedef unsigned long greg_t, gregset_t[48];
> > > >
> > > >  typedef struct {
> > > > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> > > >       int _pad0;
> > > >       unsigned long handler;
> > > >       unsigned long oldmask;
> > > > -     void *regs;
> > > > +     struct pt_regs *regs;
> > > >       gregset_t gp_regs;
> > > >       fpregset_t fp_regs;
> > > >       vrregset_t *v_regs;
> > > > --
> > > > 2.19.2
> > >
> > > Do you know if there's a reason Go is trying to use this regs member
> > > rather than gp_regs? I think that's the real issue here.
> >
> > Apparently GCCGo runtime has dependencies on GLibc and has not been
> > ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
> > a signal.
> >
> > #ifdef __PPC__
> >  #ifdef __linux__
> >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> >  #endif
> >  #ifdef _AIX
> >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
> >  #endif
> > #endif
> >
> > The program counter is not part of gp_regs.
>
> Yes it is, as musl already depends on having access to the program
> counter for thread cancellation. See arch/powerpc64/pthread_arch.h:
>
> // the kernel calls the ip "nip", it's the first saved value after the 32
> // GPRs.
> #define MC_PC gp_regs[32]

Apparently Ian Taylor now is aware of this and investigating a
solution -- possibly another ifdef in the above for PPC Musl Libc.

 - David

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[Rich Felker]

On Tue, Feb 12, 2019 at 01:17:34PM -0500, David Edelsohn wrote:
> On Tue, Feb 12, 2019 at 12:23 PM Rich Felker <dalias@libc.org> wrote:
> >
> > On Tue, Feb 12, 2019 at 12:05:17PM -0500, David Edelsohn wrote:
> > > On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
> > > >
> > > > On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > > > > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > > > > fails with the following message:
> > > > >
> > > > > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > > > > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> > > > >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > > >                                                         ^~
> > > > > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > > > > ---
> > > > >  arch/powerpc64/bits/signal.h | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > > > > index 34693a68..6736c69a 100644
> > > > > --- a/arch/powerpc64/bits/signal.h
> > > > > +++ b/arch/powerpc64/bits/signal.h
> > > > > @@ -8,6 +8,8 @@
> > > > >
> > > > >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > > > >
> > > > > +#include <bits/user.h>
> > > > > +
> > > >
> > > > This is almost surely not right. You can never use a bits/* header
> > > > except from the corresponding public header that includes it. They do
> > > > not have multiple-inclusion guards and are not designed to be usable
> > > > independently. And including sys/user.h is also almost surely wrong
> > > > since it's a problematic header you don't want getting included
> > > > (conflicts with linux/*.h stuff).
> > > >
> > > > >  typedef unsigned long greg_t, gregset_t[48];
> > > > >
> > > > >  typedef struct {
> > > > > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> > > > >       int _pad0;
> > > > >       unsigned long handler;
> > > > >       unsigned long oldmask;
> > > > > -     void *regs;
> > > > > +     struct pt_regs *regs;
> > > > >       gregset_t gp_regs;
> > > > >       fpregset_t fp_regs;
> > > > >       vrregset_t *v_regs;
> > > > > --
> > > > > 2.19.2
> > > >
> > > > Do you know if there's a reason Go is trying to use this regs member
> > > > rather than gp_regs? I think that's the real issue here.
> > >
> > > Apparently GCCGo runtime has dependencies on GLibc and has not been
> > > ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
> > > a signal.
> > >
> > > #ifdef __PPC__
> > >  #ifdef __linux__
> > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > >  #endif
> > >  #ifdef _AIX
> > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
> > >  #endif
> > > #endif
> > >
> > > The program counter is not part of gp_regs.
> >
> > Yes it is, as musl already depends on having access to the program
> > counter for thread cancellation. See arch/powerpc64/pthread_arch.h:
> >
> > // the kernel calls the ip "nip", it's the first saved value after the 32
> > // GPRs.
> > #define MC_PC gp_regs[32]
> 
> Apparently Ian Taylor now is aware of this and investigating a
> solution -- possibly another ifdef in the above for PPC Musl Libc.

This is not a public #define that you could #ifdef on. It's part of
musl thread implementation internals and only present in the musl
source tree, not anywhere installed. I just showed it to demonstrate
that the NIP register (as well as 15 other registers that are just not
the 32 numbered GPRs) is part of the gp_regs array.

Rich

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[David Edelsohn]

On Tue, Feb 12, 2019 at 1:21 PM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Feb 12, 2019 at 01:17:34PM -0500, David Edelsohn wrote:
> > On Tue, Feb 12, 2019 at 12:23 PM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Tue, Feb 12, 2019 at 12:05:17PM -0500, David Edelsohn wrote:
> > > > On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
> > > > >
> > > > > On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > > > > > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > > > > > fails with the following message:
> > > > > >
> > > > > > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > > > > > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> > > > > >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > > > >                                                         ^~
> > > > > > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > > > > > ---
> > > > > >  arch/powerpc64/bits/signal.h | 4 +++-
> > > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > > > > > index 34693a68..6736c69a 100644
> > > > > > --- a/arch/powerpc64/bits/signal.h
> > > > > > +++ b/arch/powerpc64/bits/signal.h
> > > > > > @@ -8,6 +8,8 @@
> > > > > >
> > > > > >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > > > > >
> > > > > > +#include <bits/user.h>
> > > > > > +
> > > > >
> > > > > This is almost surely not right. You can never use a bits/* header
> > > > > except from the corresponding public header that includes it. They do
> > > > > not have multiple-inclusion guards and are not designed to be usable
> > > > > independently. And including sys/user.h is also almost surely wrong
> > > > > since it's a problematic header you don't want getting included
> > > > > (conflicts with linux/*.h stuff).
> > > > >
> > > > > >  typedef unsigned long greg_t, gregset_t[48];
> > > > > >
> > > > > >  typedef struct {
> > > > > > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> > > > > >       int _pad0;
> > > > > >       unsigned long handler;
> > > > > >       unsigned long oldmask;
> > > > > > -     void *regs;
> > > > > > +     struct pt_regs *regs;
> > > > > >       gregset_t gp_regs;
> > > > > >       fpregset_t fp_regs;
> > > > > >       vrregset_t *v_regs;
> > > > > > --
> > > > > > 2.19.2
> > > > >
> > > > > Do you know if there's a reason Go is trying to use this regs member
> > > > > rather than gp_regs? I think that's the real issue here.
> > > >
> > > > Apparently GCCGo runtime has dependencies on GLibc and has not been
> > > > ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
> > > > a signal.
> > > >
> > > > #ifdef __PPC__
> > > >  #ifdef __linux__
> > > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > >  #endif
> > > >  #ifdef _AIX
> > > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
> > > >  #endif
> > > > #endif
> > > >
> > > > The program counter is not part of gp_regs.
> > >
> > > Yes it is, as musl already depends on having access to the program
> > > counter for thread cancellation. See arch/powerpc64/pthread_arch.h:
> > >
> > > // the kernel calls the ip "nip", it's the first saved value after the 32
> > > // GPRs.
> > > #define MC_PC gp_regs[32]
> >
> > Apparently Ian Taylor now is aware of this and investigating a
> > solution -- possibly another ifdef in the above for PPC Musl Libc.
>
> This is not a public #define that you could #ifdef on. It's part of
> musl thread implementation internals and only present in the musl
> source tree, not anywhere installed. I just showed it to demonstrate
> that the NIP register (as well as 15 other registers that are just not
> the 32 numbered GPRs) is part of the gp_regs array.

I meant that it appears this should be fixed in GCC Go runtime --
somehow -- not in Musl Libc referring to nip.  GCC Go apparently will
need another case for Musl Libc and somehow know to reference
gp_regs[32], or some other way to know about the internals of Musl
Libc and access the program counter.

- David

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[Rich Felker]

On Tue, Feb 12, 2019 at 01:24:09PM -0500, David Edelsohn wrote:
> On Tue, Feb 12, 2019 at 1:21 PM Rich Felker <dalias@libc.org> wrote:
> >
> > On Tue, Feb 12, 2019 at 01:17:34PM -0500, David Edelsohn wrote:
> > > On Tue, Feb 12, 2019 at 12:23 PM Rich Felker <dalias@libc.org> wrote:
> > > >
> > > > On Tue, Feb 12, 2019 at 12:05:17PM -0500, David Edelsohn wrote:
> > > > > On Tue, Feb 12, 2019 at 11:18 AM Rich Felker <dalias@libc.org> wrote:
> > > > > >
> > > > > > On Tue, Feb 12, 2019 at 09:35:22AM -0600, A. Wilcox wrote:
> > > > > > > GCC Go dereferences `regs` for `nip`.  Without this change, compilation
> > > > > > > fails with the following message:
> > > > > > >
> > > > > > > .../../../libgo/runtime/go-signal.c: In function ‘getSiginfo’:
> > > > > > > .../../../libgo/runtime/go-signal.c:225:56: warning: dereferencing ‘void *’ pointer
> > > > > > >   ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > > > > >                                                         ^~
> > > > > > > .../../../libgo/runtime/go-signal.c:225:56: error: request for member ‘nip’ in something not a structure or union
> > > > > > > ---
> > > > > > >  arch/powerpc64/bits/signal.h | 4 +++-
> > > > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/powerpc64/bits/signal.h b/arch/powerpc64/bits/signal.h
> > > > > > > index 34693a68..6736c69a 100644
> > > > > > > --- a/arch/powerpc64/bits/signal.h
> > > > > > > +++ b/arch/powerpc64/bits/signal.h
> > > > > > > @@ -8,6 +8,8 @@
> > > > > > >
> > > > > > >  #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > > > > > >
> > > > > > > +#include <bits/user.h>
> > > > > > > +
> > > > > >
> > > > > > This is almost surely not right. You can never use a bits/* header
> > > > > > except from the corresponding public header that includes it. They do
> > > > > > not have multiple-inclusion guards and are not designed to be usable
> > > > > > independently. And including sys/user.h is also almost surely wrong
> > > > > > since it's a problematic header you don't want getting included
> > > > > > (conflicts with linux/*.h stuff).
> > > > > >
> > > > > > >  typedef unsigned long greg_t, gregset_t[48];
> > > > > > >
> > > > > > >  typedef struct {
> > > > > > > @@ -29,7 +31,7 @@ typedef struct sigcontext {
> > > > > > >       int _pad0;
> > > > > > >       unsigned long handler;
> > > > > > >       unsigned long oldmask;
> > > > > > > -     void *regs;
> > > > > > > +     struct pt_regs *regs;
> > > > > > >       gregset_t gp_regs;
> > > > > > >       fpregset_t fp_regs;
> > > > > > >       vrregset_t *v_regs;
> > > > > > > --
> > > > > > > 2.19.2
> > > > > >
> > > > > > Do you know if there's a reason Go is trying to use this regs member
> > > > > > rather than gp_regs? I think that's the real issue here.
> > > > >
> > > > > Apparently GCCGo runtime has dependencies on GLibc and has not been
> > > > > ported to Musl Libc.  The GCCGo runtime is trying to obtain the PC for
> > > > > a signal.
> > > > >
> > > > > #ifdef __PPC__
> > > > >  #ifdef __linux__
> > > > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
> > > > >  #endif
> > > > >  #ifdef _AIX
> > > > >        ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.jmp_context.iar;
> > > > >  #endif
> > > > > #endif
> > > > >
> > > > > The program counter is not part of gp_regs.
> > > >
> > > > Yes it is, as musl already depends on having access to the program
> > > > counter for thread cancellation. See arch/powerpc64/pthread_arch.h:
> > > >
> > > > // the kernel calls the ip "nip", it's the first saved value after the 32
> > > > // GPRs.
> > > > #define MC_PC gp_regs[32]
> > >
> > > Apparently Ian Taylor now is aware of this and investigating a
> > > solution -- possibly another ifdef in the above for PPC Musl Libc.
> >
> > This is not a public #define that you could #ifdef on. It's part of
> > musl thread implementation internals and only present in the musl
> > source tree, not anywhere installed. I just showed it to demonstrate
> > that the NIP register (as well as 15 other registers that are just not
> > the 32 numbered GPRs) is part of the gp_regs array.
> 
> I meant that it appears this should be fixed in GCC Go runtime --
> somehow -- not in Musl Libc referring to nip.  GCC Go apparently will
> need another case for Musl Libc and somehow know to reference
> gp_regs[32], or some other way to know about the internals of Musl
> Libc and access the program counter.

This is not knowledge of musl internals. Both accessing a pt_regs
struct pointed to by the regs member, and accessing gp_regs[32], are
part of the kernel ABI for powerpc64 mcontext_t. musl simply does not
expose the struct pt_regs type here because there's no safe way to get
a definition that doesn't clash with kernel headers, so I suggested
using the alternate way that *also works on glibc*. I can't see any
reason to make this change conditional on musl.

I think it would also be acceptable to change the void * to struct
pt_regs * in musl's mcontext_t, but then it would be a pointer to
incomplete type unless you also include sys/user.h. Generally, user.h
is an awful header you want to avoid unless you have no choice,
though, so I think it's highly preferable for gccgo *not* to use it,
even if we do make this change, and instead use the gp_regs NIP slot
that avoids touching anything from user.h.

Rich

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[A. Wilcox]

[-- Attachment #1.1: Type: text/plain, Size: 1429 bytes --]

On 02/12/19 11:05, David Edelsohn wrote:
> I presume that someone is trying to use GCCGo with Alpine Linux.  I
> believe that Golang works with Alpine Linux.
> 
> Thanks, David


We're still here.  Adélie.  The distro that does big endian and ELFv2.
The distro that *cares* about *quality*, not just hack-and-slashing.

We can't use Golang because Golang doesn't work on half of our
architectures - ppc32 will likely never be supported, and their ppc64
support is unusable and written very poorly.  It assumes POWER8, not to
mention the fact it likely has plenty of broken ELF ABI assumptions.
(There's also very poor support for other architectures with musl, but I
don't recall the specifics now.)

I'm sure you're aware of all of this already.  In fact, it was probably
your team that convinced Google that POWER7 is "old" when it is newer
than Sandy Bridge:

https://github.com/golang/go/issues/19074#issuecomment-436101375

But I'm not sure if you work directly with Lynn or not, so I can't say
that for certain.

Anyway, GCC Go is definitely the best way forward for Adélie, which is
why we're trying to make it work on ppc64.  It already works perfectly
well on ppc32, x86*, and arm64.  And because it uses GCC's code
generator, it won't be subject to ridiculously unportable ABI assumptions.

--arw

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
https://www.adelielinux.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[James Larrowe]

[-- Attachment #1: Type: text/plain, Size: 135 bytes --]

Sorry, but GCC Go 8 doesn't even compile correctly on x86_64. You might
want to look into that. Something about 27 already being used.

[-- Attachment #2: Type: text/html, Size: 161 bytes --]

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[A. Wilcox]

[-- Attachment #1.1: Type: text/plain, Size: 783 bytes --]

On 02/12/19 12:42, James Larrowe wrote:
> Sorry, but GCC Go 8 doesn't even compile correctly on x86_64. You might
> want to look into that. Something about 27 already being used.

GCC 8 in general is a horrible pain in my side, but I'm trying to
actually identify bugs in it one at a time, so that they can be fixed.

Thanks for the tip, I'll keep a look out for it.

The reason I didn't stumble on it yet is because PPC64 is our primary
architecture; it's what I use daily for a workstation (Talos II, Power9)
and for media (iMac G5, 970fx).  Once the issues here are ironed out,
I'll be looking at the other tier1 architectures, including x86_64.

Best to you and yours,
--arw

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
https://www.adelielinux.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] powerpc64: use a type for mcontext_t regs field

[David Edelsohn]

On Tue, Feb 12, 2019 at 1:36 PM A. Wilcox <awilfox@adelielinux.org> wrote:
>
> On 02/12/19 11:05, David Edelsohn wrote:
> > I presume that someone is trying to use GCCGo with Alpine Linux.  I
> > believe that Golang works with Alpine Linux.
> >
> > Thanks, David
>
>
> We're still here.  Adélie.  The distro that does big endian and ELFv2.
> The distro that *cares* about *quality*, not just hack-and-slashing.
>
> We can't use Golang because Golang doesn't work on half of our
> architectures - ppc32 will likely never be supported, and their ppc64
> support is unusable and written very poorly.  It assumes POWER8, not to
> mention the fact it likely has plenty of broken ELF ABI assumptions.
> (There's also very poor support for other architectures with musl, but I
> don't recall the specifics now.)
>
> I'm sure you're aware of all of this already.  In fact, it was probably
> your team that convinced Google that POWER7 is "old" when it is newer
> than Sandy Bridge:
>
> https://github.com/golang/go/issues/19074#issuecomment-436101375
>
> But I'm not sure if you work directly with Lynn or not, so I can't say
> that for certain.
>
> Anyway, GCC Go is definitely the best way forward for Adélie, which is
> why we're trying to make it work on ppc64.  It already works perfectly
> well on ppc32, x86*, and arm64.  And because it uses GCC's code
> generator, it won't be subject to ridiculously unportable ABI assumptions.

I didn't want to assume that this request was for support of Adelie.

IBM's product focus is systems running PPC64 Little Endian ELFv2
Linux.  The official ABI for PPC64 BE Linux is PPC64 ELFv1.  PPC64 BE
Linux with ELFv2 ABI is a niche within a niche.

If someone can offer patches to make it work, that's great.  The issue
with GCC Go and Musl Libc for PPC64 seems like a more general issue
than Adelie, but still a niche market.

- David