RE: [PATCH 4/4] linux-user: fix use of SIGRTMIN

[Taylor Simpson]

> -----Original Message-----
> From: Laurent Vivier <address@hidden>
> Sent: Saturday, February 1, 2020 6:28 AM
> To: address@hidden
> Cc: Josh Kunz <address@hidden>; address@hidden; Matus Kysel
> <address@hidden>; Aleksandar Markovic <aleksandar.markovic@rt-
> rk.com>; Marlies Ruck <address@hidden>; Laurent Vivier
> <address@hidden>; Peter Maydell <address@hidden>; Taylor
> Simpson <address@hidden>; Riku Voipio <address@hidden>
> Subject: [PATCH 4/4] linux-user: fix use of SIGRTMIN
>
> Some RT signals can be in use by glibc,
> it's why SIGRTMIN (34) is generally greater than __SIGRTMIN (32).
>
> So SIGRTMIN cannot be mapped to TARGET_SIGRTMIN.
>
> Instead of swapping only SIGRTMIN and SIGRTMAX, map all the range
> [TARGET_SIGRTMIN ... TARGET_SIGRTMAX - X] to
>       [__SIGRTMIN + X ... SIGRTMAX ]
> (SIGRTMIN is __SIGRTMIN + X).
>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
>  linux-user/signal.c     | 34 ++++++++++++++++++++++++++++------
>  linux-user/trace-events |  3 +++
>  2 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c index
> 3491f0a7ecb1..c4abacde49a0 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -501,15 +501,20 @@ static void signal_table_init(void)
>      int i, j;
>
>      /*
> -     * Nasty hack: Reverse SIGRTMIN and SIGRTMAX to avoid overlap with
> -     * host libpthread signals.  This assumes no one actually uses SIGRTMAX 
> :-/
> -     * To fix this properly we need to do manual signal delivery multiplexed
> -     * over a single host signal.
> +     * some RT signals can be in use by glibc,
> +     * it's why SIGRTMIN (34) is generally greater than __SIGRTMIN (32)
>       */
> -    host_to_target_signal_table[__SIGRTMIN] = __SIGRTMAX;
> -    host_to_target_signal_table[__SIGRTMAX] = __SIGRTMIN;
> +    for (i = SIGRTMIN; i <= SIGRTMAX; i++) {
> +        j = i - SIGRTMIN + TARGET_SIGRTMIN;
> +        if (j <= TARGET_NSIG) {
> +            host_to_target_signal_table[i] = j;
> +        }
> +    }
>
>      /* generate signal conversion tables */
> +    for (i = 1; i <= TARGET_NSIG; i++) {
> +        target_to_host_signal_table[i] = _NSIG; /* poison */
> +    }
>      for (i = 1; i < _NSIG; i++) {
>          if (host_to_target_signal_table[i] == 0) {
>              host_to_target_signal_table[i] = i; @@ -519,6 +524,15 @@ static 
> void
> signal_table_init(void)
>              target_to_host_signal_table[j] = i;
>          }
>      }
> +
> +    if (TRACE_SIGNAL_TABLE_INIT_BACKEND_DSTATE()) {
> +        for (i = 1, j = 0; i <= TARGET_NSIG; i++) {
> +            if (target_to_host_signal_table[i] == _NSIG) {
> +                j++;
> +            }
> +        }
> +        trace_signal_table_init(j);

It looks like j will have a count of the number of poison entries, but the 
message in trace_signal_table_init is "missing signal number".  Is that what 
you intend?

> +    }
>  }
>
>  void signal_init(void)
> @@ -817,6 +831,8 @@ int do_sigaction(int sig, const struct target_sigaction
> *act,
>      int host_sig;
>      int ret = 0;
>
> +    trace_signal_do_sigaction_guest(sig, TARGET_NSIG);

Shouldn't this be _NSIG, not TARGET_NSIG?

> +
>      if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig ==
> TARGET_SIGSTOP) {
>          return -TARGET_EINVAL;
>      }
> @@ -847,6 +863,12 @@ int do_sigaction(int sig, const struct target_sigaction
> *act,
>
>          /* we update the host linux signal state */
>          host_sig = target_to_host_signal(sig);
> +        trace_signal_do_sigaction_host(host_sig, TARGET_NSIG);
> +        if (host_sig > SIGRTMAX) {
> +            /* we don't have enough host signals to map all target signals */
> +            qemu_log_mask(LOG_UNIMP, "Unsupported target signal #%d\n",
> sig);
> +            return -TARGET_EINVAL;
> +        }
>          if (host_sig != SIGSEGV && host_sig != SIGBUS) {
>              sigfillset(&act1.sa_mask);
>              act1.sa_flags = SA_SIGINFO; diff --git a/linux-user/trace-events
> b/linux-user/trace-events index f6de1b8befc0..eb4b7701c400 100644
> --- a/linux-user/trace-events
> +++ b/linux-user/trace-events
> @@ -1,6 +1,9 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>
>  # signal.c
> +signal_table_init(int i) "missing signal number: %d"
> +signal_do_sigaction_guest(int sig, int max) "target signal %d (MAX %d)"
> +signal_do_sigaction_host(int sig, int max) "host signal %d (MAX %d)"
>  # */signal.c
>  user_setup_frame(void *env, uint64_t frame_addr) "env=%p
> frame_addr=0x%"PRIx64  user_setup_rt_frame(void *env, uint64_t
> frame_addr) "env=%p frame_addr=0x%"PRIx64
> --
> 2.24.1
>