* [PATCH] powerpc64: use a type for mcontext_t regs field @ 2019-02-12 15:35 A. Wilcox 2019-02-12 16:18 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: A. Wilcox @ 2019-02-12 15:35 UTC (permalink / raw) To: musl; +Cc: 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 15:35 [PATCH] powerpc64: use a type for mcontext_t regs field A. Wilcox @ 2019-02-12 16:18 ` Rich Felker 2019-02-12 17:05 ` David Edelsohn 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2019-02-12 16:18 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 16:18 ` Rich Felker @ 2019-02-12 17:05 ` David Edelsohn 2019-02-12 17:23 ` Rich Felker 2019-02-12 18:36 ` A. Wilcox 0 siblings, 2 replies; 12+ messages in thread From: David Edelsohn @ 2019-02-12 17:05 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 17:05 ` David Edelsohn @ 2019-02-12 17:23 ` Rich Felker 2019-02-12 18:17 ` David Edelsohn 2019-02-12 18:36 ` A. Wilcox 1 sibling, 1 reply; 12+ messages in thread From: Rich Felker @ 2019-02-12 17:23 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 17:23 ` Rich Felker @ 2019-02-12 18:17 ` David Edelsohn 2019-02-12 18:21 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: David Edelsohn @ 2019-02-12 18:17 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:17 ` David Edelsohn @ 2019-02-12 18:21 ` Rich Felker 2019-02-12 18:24 ` David Edelsohn 0 siblings, 1 reply; 12+ messages in thread From: Rich Felker @ 2019-02-12 18:21 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:21 ` Rich Felker @ 2019-02-12 18:24 ` David Edelsohn 2019-02-12 18:32 ` Rich Felker 0 siblings, 1 reply; 12+ messages in thread From: David Edelsohn @ 2019-02-12 18:24 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:24 ` David Edelsohn @ 2019-02-12 18:32 ` Rich Felker 0 siblings, 0 replies; 12+ messages in thread From: Rich Felker @ 2019-02-12 18:32 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 17:05 ` David Edelsohn 2019-02-12 17:23 ` Rich Felker @ 2019-02-12 18:36 ` A. Wilcox 2019-02-12 18:42 ` James Larrowe 2019-02-12 18:48 ` David Edelsohn 1 sibling, 2 replies; 12+ messages in thread From: A. Wilcox @ 2019-02-12 18:36 UTC (permalink / raw) To: musl [-- 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 --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:36 ` A. Wilcox @ 2019-02-12 18:42 ` James Larrowe 2019-02-12 18:53 ` A. Wilcox 2019-02-12 18:48 ` David Edelsohn 1 sibling, 1 reply; 12+ messages in thread From: James Larrowe @ 2019-02-12 18:42 UTC (permalink / raw) To: musl [-- 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 --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:42 ` James Larrowe @ 2019-02-12 18:53 ` A. Wilcox 0 siblings, 0 replies; 12+ messages in thread From: A. Wilcox @ 2019-02-12 18:53 UTC (permalink / raw) To: musl [-- 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 --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] powerpc64: use a type for mcontext_t regs field 2019-02-12 18:36 ` A. Wilcox 2019-02-12 18:42 ` James Larrowe @ 2019-02-12 18:48 ` David Edelsohn 1 sibling, 0 replies; 12+ messages in thread From: David Edelsohn @ 2019-02-12 18:48 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-12 18:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-12 15:35 [PATCH] powerpc64: use a type for mcontext_t regs field A. Wilcox 2019-02-12 16:18 ` Rich Felker 2019-02-12 17:05 ` David Edelsohn 2019-02-12 17:23 ` Rich Felker 2019-02-12 18:17 ` David Edelsohn 2019-02-12 18:21 ` Rich Felker 2019-02-12 18:24 ` David Edelsohn 2019-02-12 18:32 ` Rich Felker 2019-02-12 18:36 ` A. Wilcox 2019-02-12 18:42 ` James Larrowe 2019-02-12 18:53 ` A. Wilcox 2019-02-12 18:48 ` David Edelsohn
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).