| Message ID | 20200201122746.1478003-5-laurent@vivier.eu |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: fix use of SIGRTMIN | expand |
> -----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? > + } > } > > 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 >
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
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); + } } 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,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
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(-)