Re: [PATCH 06/16 v3] Extended-remote Linux follow fork

[Pedro Alves <palves at redhat dot com>]

On 10/31/2014 11:28 PM, Don Breazeal wrote:
> This patch implements basic support for follow-fork and detach-on-fork on
> extended-remote Linux targets.  Only 'fork' is supported in this patch;
> 'vfork' support is added n a subsequent patch.  Sufficient extended-remote
> functionality has been implemented here to pass gdb.base/foll-fork.exp with
> the catchpoint tests commented out.
> 

Thanks.

Let's try to shake out the higher-level concepts first before
focusing on specific details.

> 
>  * implementing a new RSP 'T' Stop Reply Packet stop reason: "fork", in
>    gdbserver/remote-utils.c and remote.c.

"f" is an hexadecimal digit.  Last time we added such a packet,
"core", we broke backwards compatibility with older GDBs...  :-/
We need to careful with that.

> 
>  * implementing new target and RSP support for target_follow_fork with
>    target extended-remote.  (The RSP components were actually defined in
>    patch 4, but they see their first use here).
> 
>    - extended_remote target routine extended_remote_follow_fork
> 
>    - RSP packet vFollowFork

Reading through this, I don't think this is the model we should be exposing
at the RSP level, and requiring servers to support.  The hiding of the child
fork until the users resumes is a current detail that we may want to change in
the future.  It seems better to me to _not_ hide the child from GDB,
and then implement the hide-child-until-resume detail in GDB.  That is,
I think we should model fork events at the RSP level similarly to
how ptrace and ttrace expose them.  So, e.g., I think switching to the
child to write to its memory should be done with the regular Hg packet.
Handling detach_fork would be done by GDB calling the regular
detach packet (D;PID), etc.  I'm not even seeing a fundamental need
to keep this for "extended-remote" alone, given gdb nowadays supports
the multiprocess extensions with "target remote" too.

Also, I don't see how this packet could work correctly with non-stop
mode.  You're assuming only one thread/process has stopped for a fork.

>  	      else
>  		{
>  		  /* Silently skip unknown optional info.  */
> @@ -9418,8 +9447,11 @@ remote_pid_to_str (struct target_ops *ops, ptid_t ptid)
>        if (ptid_equal (magic_null_ptid, ptid))
>  	xsnprintf (buf, sizeof buf, "Thread <main>");
>        else if (rs->extended && remote_multi_process_p (rs))
> -	xsnprintf (buf, sizeof buf, "Thread %d.%ld",
> -		   ptid_get_pid (ptid), ptid_get_lwp (ptid));
> +	if (ptid_get_lwp (ptid) == 0)
> +	  return normal_pid_to_str (ptid);

Can you explain this bit?

Thanks,
Pedro Alves