On Wed, Mar 09, 2022 at 08:26:11AM +0100, Sören Tempel wrote:
> Ian Lance Taylor <i...@golang.org> wrote:
> > Have you tested this in 32-bit mode? It does not look correct based
> > on the glibc definitions. Looking at glibc it seems that it ought to
> > be
>
> As stated in the commit message, I have only tested this on Alpine Linux
> ppc64le (which uses musl libc). Unfortunately, I don't have access to a
> 32-bit PowerPC machine and hence haven't performed any tests with it.
>
> > reg.sigpc = ((ucontext_t*)(context))->uc_mcontext.uc_regs->gregs[32];
>
> While this should work with glibc, it doesn't work with musl. In order
> to support both (musl and glibc) on 32-bit PowerPC, we would have to do
> something along the lines of:
>
> #ifdef __PPC__
> #if defined(__PPC64__) /* ppc64 glibc & musl */
> ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.gp_regs[32]
> #elif defined(__GLIBC__) /* ppc32 glibc */
> reg.sigpc = ((ucontext_t*)(context))->uc_mcontext.uc_regs->gregs[32];
> #else /* ppc32 musl */
> ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.gregs[32];
> #endif /* __PPC64__ */
> #endif /* __PPC__ */
>
> In light of these observations, maybe using asm/ptrace.h and .regs (as
> proposed in the v1 patch) is the "better" (i.e. more readable) solution
> for now? I agree with Rich that using .regs is certainly a "code smell",
> but this gigantic ifdef block also looks pretty smelly to me. That being
> said, I can also send a v4 which uses this ifdef block.
I still prefer the #ifdef chain here, because it's at least using the
intended API. The problem is just that we can't have a matching API
because the only API glibc offers on ppc32 contradicts the POSIX
requirements.
I'm also not understanding how the .regs approach in the v1 patch was
ever supposed to work with musl anyway. The relevant line was:
ret.sigpc = ((ucontext_t*)(context))->uc_mcontext.regs->nip;
and musl has no uc_mcontext.regs member because uc_mcontext is
mcontext_t, not the glibc ppc32 pointer union thing. The only .regs in
musl's ppc32 signal.h/ucontext.h is in struct sigcontext, not anywhere
in ucontext_t.
Rich
