* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-10-23 19:59 [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t Matthias Schiffer
@ 2021-12-18 23:32 ` Matthias Schiffer
2022-01-02 23:59 ` Andreas K. Huettel
2021-12-19 15:55 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Matthias Schiffer @ 2021-12-18 23:32 UTC (permalink / raw)
To: Laurent Vivier; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 2820 bytes --]
On 23/10/2021 21:59, Matthias Schiffer wrote:
> When converting a siginfo_t from waitid(), the interpretation of si_status
> depends on the value of si_code: For CLD_EXITED, it is an exit code and
> should be copied verbatim. For other codes, it is a signal number
> (possibly with additional high bits from ptrace) that should be mapped.
>
> This code was previously changed in commit 1c3dfb506ea3
> ("linux-user/signal: Decode waitid si_code"), but the fix was
> incomplete.
ping
>
> Tested with the following test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/wait.h>
>
> int main() {
> pid_t pid = fork();
> if (pid == 0) {
> exit(12);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
>
> pid = fork();
> if (pid == 0) {
> raise(SIGUSR2);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
> }
>
> Output with an x86_64 host and mips64el target before 1c3dfb506ea3
> (incorrect: exit code 12 is translated like a signal):
>
> Code: 1, status: 17
> Code: 2, status: 17
>
> After 1c3dfb506ea3 (incorrect: signal number is not translated):
>
> Code: 1, status: 12
> Code: 2, status: 12
>
> With this patch:
>
> Code: 1, status: 12
> Code: 2, status: 17
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---
> linux-user/signal.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 14d8fdfde152..8e3af98ec0a7 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -403,7 +403,12 @@ 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 = info->si_status;
> + if (si_code == CLD_EXITED)
> + tinfo->_sifields._sigchld._status = info->si_status;
> + else
> + tinfo->_sifields._sigchld._status
> + = host_to_target_signal(info->si_status & 0x7f)
> + | (info->si_status & ~0x7f);
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-12-18 23:32 ` Matthias Schiffer
@ 2022-01-02 23:59 ` Andreas K. Huettel
0 siblings, 0 replies; 7+ messages in thread
From: Andreas K. Huettel @ 2022-01-02 23:59 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Matthias Schiffer, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 873 bytes --]
Am Sonntag, 19. Dezember 2021, 00:32:09 CET schrieb Matthias Schiffer:
> On 23/10/2021 21:59, Matthias Schiffer wrote:
> > When converting a siginfo_t from waitid(), the interpretation of si_status
> > depends on the value of si_code: For CLD_EXITED, it is an exit code and
> > should be copied verbatim. For other codes, it is a signal number
> > (possibly with additional high bits from ptrace) that should be mapped.
> >
> > This code was previously changed in commit 1c3dfb506ea3
> > ("linux-user/signal: Decode waitid si_code"), but the fix was
> > incomplete.
>
> ping
>
Sorry I can't say anything about this.
The hangs that I experience seem to be unrelated to the patch (no improvement, but also no worsening).
--
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: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-10-23 19:59 [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t Matthias Schiffer
2021-12-18 23:32 ` Matthias Schiffer
@ 2021-12-19 15:55 ` Laurent Vivier
2021-12-19 18:47 ` Andreas K. Huettel
2022-01-04 11:49 ` Laurent Vivier
2022-01-05 10:18 ` Laurent Vivier
3 siblings, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2021-12-19 15:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Andreas K. Hüttel, Alistair Francis, Matthias Schiffer
CC'ing Alistair and Andreas that were involved in original fix 1c3dfb506ea3 ("linux-user/signal:
Decode waitid si_code")
Thanks,
Laurent
Le 23/10/2021 à 21:59, Matthias Schiffer a écrit :
> When converting a siginfo_t from waitid(), the interpretation of si_status
> depends on the value of si_code: For CLD_EXITED, it is an exit code and
> should be copied verbatim. For other codes, it is a signal number
> (possibly with additional high bits from ptrace) that should be mapped.
>
> This code was previously changed in commit 1c3dfb506ea3
> ("linux-user/signal: Decode waitid si_code"), but the fix was
> incomplete.
>
> Tested with the following test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/wait.h>
>
> int main() {
> pid_t pid = fork();
> if (pid == 0) {
> exit(12);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
>
> pid = fork();
> if (pid == 0) {
> raise(SIGUSR2);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
> }
>
> Output with an x86_64 host and mips64el target before 1c3dfb506ea3
> (incorrect: exit code 12 is translated like a signal):
>
> Code: 1, status: 17
> Code: 2, status: 17
>
> After 1c3dfb506ea3 (incorrect: signal number is not translated):
>
> Code: 1, status: 12
> Code: 2, status: 12
>
> With this patch:
>
> Code: 1, status: 12
> Code: 2, status: 17
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---
> linux-user/signal.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 14d8fdfde152..8e3af98ec0a7 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -403,7 +403,12 @@ 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 = info->si_status;
> + if (si_code == CLD_EXITED)
> + tinfo->_sifields._sigchld._status = info->si_status;
> + else
> + tinfo->_sifields._sigchld._status
> + = host_to_target_signal(info->si_status & 0x7f)
> + | (info->si_status & ~0x7f);
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-12-19 15:55 ` Laurent Vivier
@ 2021-12-19 18:47 ` Andreas K. Huettel
0 siblings, 0 replies; 7+ messages in thread
From: Andreas K. Huettel @ 2021-12-19 18:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Matthias Schiffer, Alistair Francis, Laurent Vivier
[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]
Well, the original fix 1c3dfb506ea3 did clearly improve things for me, but it wasn't
complete yet. At some point I gave up on finding a minimal reproducer for my remaining
problems (futex-related hangs in a complex python+bash app).
So, this *may* be the missing piece.
Will test, but that takes a few days.
Andreas
Am Sonntag, 19. Dezember 2021, 16:55:16 CET schrieb Laurent Vivier:
> CC'ing Alistair and Andreas that were involved in original fix 1c3dfb506ea3 ("linux-user/signal:
> Decode waitid si_code")
>
> Thanks,
> Laurent
>
> Le 23/10/2021 à 21:59, Matthias Schiffer a écrit :
> > When converting a siginfo_t from waitid(), the interpretation of si_status
> > depends on the value of si_code: For CLD_EXITED, it is an exit code and
> > should be copied verbatim. For other codes, it is a signal number
> > (possibly with additional high bits from ptrace) that should be mapped.
> >
> > This code was previously changed in commit 1c3dfb506ea3
> > ("linux-user/signal: Decode waitid si_code"), but the fix was
> > incomplete.
> >
> > Tested with the following test program:
> >
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > #include <sys/wait.h>
> >
> > int main() {
> > pid_t pid = fork();
> > if (pid == 0) {
> > exit(12);
> > } else {
> > siginfo_t siginfo = {};
> > waitid(P_PID, pid, &siginfo, WEXITED);
> > printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> > }
> >
> > pid = fork();
> > if (pid == 0) {
> > raise(SIGUSR2);
> > } else {
> > siginfo_t siginfo = {};
> > waitid(P_PID, pid, &siginfo, WEXITED);
> > printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> > }
> > }
> >
> > Output with an x86_64 host and mips64el target before 1c3dfb506ea3
> > (incorrect: exit code 12 is translated like a signal):
> >
> > Code: 1, status: 17
> > Code: 2, status: 17
> >
> > After 1c3dfb506ea3 (incorrect: signal number is not translated):
> >
> > Code: 1, status: 12
> > Code: 2, status: 12
> >
> > With this patch:
> >
> > Code: 1, status: 12
> > Code: 2, status: 17
> >
> > Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> > ---
> > linux-user/signal.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/linux-user/signal.c b/linux-user/signal.c
> > index 14d8fdfde152..8e3af98ec0a7 100644
> > --- a/linux-user/signal.c
> > +++ b/linux-user/signal.c
> > @@ -403,7 +403,12 @@ 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 = info->si_status;
> > + if (si_code == CLD_EXITED)
> > + tinfo->_sifields._sigchld._status = info->si_status;
> > + else
> > + tinfo->_sifields._sigchld._status
> > + = host_to_target_signal(info->si_status & 0x7f)
> > + | (info->si_status & ~0x7f);
> > 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, toolchain, base-system, perl, libreoffice)
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-10-23 19:59 [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t Matthias Schiffer
2021-12-18 23:32 ` Matthias Schiffer
2021-12-19 15:55 ` Laurent Vivier
@ 2022-01-04 11:49 ` Laurent Vivier
2022-01-05 10:18 ` Laurent Vivier
3 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2022-01-04 11:49 UTC (permalink / raw)
To: Matthias Schiffer; +Cc: qemu-devel
Le 23/10/2021 à 21:59, Matthias Schiffer a écrit :
> When converting a siginfo_t from waitid(), the interpretation of si_status
> depends on the value of si_code: For CLD_EXITED, it is an exit code and
> should be copied verbatim. For other codes, it is a signal number
> (possibly with additional high bits from ptrace) that should be mapped.
>
> This code was previously changed in commit 1c3dfb506ea3
> ("linux-user/signal: Decode waitid si_code"), but the fix was
> incomplete.
>
> Tested with the following test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/wait.h>
>
> int main() {
> pid_t pid = fork();
> if (pid == 0) {
> exit(12);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
>
> pid = fork();
> if (pid == 0) {
> raise(SIGUSR2);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
> }
>
> Output with an x86_64 host and mips64el target before 1c3dfb506ea3
> (incorrect: exit code 12 is translated like a signal):
>
> Code: 1, status: 17
> Code: 2, status: 17
>
> After 1c3dfb506ea3 (incorrect: signal number is not translated):
>
> Code: 1, status: 12
> Code: 2, status: 12
>
> With this patch:
>
> Code: 1, status: 12
> Code: 2, status: 17
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---
> linux-user/signal.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 14d8fdfde152..8e3af98ec0a7 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -403,7 +403,12 @@ 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 = info->si_status;
> + if (si_code == CLD_EXITED)
> + tinfo->_sifields._sigchld._status = info->si_status;
> + else
> + tinfo->_sifields._sigchld._status
> + = host_to_target_signal(info->si_status & 0x7f)
> + | (info->si_status & ~0x7f);
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t
2021-10-23 19:59 [PATCH] linux-user/signal: Map exit signals in SIGCHLD siginfo_t Matthias Schiffer
` (2 preceding siblings ...)
2022-01-04 11:49 ` Laurent Vivier
@ 2022-01-05 10:18 ` Laurent Vivier
3 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2022-01-05 10:18 UTC (permalink / raw)
To: Matthias Schiffer; +Cc: qemu-devel
Le 23/10/2021 à 21:59, Matthias Schiffer a écrit :
> When converting a siginfo_t from waitid(), the interpretation of si_status
> depends on the value of si_code: For CLD_EXITED, it is an exit code and
> should be copied verbatim. For other codes, it is a signal number
> (possibly with additional high bits from ptrace) that should be mapped.
>
> This code was previously changed in commit 1c3dfb506ea3
> ("linux-user/signal: Decode waitid si_code"), but the fix was
> incomplete.
>
> Tested with the following test program:
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/wait.h>
>
> int main() {
> pid_t pid = fork();
> if (pid == 0) {
> exit(12);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
>
> pid = fork();
> if (pid == 0) {
> raise(SIGUSR2);
> } else {
> siginfo_t siginfo = {};
> waitid(P_PID, pid, &siginfo, WEXITED);
> printf("Code: %d, status: %d\n", (int)siginfo.si_code, (int)siginfo.si_status);
> }
> }
>
> Output with an x86_64 host and mips64el target before 1c3dfb506ea3
> (incorrect: exit code 12 is translated like a signal):
>
> Code: 1, status: 17
> Code: 2, status: 17
>
> After 1c3dfb506ea3 (incorrect: signal number is not translated):
>
> Code: 1, status: 12
> Code: 2, status: 12
>
> With this patch:
>
> Code: 1, status: 12
> Code: 2, status: 17
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---
> linux-user/signal.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 14d8fdfde152..8e3af98ec0a7 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -403,7 +403,12 @@ 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 = info->si_status;
> + if (si_code == CLD_EXITED)
> + tinfo->_sifields._sigchld._status = info->si_status;
> + else
> + tinfo->_sifields._sigchld._status
> + = host_to_target_signal(info->si_status & 0x7f)
> + | (info->si_status & ~0x7f);
> tinfo->_sifields._sigchld._utime = info->si_utime;
> tinfo->_sifields._sigchld._stime = info->si_stime;
> si_type = QEMU_SI_CHLD;
Applied to my linux-user-for-7.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 7+ messages in thread