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

[Laurent Vivier <laurent@vivier.eu>]

Le 04/02/2020 à 00:15, Taylor Simpson a écrit :
> 
> 
>> -----Original Message-----
>> From: Laurent Vivier <laurent@vivier.eu>
>> Sent: Saturday, February 1, 2020 6:28 AM
>> To: qemu-devel@nongnu.org
>> Cc: Josh Kunz <jkz@google.com>; milos.stojanovic@rt-rk.com; Matus Kysel
>> <mkysel@tachyum.com>; Aleksandar Markovic <aleksandar.markovic@rt-
>> rk.com>; Marlies Ruck <marlies.ruck@gmail.com>; Laurent Vivier
>> <laurent@vivier.eu>; Peter Maydell <peter.maydell@linaro.org>; Taylor
>> Simpson <tsimpson@quicinc.com>; Riku Voipio <riku.voipio@iki.fi>
>> 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 <laurent@vivier.eu>
>> ---
>>  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?

Yes, poison entries are the entries that cannot be used for the guest.
Perhaps it would be more correct as "Number of missing signals:"?

> 
>> +    }
>>  }
>>
>>  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?

No: do_sigaction() takes a number from the guest, so "sig" is a target
signal number, and this trace displays also the maximum value for the
target.

Thanks,
Laurent