Re: [RFC][PATCH v2 2/2][PR gdb/24069] [fix] Skip over WIFSTOPPED wait4 status

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 8b0ecfd5b77..a3c6a978676 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1063,7 +1063,7 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>      }
>    else if (hdr->msgh_id == 0x48)
>      {
> -      /* MACH_NOTIFY_DEAD_NAME: notification for exit.  */
> +      /* MACH_NOTIFY_DEAD_NAME: notification for exit *or* WIFSTOPPED. */

Two spaces after period.

>        int res;
>  
>        res = darwin_decode_notify_message (hdr, &inf);
> @@ -1103,16 +1103,26 @@ darwin_nat_target::decode_message (mach_msg_header_t *hdr,
>  		  return minus_one_ptid;
>  		}
>  	      if (WIFEXITED (wstatus))
> -		status->set_exited (WEXITSTATUS (wstatus));
> +		{
> +		  status->set_exited (WEXITSTATUS (wstatus));
> +	          inferior_debug (4, _("darwin_wait: pid=%d exit, status=0x%x\n"),
> +				  res_pid, wstatus);
> +		}
> +	      else if (WIFSTOPPED (wstatus))
> +		{
> +		  status->set_ignore ();
> +		  inferior_debug (4, _("darwin_wait: pid %d received WIFSTOPPED\n"),
> +				  res_pid);
> +		  return minus_one_ptid;

Can you add a comment explaining that the stop will be handled by a
following exception?  It can be confusing why we ignore a WIFSTOPPED.  A
bit like you explained in your commit message:

 - Read and ignore such messages (counting on the next exception
 message to let us know of the inferior's new state again)



> +		}
>  	      else
>  		{
>  		  status->set_signalled
>  		    (gdb_signal_from_host (WTERMSIG (wstatus)));
> +		  inferior_debug (4, _("darwin_wait: pid=%d received signal %d\n"),
> +				  res_pid, status->sig());
>  		}

Can you replace the else with

  else if (WIFSIGNALED (wstatus))

and add an else with a warning like:

  warning (_("Unexpected wait status after MACH_NOTIFY_DEAD_NAME "
             "notification: 0x%x"), wstatus);

I think that having this warning would have made the bug you fix more
visible, so easier to find.  Perhaps it will help catch another similar
problem in the future.

The else should probably do status->set_ignore () and return
minus_one_ptid.

Simon