* [PATCH v1 1/1] linux-user/signal: Decode waitid si_code
@ 2020-12-19 18:11 Alistair Francis
2021-01-15 23:01 ` Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alistair Francis @ 2020-12-19 18:11 UTC (permalink / raw)
To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23
When mapping the host waitid status to the target status we previously
just used decoding information in the status value. This doesn't follow
what the waitid documentation describes, which instead suggests using
the si_code value for the decoding. This results in the incorrect values
seen when calling waitid. This is especially apparent on RV32 where all
wait calls use waitid (see the bug case).
This patch uses the si_code value to map the waitid status.
Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
linux-user/signal.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 73de934c65..b6c9326521 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -305,6 +305,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
int sig = host_to_target_signal(info->si_signo);
int si_code = info->si_code;
int si_type;
+ int status = info->si_status;
tinfo->si_signo = sig;
tinfo->si_errno = 0;
tinfo->si_code = info->si_code;
@@ -349,8 +350,29 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
case TARGET_SIGCHLD:
tinfo->_sifields._sigchld._pid = info->si_pid;
tinfo->_sifields._sigchld._uid = info->si_uid;
- tinfo->_sifields._sigchld._status
- = host_to_target_waitstatus(info->si_status);
+
+ /*
+ * Map host to target signal numbers for the waitid family of
+ * syscalls. This is similar to the functionality in
+ * host_to_target_waitstatus() except we use the si_code to
+ * determine the operation.
+ */
+ switch (info->si_code) {
+ case CLD_KILLED:
+ case CLD_DUMPED:
+ tinfo->_sifields._sigchld._status =
+ host_to_target_signal(WTERMSIG(status)) |
+ (status & ~0x7f);
+ break;
+ case CLD_STOPPED:
+ tinfo->_sifields._sigchld._status =
+ (host_to_target_signal(WSTOPSIG(status)) << 8) |
+ (status & 0xff);
+ break;
+ default:
+ tinfo->_sifields._sigchld._status = status;
+ }
+
tinfo->_sifields._sigchld._utime = info->si_utime;
tinfo->_sifields._sigchld._stime = info->si_stime;
si_type = QEMU_SI_CHLD;
--
2.29.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v1 1/1] linux-user/signal: Decode waitid si_code
2020-12-19 18:11 [PATCH v1 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
@ 2021-01-15 23:01 ` Alistair Francis
2021-01-16 18:57 ` Andreas K. Hüttel
2021-01-18 14:36 ` Laurent Vivier
2 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2021-01-15 23:01 UTC (permalink / raw)
To: Alistair Francis
Cc: Palmer Dabbelt, Bin Meng, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Sat, Dec 19, 2020 at 10:11 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> When mapping the host waitid status to the target status we previously
> just used decoding information in the status value. This doesn't follow
> what the waitid documentation describes, which instead suggests using
> the si_code value for the decoding. This results in the incorrect values
> seen when calling waitid. This is especially apparent on RV32 where all
> wait calls use waitid (see the bug case).
>
> This patch uses the si_code value to map the waitid status.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Ping!
Alistair
> ---
> linux-user/signal.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..b6c9326521 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -305,6 +305,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> int sig = host_to_target_signal(info->si_signo);
> int si_code = info->si_code;
> int si_type;
> + int status = info->si_status;
> tinfo->si_signo = sig;
> tinfo->si_errno = 0;
> tinfo->si_code = info->si_code;
> @@ -349,8 +350,29 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> +
> + /*
> + * Map host to target signal numbers for the waitid family of
> + * syscalls. This is similar to the functionality in
> + * host_to_target_waitstatus() except we use the si_code to
> + * determine the operation.
> + */
> + switch (info->si_code) {
> + case CLD_KILLED:
> + case CLD_DUMPED:
> + tinfo->_sifields._sigchld._status =
> + host_to_target_signal(WTERMSIG(status)) |
> + (status & ~0x7f);
> + break;
> + case CLD_STOPPED:
> + tinfo->_sifields._sigchld._status =
> + (host_to_target_signal(WSTOPSIG(status)) << 8) |
> + (status & 0xff);
> + break;
> + default:
> + tinfo->_sifields._sigchld._status = status;
> + }
> +
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1 1/1] linux-user/signal: Decode waitid si_code
2020-12-19 18:11 [PATCH v1 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-15 23:01 ` Alistair Francis
@ 2021-01-16 18:57 ` Andreas K. Hüttel
2021-01-18 14:36 ` Laurent Vivier
2 siblings, 0 replies; 5+ messages in thread
From: Andreas K. Hüttel @ 2021-01-16 18:57 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3000 bytes --]
Am Samstag, 19. Dezember 2020, 20:11:13 EET schrieb Alistair Francis:
> When mapping the host waitid status to the target status we previously
> just used decoding information in the status value. This doesn't follow
> what the waitid documentation describes, which instead suggests using
> the si_code value for the decoding. This results in the incorrect values
> seen when calling waitid. This is especially apparent on RV32 where all
> wait calls use waitid (see the bug case).
>
> This patch uses the si_code value to map the waitid status.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Tested-by: Andreas K. Hüttel <dilfridge@gentoo.org>
> ---
> linux-user/signal.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..b6c9326521 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -305,6 +305,7 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo, int sig =
> host_to_target_signal(info->si_signo);
> int si_code = info->si_code;
> int si_type;
> + int status = info->si_status;
> tinfo->si_signo = sig;
> tinfo->si_errno = 0;
> tinfo->si_code = info->si_code;
> @@ -349,8 +350,29 @@ static inline void
> host_to_target_siginfo_noswap(target_siginfo_t *tinfo, case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> +
> + /*
> + * Map host to target signal numbers for the waitid family of
> + * syscalls. This is similar to the functionality in
> + * host_to_target_waitstatus() except we use the si_code to
> + * determine the operation.
> + */
> + switch (info->si_code) {
> + case CLD_KILLED:
> + case CLD_DUMPED:
> + tinfo->_sifields._sigchld._status =
> + host_to_target_signal(WTERMSIG(status)) |
> + (status & ~0x7f);
> + break;
> + case CLD_STOPPED:
> + tinfo->_sifields._sigchld._status =
> + (host_to_target_signal(WSTOPSIG(status)) << 8) |
> + (status & 0xff);
> + break;
> + default:
> + tinfo->_sifields._sigchld._status = status;
> + }
> +
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, qa, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1 1/1] linux-user/signal: Decode waitid si_code
2020-12-19 18:11 [PATCH v1 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-15 23:01 ` Alistair Francis
2021-01-16 18:57 ` Andreas K. Hüttel
@ 2021-01-18 14:36 ` Laurent Vivier
2021-01-19 17:34 ` Alistair Francis
2 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2021-01-18 14:36 UTC (permalink / raw)
To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer
Le 19/12/2020 à 19:11, Alistair Francis a écrit :
> When mapping the host waitid status to the target status we previously
> just used decoding information in the status value. This doesn't follow
> what the waitid documentation describes, which instead suggests using
> the si_code value for the decoding. This results in the incorrect values
> seen when calling waitid. This is especially apparent on RV32 where all
> wait calls use waitid (see the bug case).
>
> This patch uses the si_code value to map the waitid status.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> linux-user/signal.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 73de934c65..b6c9326521 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -305,6 +305,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> int sig = host_to_target_signal(info->si_signo);
> int si_code = info->si_code;
> int si_type;
> + int status = info->si_status;
> tinfo->si_signo = sig;
> tinfo->si_errno = 0;
> tinfo->si_code = info->si_code;
> @@ -349,8 +350,29 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> case TARGET_SIGCHLD:
> tinfo->_sifields._sigchld._pid = info->si_pid;
> tinfo->_sifields._sigchld._uid = info->si_uid;
> - tinfo->_sifields._sigchld._status
> - = host_to_target_waitstatus(info->si_status);
> +
> + /*
> + * Map host to target signal numbers for the waitid family of
> + * syscalls. This is similar to the functionality in
> + * host_to_target_waitstatus() except we use the si_code to
> + * determine the operation.
> + */
> + switch (info->si_code) {
> + case CLD_KILLED:
> + case CLD_DUMPED:
> + tinfo->_sifields._sigchld._status =
> + host_to_target_signal(WTERMSIG(status)) |
> + (status & ~0x7f);
> + break;
> + case CLD_STOPPED:
> + tinfo->_sifields._sigchld._status =
> + (host_to_target_signal(WSTOPSIG(status)) << 8) |
> + (status & 0xff);
> + break;
> + default:
I guess the the operation is not encoded in the status coming from the host as we need to use the
si_code to decode the status, so why do we need to encode it in the status we send to the guest?
Can it be only "tinfo->_sifields._sigchld._status = status" for all the cases?
Thanks,
Laurent
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1 1/1] linux-user/signal: Decode waitid si_code
2021-01-18 14:36 ` Laurent Vivier
@ 2021-01-19 17:34 ` Alistair Francis
0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2021-01-19 17:34 UTC (permalink / raw)
To: Laurent Vivier
Cc: open list:RISC-V, Palmer Dabbelt, Bin Meng, Alistair Francis,
qemu-devel@nongnu.org Developers
On Mon, Jan 18, 2021 at 6:36 AM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 19/12/2020 à 19:11, Alistair Francis a écrit :
> > When mapping the host waitid status to the target status we previously
> > just used decoding information in the status value. This doesn't follow
> > what the waitid documentation describes, which instead suggests using
> > the si_code value for the decoding. This results in the incorrect values
> > seen when calling waitid. This is especially apparent on RV32 where all
> > wait calls use waitid (see the bug case).
> >
> > This patch uses the si_code value to map the waitid status.
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > linux-user/signal.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-user/signal.c b/linux-user/signal.c
> > index 73de934c65..b6c9326521 100644
> > --- a/linux-user/signal.c
> > +++ b/linux-user/signal.c
> > @@ -305,6 +305,7 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> > int sig = host_to_target_signal(info->si_signo);
> > int si_code = info->si_code;
> > int si_type;
> > + int status = info->si_status;
> > tinfo->si_signo = sig;
> > tinfo->si_errno = 0;
> > tinfo->si_code = info->si_code;
> > @@ -349,8 +350,29 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
> > case TARGET_SIGCHLD:
> > tinfo->_sifields._sigchld._pid = info->si_pid;
> > tinfo->_sifields._sigchld._uid = info->si_uid;
> > - tinfo->_sifields._sigchld._status
> > - = host_to_target_waitstatus(info->si_status);
> > +
> > + /*
> > + * Map host to target signal numbers for the waitid family of
> > + * syscalls. This is similar to the functionality in
> > + * host_to_target_waitstatus() except we use the si_code to
> > + * determine the operation.
> > + */
> > + switch (info->si_code) {
> > + case CLD_KILLED:
> > + case CLD_DUMPED:
> > + tinfo->_sifields._sigchld._status =
> > + host_to_target_signal(WTERMSIG(status)) |
> > + (status & ~0x7f);
> > + break;
> > + case CLD_STOPPED:
> > + tinfo->_sifields._sigchld._status =
> > + (host_to_target_signal(WSTOPSIG(status)) << 8) |
> > + (status & 0xff);
> > + break;
> > + default:
>
> I guess the the operation is not encoded in the status coming from the host as we need to use the
> si_code to decode the status, so why do we need to encode it in the status we send to the guest?
>
> Can it be only "tinfo->_sifields._sigchld._status = status" for all the cases?
That also works, I'll send a v2.
Alistair
>
> Thanks,
> Laurent
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-19 18:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19 18:11 [PATCH v1 1/1] linux-user/signal: Decode waitid si_code Alistair Francis
2021-01-15 23:01 ` Alistair Francis
2021-01-16 18:57 ` Andreas K. Hüttel
2021-01-18 14:36 ` Laurent Vivier
2021-01-19 17:34 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).