Re: [PATCH 28/30] bsd-user/signal.c: implement do_sigreturn

[Peter Maydell <peter.maydell@linaro.org>]

On Sun, 9 Jan 2022 at 17:00, Warner Losh <imp@bsdimp.com> wrote:
>
> Implements the meat of a sigreturn(2) system call via do_sigreturn, and
> helper reset_signal_mask. Fix the prototype of do_sigreturn in qemu.h
> and remove do_rt_sigreturn since it's linux only.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Kyle Evans <kevans@freebsd.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/qemu.h   |  3 +--
>  bsd-user/signal.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 011fdfebbaa..b8c64ca0e5b 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -219,14 +219,13 @@ extern int do_strace;
>  /* signal.c */
>  void process_pending_signals(CPUArchState *cpu_env);
>  void signal_init(void);
> -long do_sigreturn(CPUArchState *env);
> -long do_rt_sigreturn(CPUArchState *env);
>  void queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
>  abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
>  int target_to_host_signal(int sig);
>  int host_to_target_signal(int sig);
>  void host_to_target_sigset(target_sigset_t *d, const sigset_t *s);
>  void target_to_host_sigset(sigset_t *d, const target_sigset_t *s);
> +long do_sigreturn(CPUArchState *regs, abi_ulong addr);

Please always call CPUArchState* arguments 'env'.

>  void QEMU_NORETURN force_sig(int target_sig);
>  int qemu_sigorset(sigset_t *dest, const sigset_t *left, const sigset_t *right);
>
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index 1dd6dbb4ee1..d11f5eddd7e 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -640,6 +640,62 @@ give_sigsegv:
>      force_sig(TARGET_SIGSEGV);
>  }
>
> +static int reset_signal_mask(target_ucontext_t *ucontext)
> +{
> +    int i;
> +    sigset_t blocked;
> +    target_sigset_t target_set;
> +    TaskState *ts = (TaskState *)thread_cpu->opaque;
> +
> +    for (i = 0; i < TARGET_NSIG_WORDS; i++)
> +        if (__get_user(target_set.__bits[i],
> +                    &ucontext->uc_sigmask.__bits[i])) {
> +            return -TARGET_EFAULT;
> +        }
> +    target_to_host_sigset_internal(&blocked, &target_set);
> +    ts->signal_mask = blocked;
> +    sigprocmask(SIG_SETMASK, &ts->signal_mask, NULL);

do_sigreturn() itself shouldn't be setting the active signal
mask, at least if you follow the linux-user design. It just
sets the thread's signal_mask field in the TaskState by
calling set_sigmask(), and then on our way out in the
main cpu loop we'll call process_pending_signals() which
sets the real thread signal mask to that value. (This, together
with do_sigreturn() calling block_signals() before it starts
work, avoids some race conditions where a host signal is delivered
as soon as we unblock, I think.)

-- PMM