Re: s390x regression - Re: [PATCH v5 21/26] linux-user/s390x: Implement setup_sigtramp

[Richard Henderson]

On 4/28/22 11:15, Ulrich Weigand wrote:
> Richard Henderson <richard.henderson@linaro.org> wrote:
>
> > Create and record the two signal trampolines.
> > Use them when the guest does not use SA_RESTORER.
>
> This patch caused a regression when running the wasmtime CI under qemu:
> https://github.com/bytecodealliance/wasmtime/pull/4076
>
> The problem is that this part:
>
> > diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
> > index 80f34086d7..676b948147 100644
> > --- a/linux-user/s390x/signal.c
> > +++ b/linux-user/s390x/signal.c
> > @@ -68,7 +68,6 @@ typedef struct {
> >      target_sigregs sregs;
> >      int signo;
> >      target_sigregs_ext sregs_ext;
> > -    uint16_t retcode;
> > } sigframe;
> >
> > #define TARGET_UC_VXRS 2
> > @@ -85,7 +84,6 @@ struct target_ucontext {
> >
> > typedef struct {
> >      uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
> > -    uint16_t retcode;
> >      struct target_siginfo info;
> >      struct target_ucontext uc;
> > } rt_sigframe;
>
> changes the layout of the signal stack frame that is visible from user
> space.  Some user space code, in particular the GCC unwinder
> (s390_fallback_frame_state in libgcc), relies on that layout and no
> longer works correctly if it is changed.
>
>
> Reverting just those two hunks above on top of QEMU 7.0 makes the
> wasmtime CI pass again.  (Actually, just the second hunk is enough; the
> first hunk is not visible since the removed variable is at the very top
> of the frame.)

Ah, quite right -- I had read the comment for sigframe,

        __u16 svc_insn;         /* Offset of svc_insn is NOT fixed! */


and incorrectly assumed that applied to rt_sigframe too.
So, yes, the second hunk should be reverted, with a comment that it is not used and not 
even initialized by the kernel.


r~