Re: [PATCH] linux-user: Trace wait4()'s and waitpid()'s wstatus

[Richard Henderson <richard.henderson@linaro.org>]

On 10/1/24 12:32, Ilya Leoshkevich wrote:
> Borrow the code for formatting the most frequent WIFEXITED() and
> WIFSIGNALED() special cases from from the strace's printstatus().
> 
> Output examples:
> 
>      474729 wait4(-1,0x7f00767ff0a0,0,(nil)) = 474733 (wstatus={WIFEXITED(s) && WEXITSTATUS(s) == 1})
>      475833 wait4(-1,0x7f7de61ff0a0,0,(nil)) = 475837 (wstatus={WIFSIGNALED(s) && WTERMSIG(s) == SIGKILL})
>      1168 waitpid(1171,0x7f44eea00340,0) = 1171 (wstatus={WIFSIGNALED(s) && WTERMSIG(s) == SIGKILL})
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   linux-user/strace.c    | 61 ++++++++++++++++++++++++++++++++++++++++++
>   linux-user/strace.list |  6 +++--
>   2 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index b4d1098170e..d5d1f18304d 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -4168,6 +4168,67 @@ print_ioctl(CPUArchState *cpu_env, const struct syscallname *name,
>   }
>   #endif
>   
> +#if defined(TARGET_NR_wait4) || defined(TARGET_NR_waitpid)
> +static void print_wstatus(int wstatus)
> +{
> +    if (WIFSIGNALED(wstatus)) {
> +        qemu_log("{WIFSIGNALED(s) && WTERMSIG(s) == ");
> +        print_signal(WTERMSIG(wstatus), 1);
> +        if (WCOREDUMP(wstatus)) {
> +            qemu_log(" && WCOREDUMP(s)");
> +        }
> +        qemu_log("}");
> +    } else if (WIFEXITED(wstatus)) {
> +        qemu_log("{WIFEXITED(s) && WEXITSTATUS(s) == %d}",
> +                 WEXITSTATUS(wstatus));
> +    } else {
> +        print_number(wstatus, 1);
> +    }
> +}
> +
> +static void print_ret_wstatus(abi_long ret, abi_long wstatus_addr)
> +{
> +    if (!print_syscall_err(ret)) {
> +        qemu_log(TARGET_ABI_FMT_ld " (wstatus=", ret);
> +        if (wstatus_addr == 0) {
> +            qemu_log("NULL");

I think you could merge the == 0 check into the outer IF, and avoid printing the returned 
wstatus entirely.

Otherwise
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~