Re: [PATCH] signal: Preserve the ptrace stop code for SIGCHLD notification

[Oleg Nesterov <oleg@redhat.com>]

On 11/11, Zhu Jun wrote:
>
> After ptrace_stop() drops siglock, the tracer can consume the stop with
> waitpid() and clear the tracee's exit_code. do_notify_parent_cldstop()
> then rereads that field without siglock and can report CLD_TRAPPED with
> a zero si_status instead of the stop signal.
>
> Pass the stop code saved in ptrace_stop() to the notification helper.
> The other notification reasons continue to obtain their status from
> the existing group-stop and continuation state.

But how the "correct" .si_status can actually help debugger if it has
already consumed tsk->exit_code? Why do we want this change?

> Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")

Yes, this logic is very old. And unlikely useful.

I don't think userspace actually uses the siginfo which comes with SIGCHLD.
If nothing else, SIGCHLD doesn't queue, so si_code/si_status is not necessary
uptodate even if parent/debugger has a single child/tracee.

>  static void do_notify_parent_cldstop(struct task_struct *tsk,
> -				     bool for_ptracer, int why)
> +				     bool for_ptracer, int why, int exit_code)
>  {
>  	struct kernel_siginfo info;
>  	unsigned long flags;
> @@ -2394,7 +2395,7 @@ static void do_notify_parent_cldstop(struct task_struct *tsk,
>   		info.si_status = tsk->signal->group_exit_code & 0x7f;
>   		break;
>   	case CLD_TRAPPED:
> - 		info.si_status = tsk->exit_code & 0x7f;
> +		info.si_status = exit_code & 0x7f;

Note that tsk->signal->group_exit_code can be cleared by waitpid() the same way,
even if tsk is not traced, so this change is incomplete.

Again, I personally think we don't care.. Perhaps something like this patch makes
sense just to make this code look more correct, but then please consider the
CLD_STOPPED case as well.

Oleg.