Re: [Qemu-devel] [PATCH 1/2] linux-user: Implement signals for openrisc

[Laurent Vivier <laurent@vivier.eu>]

Le 31/05/2018 à 06:18, Richard Henderson a écrit :
> All of the existing code was boilerplate from elsewhere,
> and would crash the guest upon the first signal.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/openrisc/target_syscall.h |  23 +--
>  linux-user/openrisc/signal.c         | 210 +++++++++++----------------
>  linux-user/signal.c                  |   2 +-
>  target/openrisc/cpu.c                |   1 +
>  4 files changed, 87 insertions(+), 149 deletions(-)
> 
...
> @@ -148,11 +102,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>                      target_siginfo_t *info,
>                      target_sigset_t *set, CPUOpenRISCState *env)
>  {
...
> @@ -160,47 +111,32 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>          goto give_sigsegv;
>      }
>  
> -    info_addr = frame_addr + offsetof(struct target_rt_sigframe, info);
> -    __put_user(info_addr, &frame->pinfo);
> -    uc_addr = frame_addr + offsetof(struct target_rt_sigframe, uc);
> -    __put_user(uc_addr, &frame->puc);
> +    tswap_siginfo(&frame->info, info);
>  
> -    if (ka->sa_flags & SA_SIGINFO) {
> -        tswap_siginfo(&frame->info, info);
> -    }

Why do you remove the "ka->sa_flags & SA_SIGINFO"?

> -    /*err |= __clear_user(&frame->uc, offsetof(ucontext_t, uc_mcontext));*/
>      __put_user(0, &frame->uc.tuc_flags);
>      __put_user(0, &frame->uc.tuc_link);
> +
>      target_save_altstack(&frame->uc.tuc_stack, env);
> -    setup_sigcontext(&frame->sc, env, set->sig[0]);
> +    setup_sigcontext(&frame->uc.tuc_mcontext, env);
>  
> -    /*err |= copy_to_user(frame->uc.tuc_sigmask, set, sizeof(*set));*/

other targets have something like:

    for (i = 0; i < TARGET_NSIG_WORDS; i++) {
        __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
    }

to match kernel

        err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));

Do we need it?

Thanks,
Laurent