This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v8 3/7] Extended-remote Linux follow fork


On 04/17/2015 09:58 PM, Don Breazeal wrote:

> +	if (status->kind == TARGET_WAITKIND_FORKED && report_fork_events)
> +	  {
> +	    enum gdb_signal signal = GDB_SIGNAL_TRAP;
> +
> +	    sprintf (buf, "T%02xfork:", signal);
> +	    buf += strlen (buf);
> +	    buf = write_ptid (buf, status->value.related_pid);
> +	    strcat (buf, ";");

We don't really need that ';', right?

> +	  }
> +	else
> +	  sprintf (buf, "T%02x", status->value.sig);

I think we should not rely on the value of "status->value.sig"
here, and send GDB_SIGNAL_0 explicitly.

>>> -  if (from_tty && !extended)
>>> +  if (from_tty && !rs->extended)
>>>      puts_filtered (_("Ending remote debugging.\n"));
>>>  
>>> -  target_mourn_inferior ();
>>> +  /* Check to see if we are detaching a fork parent.  Note that if we
>>> +     are detaching a fork child, tp == NULL.  */
>>> +  if (tp != NULL)
>>> +    is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;
>>> +
>>> +  /* If doing detach-on-fork, we don't mourn, because that will delete
>>> +     breakpoints that should be available for the followed inferior.  */
>>
>> How does this work in the native case then?
> 
> In this scenario we are following the fork child and detaching from
> the parent, which is sending us through the target's detach code.
> 
> In the native Linux case the detach code is linux_nat_detach, which in
> turn calls inf_ptrace_detach.  There is no call to target_mourn_inferior
> in this sequence of function calls, so the breakpoints aren't affected
> at that point.  The breakpoints are "fixed up" for the child by
> follow_inferior_reset_breakpoints at the very end of follow_fork, at
> which point the child inferior has been set up.
> 
> The is_fork_parent condition in remote_detach_1 preventing the call
> to target_mourn_inferior essentially makes the procedure the same as
> for the native case.

But how come the native side doesn't ever need to call mourn from
detach then?  Or put another way, why do we call generic_mourn_inferior
in remote.c's detach for?

I think something's still not very right in grand scheme of things.


> +
> +/* This detaches a program to which we previously attached, using
> +   inferior_ptid to identify the process.  After this is done, GDB
> +   can be used to debug some other program.  We better not have left
> +   any breakpoints in the target program or it'll die when it hits
> +   one.  */
>  
>  static void
> -remote_detach_1 (const char *args, int from_tty, int extended)
> +remote_detach_1 (const char *args, int from_tty)
>  {
>    int pid = ptid_get_pid (inferior_ptid);
>    struct remote_state *rs = get_remote_state ();
> +  struct thread_info *tp = find_thread_ptid (inferior_ptid);
> +  int is_fork_parent;

...

>  
> -  target_mourn_inferior ();
> +  /* Check to see if we are detaching a fork parent.  Note that if we
> +     are detaching a fork child, tp == NULL.  */
> +  if (tp != NULL)
> +    is_fork_parent = tp->pending_follow.kind == TARGET_WAITKIND_FORKED;

Seems like is_fork_parent is left uninitialized in the fork child case.
Should probably be:

  is_fork_parent = (tp != NULL
                    && tp->pending_follow.kind == TARGET_WAITKIND_FORKED);


I'm still confused about this though.

How can we distinguish whether we're detaching the parent for
detach-on-fork, or whether detach-on-fork is off, vs the user
doing an explicit "detach" before following the fork?

I notice that the native side forgets to detach the child in
the latter case...

...
Catchpoint 2 (forked process 32471), 0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130
130       pid = ARCH_FORK ();
(gdb) detach
Detaching from program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.base/foll-fork, process 32467
(gdb) shell cat /proc/32471/status
Name:   foll-fork
State:  t (tracing stop)
Tgid:   32471

> +
> +  /* If doing detach-on-fork, we don't mourn, because that will delete
> +     breakpoints that should be available for the followed inferior.  */
> +  if (!is_fork_parent)
> +    target_mourn_inferior ();
> +  else
> +    {
> +      inferior_ptid = null_ptid;
> +      detach_inferior (pid);
> +    }
>  }


So I think the detach scenario is still in need of further work,
but given that the native code doesn't get it right either, we can
address that as follow ups.

Thus, this version is OK with the issues mentioned
above fixed.

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]