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

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

[Laurent Vivier]

This series fixes the problem of the first real-time signals already
in use by the glibc that are not available for the target glibc.

Instead of reverting the first and last real-time signals we rely on
the value provided by the glibc (SIGRTMIN) to know the first available
signal and we map all the signals from this value to SIGRTMAX on top
of TARGET_SIGRTMIN. So the consequence is we have less available signals
in the target (generally 2) but all seems fine as at least 30 signals are
still available.

This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64
fedora 31. We can avoid the failure in this case allowing the unsupported
signals when we don't provide the "act" parameters to sigaction, only the
"oldact" one. I have also run the LTP suite with several target and debian
based distros.

v2: tested with golang 1.12.10 linux/arm64, eoan)
    Ignore unsupported signals rather than returning an error
    replace i, j by target_sig, host_sig

Laurent Vivier (4):
  linux-user: add missing TARGET_SIGRTMIN for hppa
  linux-user: cleanup signal.c
  linux-user: fix TARGET_NSIG and _NSIG uses
  linux-user: fix use of SIGRTMIN

 linux-user/hppa/target_signal.h |   1 +
 linux-user/signal.c             | 117 +++++++++++++++++++++++---------
 linux-user/trace-events         |   3 +
 3 files changed, 89 insertions(+), 32 deletions(-)

-- 
2.24.1

[PATCH v2 1/4] linux-user: add missing TARGET_SIGRTMIN for hppa

[Laurent Vivier]

This signal is defined for all other targets and we will need it later

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/hppa/target_signal.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
index ba159ff8d006..c2a0102ed73d 100644
--- a/linux-user/hppa/target_signal.h
+++ b/linux-user/hppa/target_signal.h
@@ -34,6 +34,7 @@
 #define TARGET_SIGURG          29
 #define TARGET_SIGXFSZ         30
 #define TARGET_SIGSYS          31
+#define TARGET_SIGRTMIN        32
 
 #define TARGET_SIG_BLOCK       0
 #define TARGET_SIG_UNBLOCK     1
-- 
2.24.1

Re: [PATCH v2 1/4] linux-user: add missing TARGET_SIGRTMIN for hppa

[Peter Maydell]

On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>
> This signal is defined for all other targets and we will need it later
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/hppa/target_signal.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
> index ba159ff8d006..c2a0102ed73d 100644
> --- a/linux-user/hppa/target_signal.h
> +++ b/linux-user/hppa/target_signal.h
> @@ -34,6 +34,7 @@
>  #define TARGET_SIGURG          29
>  #define TARGET_SIGXFSZ         30
>  #define TARGET_SIGSYS          31
> +#define TARGET_SIGRTMIN        32
>
>  #define TARGET_SIG_BLOCK       0
>  #define TARGET_SIG_UNBLOCK     1
> --
> 2.24.1

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v2 1/4] linux-user: add missing TARGET_SIGRTMIN for hppa

[Peter Maydell]

On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>
> This signal is defined for all other targets and we will need it later
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/hppa/target_signal.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/hppa/target_signal.h b/linux-user/hppa/target_signal.h
> index ba159ff8d006..c2a0102ed73d 100644
> --- a/linux-user/hppa/target_signal.h
> +++ b/linux-user/hppa/target_signal.h
> @@ -34,6 +34,7 @@
>  #define TARGET_SIGURG          29
>  #define TARGET_SIGXFSZ         30
>  #define TARGET_SIGSYS          31
> +#define TARGET_SIGRTMIN        32
>
>  #define TARGET_SIG_BLOCK       0
>  #define TARGET_SIG_UNBLOCK     1

I've just discovered that this was actually an ABI change
in the hppa kernel (at kernel version 3.17, kernel commit
1f25df2eff5b25f52c139d). Before that SIGRTMIN was 37...
All our other HPPA TARGET_SIG* values are for the updated
ABI following that commit, so using 32 for SIGRTMIN is
the right thing for us.

thanks
-- PMM

[PATCH v2 2/4] linux-user: cleanup signal.c

[Laurent Vivier]

No functional changes. Prepare the field for future fixes.

Remove memset(.., 0, ...) that is useless on a static array

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    v2: replace i, j by target_sig, host_sig

 linux-user/signal.c | 48 ++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 5ca6d62b15d3..246315571c09 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -66,12 +66,6 @@ static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGPWR] = TARGET_SIGPWR,
     [SIGSYS] = TARGET_SIGSYS,
     /* next signals stay the same */
-    /* 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.  */
-    [__SIGRTMIN] = __SIGRTMAX,
-    [__SIGRTMAX] = __SIGRTMIN,
 };
 static uint8_t target_to_host_signal_table[_NSIG];
 
@@ -480,31 +474,45 @@ static int core_dump_signal(int sig)
     }
 }
 
+static void signal_table_init(void)
+{
+    int host_sig, target_sig;
+
+    /*
+     * 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.
+     */
+    host_to_target_signal_table[__SIGRTMIN] = __SIGRTMAX;
+    host_to_target_signal_table[__SIGRTMAX] = __SIGRTMIN;
+
+    /* generate signal conversion tables */
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        if (host_to_target_signal_table[host_sig] == 0) {
+            host_to_target_signal_table[host_sig] = host_sig;
+        }
+    }
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        target_sig = host_to_target_signal_table[host_sig];
+        target_to_host_signal_table[target_sig] = host_sig;
+    }
+}
+
 void signal_init(void)
 {
     TaskState *ts = (TaskState *)thread_cpu->opaque;
     struct sigaction act;
     struct sigaction oact;
-    int i, j;
+    int i;
     int host_sig;
 
-    /* generate signal conversion tables */
-    for(i = 1; i < _NSIG; i++) {
-        if (host_to_target_signal_table[i] == 0)
-            host_to_target_signal_table[i] = i;
-    }
-    for(i = 1; i < _NSIG; i++) {
-        j = host_to_target_signal_table[i];
-        target_to_host_signal_table[j] = i;
-    }
+    /* initialize signal conversion tables */
+    signal_table_init();
 
     /* Set the signal mask from the host mask. */
     sigprocmask(0, 0, &ts->signal_mask);
 
-    /* set all host signal handlers. ALL signals are blocked during
-       the handlers to serialize them. */
-    memset(sigact_table, 0, sizeof(sigact_table));
-
     sigfillset(&act.sa_mask);
     act.sa_flags = SA_SIGINFO;
     act.sa_sigaction = host_signal_handler;
-- 
2.24.1

Re: [PATCH v2 2/4] linux-user: cleanup signal.c

[Philippe Mathieu-Daudé]

On 2/4/20 6:10 PM, Laurent Vivier wrote:
> No functional changes. Prepare the field for future fixes.
> 
> Remove memset(.., 0, ...) that is useless on a static array
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 
> Notes:
>      v2: replace i, j by target_sig, host_sig
> 
>   linux-user/signal.c | 48 ++++++++++++++++++++++++++-------------------
>   1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 5ca6d62b15d3..246315571c09 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -66,12 +66,6 @@ static uint8_t host_to_target_signal_table[_NSIG] = {
>       [SIGPWR] = TARGET_SIGPWR,
>       [SIGSYS] = TARGET_SIGSYS,
>       /* next signals stay the same */
> -    /* 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.  */
> -    [__SIGRTMIN] = __SIGRTMAX,
> -    [__SIGRTMAX] = __SIGRTMIN,
>   };
>   static uint8_t target_to_host_signal_table[_NSIG];
>   
> @@ -480,31 +474,45 @@ static int core_dump_signal(int sig)
>       }
>   }
>   
> +static void signal_table_init(void)
> +{
> +    int host_sig, target_sig;
> +
> +    /*
> +     * 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.
> +     */
> +    host_to_target_signal_table[__SIGRTMIN] = __SIGRTMAX;
> +    host_to_target_signal_table[__SIGRTMAX] = __SIGRTMIN;
> +
> +    /* generate signal conversion tables */
> +    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
> +        if (host_to_target_signal_table[host_sig] == 0) {
> +            host_to_target_signal_table[host_sig] = host_sig;
> +        }
> +    }
> +    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
> +        target_sig = host_to_target_signal_table[host_sig];
> +        target_to_host_signal_table[target_sig] = host_sig;
> +    }
> +}
> +
>   void signal_init(void)
>   {
>       TaskState *ts = (TaskState *)thread_cpu->opaque;
>       struct sigaction act;
>       struct sigaction oact;
> -    int i, j;
> +    int i;
>       int host_sig;
>   
> -    /* generate signal conversion tables */
> -    for(i = 1; i < _NSIG; i++) {
> -        if (host_to_target_signal_table[i] == 0)
> -            host_to_target_signal_table[i] = i;
> -    }
> -    for(i = 1; i < _NSIG; i++) {
> -        j = host_to_target_signal_table[i];
> -        target_to_host_signal_table[j] = i;
> -    }
> +    /* initialize signal conversion tables */
> +    signal_table_init();
>   
>       /* Set the signal mask from the host mask. */
>       sigprocmask(0, 0, &ts->signal_mask);
>   
> -    /* set all host signal handlers. ALL signals are blocked during
> -       the handlers to serialize them. */
> -    memset(sigact_table, 0, sizeof(sigact_table));
> -
>       sigfillset(&act.sa_mask);
>       act.sa_flags = SA_SIGINFO;
>       act.sa_sigaction = host_signal_handler;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 2/4] linux-user: cleanup signal.c

[Peter Maydell]

On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>
> No functional changes. Prepare the field for future fixes.
>
> Remove memset(.., 0, ...) that is useless on a static array
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>
> Notes:
>     v2: replace i, j by target_sig, host_sig
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v2 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Laurent Vivier]

Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.

System includes define _NSIG to SIGRTMAX + 1, but
QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.

Fix all the checks involving the signal range.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    v2: replace i, j by target_sig, host_sig

 linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 246315571c09..c1e664f97a7c 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
 static void host_signal_handler(int host_signum, siginfo_t *info,
                                 void *puc);
 
+
+/*
+ * System includes define _NSIG as SIGRTMAX + 1,
+ * but qemu (like the kernel) defines TARGET_NSIG as TARGET_SIGRTMAX
+ * and the first signal is SIGHUP defined as 1
+ * Signal number 0 is reserved for use as kill(pid, 0), to test whether
+ * a process exists without sending it a signal.
+ */
+QEMU_BUILD_BUG_ON(__SIGRTMAX + 1 != _NSIG);
 static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGHUP] = TARGET_SIGHUP,
     [SIGINT] = TARGET_SIGINT,
@@ -67,19 +76,24 @@ static uint8_t host_to_target_signal_table[_NSIG] = {
     [SIGSYS] = TARGET_SIGSYS,
     /* next signals stay the same */
 };
-static uint8_t target_to_host_signal_table[_NSIG];
 
+static uint8_t target_to_host_signal_table[TARGET_NSIG + 1];
+
+/* valid sig is between 1 and _NSIG - 1 */
 int host_to_target_signal(int sig)
 {
-    if (sig < 0 || sig >= _NSIG)
+    if (sig < 1 || sig >= _NSIG) {
         return sig;
+    }
     return host_to_target_signal_table[sig];
 }
 
+/* valid sig is between 1 and TARGET_NSIG */
 int target_to_host_signal(int sig)
 {
-    if (sig < 0 || sig >= _NSIG)
+    if (sig < 1 || sig > TARGET_NSIG) {
         return sig;
+    }
     return target_to_host_signal_table[sig];
 }
 
@@ -100,11 +114,15 @@ static inline int target_sigismember(const target_sigset_t *set, int signum)
 void host_to_target_sigset_internal(target_sigset_t *d,
                                     const sigset_t *s)
 {
-    int i;
+    int host_sig, target_sig;
     target_sigemptyset(d);
-    for (i = 1; i <= TARGET_NSIG; i++) {
-        if (sigismember(s, i)) {
-            target_sigaddset(d, host_to_target_signal(i));
+    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
+        target_sig = host_to_target_signal(host_sig);
+        if (target_sig < 1 || target_sig > TARGET_NSIG) {
+            continue;
+        }
+        if (sigismember(s, host_sig)) {
+            target_sigaddset(d, target_sig);
         }
     }
 }
@@ -122,11 +140,15 @@ void host_to_target_sigset(target_sigset_t *d, const sigset_t *s)
 void target_to_host_sigset_internal(sigset_t *d,
                                     const target_sigset_t *s)
 {
-    int i;
+    int host_sig, target_sig;
     sigemptyset(d);
-    for (i = 1; i <= TARGET_NSIG; i++) {
-        if (target_sigismember(s, i)) {
-            sigaddset(d, target_to_host_signal(i));
+    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
+        host_sig = target_to_host_signal(target_sig);
+        if (host_sig < 1 || host_sig >= _NSIG) {
+            continue;
+        }
+        if (target_sigismember(s, target_sig)) {
+            sigaddset(d, host_sig);
         }
     }
 }
@@ -492,10 +514,10 @@ static void signal_table_init(void)
         if (host_to_target_signal_table[host_sig] == 0) {
             host_to_target_signal_table[host_sig] = host_sig;
         }
-    }
-    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
         target_sig = host_to_target_signal_table[host_sig];
-        target_to_host_signal_table[target_sig] = host_sig;
+        if (target_sig <= TARGET_NSIG) {
+            target_to_host_signal_table[target_sig] = host_sig;
+        }
     }
 }
 
@@ -518,7 +540,7 @@ void signal_init(void)
     act.sa_sigaction = host_signal_handler;
     for(i = 1; i <= TARGET_NSIG; i++) {
 #ifdef TARGET_GPROF
-        if (i == SIGPROF) {
+        if (i == TARGET_SIGPROF) {
             continue;
         }
 #endif
-- 
2.24.1

Re: [PATCH v2 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Peter Maydell]

On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.
>
> System includes define _NSIG to SIGRTMAX + 1, but
> QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.
>
> Fix all the checks involving the signal range.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>
> Notes:
>     v2: replace i, j by target_sig, host_sig
>
>  linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 37 insertions(+), 15 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 246315571c09..c1e664f97a7c 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];

Optional follow-on patch: make sigact_table[] also size
TARGET_NSIG + 1, for consistency with target_to_host_signal_table[],
and remove all the "- 1"s when we index into it.


> @@ -492,10 +514,10 @@ static void signal_table_init(void)
>          if (host_to_target_signal_table[host_sig] == 0) {
>              host_to_target_signal_table[host_sig] = host_sig;
>          }
> -    }
> -    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>          target_sig = host_to_target_signal_table[host_sig];
> -        target_to_host_signal_table[target_sig] = host_sig;
> +        if (target_sig <= TARGET_NSIG) {
> +            target_to_host_signal_table[target_sig] = host_sig;
> +        }

Why does this hunk apparently delete the for() line ?

Why do we need the if() -- surely there should never be any
entries in host_to_target_signal_table[] that aren't
valid target signal numbers ?

>      }
>  }
>
> @@ -518,7 +540,7 @@ void signal_init(void)
>      act.sa_sigaction = host_signal_handler;
>      for(i = 1; i <= TARGET_NSIG; i++) {
>  #ifdef TARGET_GPROF
> -        if (i == SIGPROF) {
> +        if (i == TARGET_SIGPROF) {
>              continue;
>          }
>  #endif
> --

thanks
-- PMM

Re: [PATCH v2 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Laurent Vivier]

Le 11/02/2020 à 17:47, Peter Maydell a écrit :
> On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.
>>
>> System includes define _NSIG to SIGRTMAX + 1, but
>> QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.
>>
>> Fix all the checks involving the signal range.
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>
>> Notes:
>>     v2: replace i, j by target_sig, host_sig
>>
>>  linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 246315571c09..c1e664f97a7c 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
> 
> Optional follow-on patch: make sigact_table[] also size
> TARGET_NSIG + 1, for consistency with target_to_host_signal_table[],
> and remove all the "- 1"s when we index into it.
> 

OK,

>> @@ -492,10 +514,10 @@ static void signal_table_init(void)
>>          if (host_to_target_signal_table[host_sig] == 0) {
>>              host_to_target_signal_table[host_sig] = host_sig;
>>          }
>> -    }
>> -    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>>          target_sig = host_to_target_signal_table[host_sig];
>> -        target_to_host_signal_table[target_sig] = host_sig;
>> +        if (target_sig <= TARGET_NSIG) {
>> +            target_to_host_signal_table[target_sig] = host_sig;
>> +        }
> 
> Why does this hunk apparently delete the for() line ?

It effectively deletes the for() line because I merge the two "for
(host_sig = 1; host_sig < _NSIG; host_sig++)" loops into one.

> Why do we need the if() -- surely there should never be any
> entries in host_to_target_signal_table[] that aren't
> valid target signal numbers ?
> 

we have above the "host_to_target_signal_table[host_sig] = host_sig;"
and host_sig can be greater than TARGET_NSIG.

Setting like this allows to ignore them later in the target as we can
compare them to TARGET_NSIG. This mapping 1:1 in the default case is the
original behaviour.

Thanks,
Laurent

Re: [PATCH v2 3/4] linux-user: fix TARGET_NSIG and _NSIG uses

[Peter Maydell]

On Tue, 11 Feb 2020 at 16:59, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 11/02/2020 à 17:47, Peter Maydell a écrit :
> > On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Valid signal numbers are between 1 (SIGHUP) and SIGRTMAX.
> >>
> >> System includes define _NSIG to SIGRTMAX + 1, but
> >> QEMU (like kernel) defines TARGET_NSIG to TARGET_SIGRTMAX.
> >>
> >> Fix all the checks involving the signal range.
> >>
> >> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> >> ---
> >>
> >> Notes:
> >>     v2: replace i, j by target_sig, host_sig
> >>
> >>  linux-user/signal.c | 52 ++++++++++++++++++++++++++++++++-------------
> >>  1 file changed, 37 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/linux-user/signal.c b/linux-user/signal.c
> >> index 246315571c09..c1e664f97a7c 100644
> >> --- a/linux-user/signal.c
> >> +++ b/linux-user/signal.c
> >> @@ -30,6 +30,15 @@ static struct target_sigaction sigact_table[TARGET_NSIG];
> >
> > Optional follow-on patch: make sigact_table[] also size
> > TARGET_NSIG + 1, for consistency with target_to_host_signal_table[],
> > and remove all the "- 1"s when we index into it.
> >
>
> OK,
>
> >> @@ -492,10 +514,10 @@ static void signal_table_init(void)
> >>          if (host_to_target_signal_table[host_sig] == 0) {
> >>              host_to_target_signal_table[host_sig] = host_sig;
> >>          }
> >> -    }
> >> -    for (host_sig = 1; host_sig < _NSIG; host_sig++) {
> >>          target_sig = host_to_target_signal_table[host_sig];
> >> -        target_to_host_signal_table[target_sig] = host_sig;
> >> +        if (target_sig <= TARGET_NSIG) {
> >> +            target_to_host_signal_table[target_sig] = host_sig;
> >> +        }
> >
> > Why does this hunk apparently delete the for() line ?
>
> It effectively deletes the for() line because I merge the two "for
> (host_sig = 1; host_sig < _NSIG; host_sig++)" loops into one.

Oh, I see, I missed the closing brace being deleted.

> > Why do we need the if() -- surely there should never be any
> > entries in host_to_target_signal_table[] that aren't
> > valid target signal numbers ?
> >
>
> we have above the "host_to_target_signal_table[host_sig] = host_sig;"
> and host_sig can be greater than TARGET_NSIG.
>
> Setting like this allows to ignore them later in the target as we can
> compare them to TARGET_NSIG. This mapping 1:1 in the default case is the
> original behaviour.

I guess so (I was sort of expecting us to do the filter on
"is this valid" when we filled the array, rather than having
to do it every time we used the entries, but this works).

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

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

[Laurent Vivier]

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>
---

Notes:
    v2: ignore error when target sig <= TARGET_NSIG but host sig > SIGRTMAX
        replace i, j by target_sig, host_sig
        update signal_table_init() trace message

 linux-user/signal.c     | 37 ++++++++++++++++++++++++++++++-------
 linux-user/trace-events |  3 +++
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index c1e664f97a7c..e7e5581a016f 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -498,18 +498,23 @@ static int core_dump_signal(int sig)
 
 static void signal_table_init(void)
 {
-    int host_sig, target_sig;
+    int host_sig, target_sig, count;
 
     /*
-     * 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 (host_sig = SIGRTMIN; host_sig <= SIGRTMAX; host_sig++) {
+        target_sig = host_sig - SIGRTMIN + TARGET_SIGRTMIN;
+        if (target_sig <= TARGET_NSIG) {
+            host_to_target_signal_table[host_sig] = target_sig;
+        }
+    }
 
     /* generate signal conversion tables */
+    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
+        target_to_host_signal_table[target_sig] = _NSIG; /* poison */
+    }
     for (host_sig = 1; host_sig < _NSIG; host_sig++) {
         if (host_to_target_signal_table[host_sig] == 0) {
             host_to_target_signal_table[host_sig] = host_sig;
@@ -519,6 +524,15 @@ static void signal_table_init(void)
             target_to_host_signal_table[target_sig] = host_sig;
         }
     }
+
+    if (TRACE_SIGNAL_TABLE_INIT_BACKEND_DSTATE()) {
+        for (target_sig = 1, count = 0; target_sig <= TARGET_NSIG; target_sig++) {
+            if (target_to_host_signal_table[target_sig] == _NSIG) {
+                count++;
+            }
+        }
+        trace_signal_table_init(count);
+    }
 }
 
 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);
+
     if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig == TARGET_SIGSTOP) {
         return -TARGET_EINVAL;
     }
@@ -847,6 +863,13 @@ 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, ignored\n",
+                          sig);
+            return 0;
+        }
         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..0296133daeb6 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) "number of unavailable signals: %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

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

[Taylor Simpson]

Reviewed-by: Taylor Simson <tsimpson@quicinc.com>

> -----Original Message-----
> From: Laurent Vivier <laurent@vivier.eu>
> Sent: Tuesday, February 4, 2020 11:11 AM
> To: qemu-devel@nongnu.org
> Cc: Peter Maydell <peter.maydell@linaro.org>; Josh Kunz
> <jkz@google.com>; Laurent Vivier <laurent@vivier.eu>; Aleksandar
> Markovic <aleksandar.markovic@rt-rk.com>; Matus Kysel
> <mkysel@tachyum.com>; Riku Voipio <riku.voipio@iki.fi>; Taylor Simpson
> <tsimpson@quicinc.com>; milos.stojanovic@rt-rk.com; Marlies Ruck
> <marlies.ruck@gmail.com>
> Subject: [PATCH v2 4/4] linux-user: fix use of SIGRTMIN
>
> -------------------------------------------------------------------------
> CAUTION: This email originated from outside of the organization.
> -------------------------------------------------------------------------
>
> 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>
> ---
>
> Notes:
>     v2: ignore error when target sig <= TARGET_NSIG but host sig > SIGRTMAX
>         replace i, j by target_sig, host_sig
>         update signal_table_init() trace message
>
>  linux-user/signal.c     | 37 ++++++++++++++++++++++++++++++-------
>  linux-user/trace-events |  3 +++
>  2 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index c1e664f97a7c..e7e5581a016f 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -498,18 +498,23 @@ static int core_dump_signal(int sig)
>
>  static void signal_table_init(void)
>  {
> -    int host_sig, target_sig;
> +    int host_sig, target_sig, count;
>
>      /*
> -     * 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 (host_sig = SIGRTMIN; host_sig <= SIGRTMAX; host_sig++) {
> +        target_sig = host_sig - SIGRTMIN + TARGET_SIGRTMIN;
> +        if (target_sig <= TARGET_NSIG) {
> +            host_to_target_signal_table[host_sig] = target_sig;
> +        }
> +    }
>
>      /* generate signal conversion tables */
> +    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
> +        target_to_host_signal_table[target_sig] = _NSIG; /* poison */
> +    }
>      for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>          if (host_to_target_signal_table[host_sig] == 0) {
>              host_to_target_signal_table[host_sig] = host_sig;
> @@ -519,6 +524,15 @@ static void signal_table_init(void)
>              target_to_host_signal_table[target_sig] = host_sig;
>          }
>      }
> +
> +    if (TRACE_SIGNAL_TABLE_INIT_BACKEND_DSTATE()) {
> +        for (target_sig = 1, count = 0; target_sig <= TARGET_NSIG; target_sig++)
> {
> +            if (target_to_host_signal_table[target_sig] == _NSIG) {
> +                count++;
> +            }
> +        }
> +        trace_signal_table_init(count);
> +    }
>  }
>
>  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);
> +
>      if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig ==
> TARGET_SIGSTOP) {
>          return -TARGET_EINVAL;
>      }
> @@ -847,6 +863,13 @@ 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,
> ignored\n",
> +                          sig);
> +            return 0;
> +        }
>          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..0296133daeb6 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) "number of unavailable signals: %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
>

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

[Peter Maydell]

On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>
> 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>
> ---

In general I think this is a good approach to trying to deal
with this long-standing issue in a pragmatic and not too
complicated way, so thanks for writing this patchset. I have
some fairly minor comments on the code below.

>
> Notes:
>     v2: ignore error when target sig <= TARGET_NSIG but host sig > SIGRTMAX
>         replace i, j by target_sig, host_sig
>         update signal_table_init() trace message
>
>  linux-user/signal.c     | 37 ++++++++++++++++++++++++++++++-------
>  linux-user/trace-events |  3 +++
>  2 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index c1e664f97a7c..e7e5581a016f 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -498,18 +498,23 @@ static int core_dump_signal(int sig)
>
>  static void signal_table_init(void)
>  {
> -    int host_sig, target_sig;
> +    int host_sig, target_sig, count;
>
>      /*
> -     * 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 (host_sig = SIGRTMIN; host_sig <= SIGRTMAX; host_sig++) {
> +        target_sig = host_sig - SIGRTMIN + TARGET_SIGRTMIN;
> +        if (target_sig <= TARGET_NSIG) {
> +            host_to_target_signal_table[host_sig] = target_sig;
> +        }
> +    }

So the effect of this is that we now support target signals
starting from TARGET_SIGRTMIN and going up until we run out
of host realtime signals that the host libc hasn't reserved ?
That seems reasonable, since glibc at least uses only the
lower 2 rt signals and probably nobody's using the upper ones.
But this would be a good place to have a comment explaining
the limitation (and that if it needed to be fixed we'd have
to multiplex guest signals onto a single host signal). You
could also mention that attempts to configure the "missing"
signals via sigaction will be silently ignored.

>      /* generate signal conversion tables */
> +    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
> +        target_to_host_signal_table[target_sig] = _NSIG; /* poison */
> +    }
>      for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>          if (host_to_target_signal_table[host_sig] == 0) {
>              host_to_target_signal_table[host_sig] = host_sig;
> @@ -519,6 +524,15 @@ static void signal_table_init(void)
>              target_to_host_signal_table[target_sig] = host_sig;
>          }
>      }
> +
> +    if (TRACE_SIGNAL_TABLE_INIT_BACKEND_DSTATE()) {

This isn't the right way to conditionalize expensive stuff
that's only used in trace events. You want to use
trace_event_get_state_backends() (see docs/devel/tracing.txt
for details).

> +        for (target_sig = 1, count = 0; target_sig <= TARGET_NSIG; target_sig++) {
> +            if (target_to_host_signal_table[target_sig] == _NSIG) {
> +                count++;
> +            }
> +        }
> +        trace_signal_table_init(count);
> +    }
>  }
>
>  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);
> +
>      if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig == TARGET_SIGSTOP) {
>          return -TARGET_EINVAL;
>      }
> @@ -847,6 +863,13 @@ 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, ignored\n",
> +                          sig);
> +            return 0;

We should have a comment here mentioning why we don't return
an error code here (and explicitly noting that the Go runtime
is the major one which we don't want to upset).

> +        }
>          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..0296133daeb6 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) "number of unavailable signals: %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

thanks
-- PMM

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

[Laurent Vivier]

Thank you Peter,

I will address you comments and send a new version of the series.

Laurent

Le 11/02/2020 à 18:05, Peter Maydell a écrit :
> On Tue, 4 Feb 2020 at 17:11, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> 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>
>> ---
> 
> In general I think this is a good approach to trying to deal
> with this long-standing issue in a pragmatic and not too
> complicated way, so thanks for writing this patchset. I have
> some fairly minor comments on the code below.
> 
>>
>> Notes:
>>     v2: ignore error when target sig <= TARGET_NSIG but host sig > SIGRTMAX
>>         replace i, j by target_sig, host_sig
>>         update signal_table_init() trace message
>>
>>  linux-user/signal.c     | 37 ++++++++++++++++++++++++++++++-------
>>  linux-user/trace-events |  3 +++
>>  2 files changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index c1e664f97a7c..e7e5581a016f 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -498,18 +498,23 @@ static int core_dump_signal(int sig)
>>
>>  static void signal_table_init(void)
>>  {
>> -    int host_sig, target_sig;
>> +    int host_sig, target_sig, count;
>>
>>      /*
>> -     * 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 (host_sig = SIGRTMIN; host_sig <= SIGRTMAX; host_sig++) {
>> +        target_sig = host_sig - SIGRTMIN + TARGET_SIGRTMIN;
>> +        if (target_sig <= TARGET_NSIG) {
>> +            host_to_target_signal_table[host_sig] = target_sig;
>> +        }
>> +    }
> 
> So the effect of this is that we now support target signals
> starting from TARGET_SIGRTMIN and going up until we run out
> of host realtime signals that the host libc hasn't reserved ?
> That seems reasonable, since glibc at least uses only the
> lower 2 rt signals and probably nobody's using the upper ones.
> But this would be a good place to have a comment explaining
> the limitation (and that if it needed to be fixed we'd have
> to multiplex guest signals onto a single host signal). You
> could also mention that attempts to configure the "missing"
> signals via sigaction will be silently ignored.
> 
>>      /* generate signal conversion tables */
>> +    for (target_sig = 1; target_sig <= TARGET_NSIG; target_sig++) {
>> +        target_to_host_signal_table[target_sig] = _NSIG; /* poison */
>> +    }
>>      for (host_sig = 1; host_sig < _NSIG; host_sig++) {
>>          if (host_to_target_signal_table[host_sig] == 0) {
>>              host_to_target_signal_table[host_sig] = host_sig;
>> @@ -519,6 +524,15 @@ static void signal_table_init(void)
>>              target_to_host_signal_table[target_sig] = host_sig;
>>          }
>>      }
>> +
>> +    if (TRACE_SIGNAL_TABLE_INIT_BACKEND_DSTATE()) {
> 
> This isn't the right way to conditionalize expensive stuff
> that's only used in trace events. You want to use
> trace_event_get_state_backends() (see docs/devel/tracing.txt
> for details).
> 
>> +        for (target_sig = 1, count = 0; target_sig <= TARGET_NSIG; target_sig++) {
>> +            if (target_to_host_signal_table[target_sig] == _NSIG) {
>> +                count++;
>> +            }
>> +        }
>> +        trace_signal_table_init(count);
>> +    }
>>  }
>>
>>  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);
>> +
>>      if (sig < 1 || sig > TARGET_NSIG || sig == TARGET_SIGKILL || sig == TARGET_SIGSTOP) {
>>          return -TARGET_EINVAL;
>>      }
>> @@ -847,6 +863,13 @@ 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, ignored\n",
>> +                          sig);
>> +            return 0;
> 
> We should have a comment here mentioning why we don't return
> an error code here (and explicitly noting that the Go runtime
> is the major one which we don't want to upset).
> 
>> +        }
>>          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..0296133daeb6 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) "number of unavailable signals: %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
> 
> thanks
> -- PMM
> 

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

[Laurent Vivier]

Peter,

do you have any comment on this way to fix the SIGRTMIN problem we have
now for years?

Thanks,
Laurent

Le 04/02/2020 à 18:10, Laurent Vivier a écrit :
> This series fixes the problem of the first real-time signals already
> in use by the glibc that are not available for the target glibc.
> 
> Instead of reverting the first and last real-time signals we rely on
> the value provided by the glibc (SIGRTMIN) to know the first available
> signal and we map all the signals from this value to SIGRTMAX on top
> of TARGET_SIGRTMIN. So the consequence is we have less available signals
> in the target (generally 2) but all seems fine as at least 30 signals are
> still available.
> 
> This has been tested with Go (golang 1.10.1 linux/arm64, bionic) on x86_64
> fedora 31. We can avoid the failure in this case allowing the unsupported
> signals when we don't provide the "act" parameters to sigaction, only the
> "oldact" one. I have also run the LTP suite with several target and debian
> based distros.
> 
> v2: tested with golang 1.12.10 linux/arm64, eoan)
>     Ignore unsupported signals rather than returning an error
>     replace i, j by target_sig, host_sig
> 
> Laurent Vivier (4):
>   linux-user: add missing TARGET_SIGRTMIN for hppa
>   linux-user: cleanup signal.c
>   linux-user: fix TARGET_NSIG and _NSIG uses
>   linux-user: fix use of SIGRTMIN
> 
>  linux-user/hppa/target_signal.h |   1 +
>  linux-user/signal.c             | 117 +++++++++++++++++++++++---------
>  linux-user/trace-events         |   3 +
>  3 files changed, 89 insertions(+), 32 deletions(-)
>