Re: [PATCH] Fix for follow-fork: followed child doesn't stop

[Pedro Alves <palves@redhat.com>]

On 06/05/2014 07:34 PM, Breazeal, Don wrote:
> Hi Pedro,
> Thanks for looking at this.
> 
> On 6/5/2014 5:52 AM, Pedro Alves wrote:
>> Hi Don,
>>
>> On 06/04/2014 11:19 PM, Don Breazeal wrote:
>>> Using the test program gdb.base/foll-fork.c, with follow-fork-mode
>>> set to "child" and detach-on-fork set to "on", stepping past the
>>> fork call results in the child process running to completion, when
>>> it should just finish the single step.
>>>
>>> This is the result of how the single-step state is transferred from
>>> the parent to the child in infrun.c:follow_fork.  For the parent, the
>>> single-step breakpoint is marked as "inserted" (bp->loc->inserted).
>>
>>> The breakpoint is transferred to the child, where it clearly has never
>>> been inserted.  
>>
>> Was it removed from the parent already at this point?  If so,
>> why is it still marked as inserted?  If not, then it would sound
>> like your patch would make us miss removing it.
>>
> Yes, by the time the 'inserted' flag is cleared, the breakpoint has been
> removed from the parent.  The flag is set because the child's breakpoint
> is a clone of the parent's breakpoint that was created before the
> parent's breakpoint was removed.

It actually hasn't.

> 
> The step-resume breakpoint is cloned from the parent's copy earlier in
> the function, bringing the value of the 'inserted' flag along with it.
> Then the parent's breakpoint is deleted, with the side-effect of
> removing it.

No, when the parent's breakpoint is deleted, nothing is removed,
because the child's breakpoint, being a clone of the parent's is
still associated with the parent thread and pspace, so the breakpoints
module considers it a duplicate of the parent's sr breakpoint.
So, when we delete the parent breakpoint, given the child's is
around, we don't actually remove the breakpoint from the
target.  We just happen to remove the new _child_'s breakpoint
from the parent a bit later when we detach the parent, again
because the child's is still associated with the parent:

(top-gdb) bt
#0  target_remove_breakpoint (gdbarch=0x1000ee0, bp_tgt=0x10db7d8) at ../../src/gdb/target.c:1938
#1  0x0000000000584aac in bkpt_remove_location (bl=0x10db740) at ../../src/gdb/breakpoint.c:13189
#2  0x0000000000574095 in remove_breakpoint_1 (bl=0x10db740, is=mark_uninserted) at ../../src/gdb/breakpoint.c:3823
#3  0x0000000000574436 in remove_breakpoint (bl=0x10db740, is=mark_uninserted) at ../../src/gdb/breakpoint.c:3940
#4  0x0000000000572d41 in remove_breakpoints_pid (pid=1408) at ../../src/gdb/breakpoint.c:3119
#5  0x000000000063d5f8 in target_detach (args=0x0, from_tty=0) at ../../src/gdb/target.c:2079
#6  0x00000000004b1c81 in linux_child_follow_fork (ops=0xdcfc30, follow_child=1, detach_fork=1) at ../../src/gdb/linux-nat.c:657
#7  0x00000000006372b6 in delegate_follow_fork (self=0xdcfc30, arg1=1, arg2=1) at ../../src/gdb/target-delegates.c:464
#8  0x000000000063daed in target_follow_fork (follow_child=1, detach_fork=1) at ../../src/gdb/target.c:2220
#9  0x00000000005eda7a in follow_fork () at ../../src/gdb/infrun.c:493
(top-gdb) up
#1  0x0000000000584aac in bkpt_remove_location (bl=0x10db740) at ../../src/gdb/breakpoint.c:13189
13189         return target_remove_breakpoint (bl->gdbarch, bp_tgt);
(top-gdb) p bl->owner->type
$8 = bp_step_resume

> 
> From infrun.c:follow_fork:
> 
> 	[...here 'tp' is the parent thread...]
>         if (follow_child && should_resume)
>           {
>             step_resume_breakpoint = clone_momentary_breakpoint
>                                  (tp->control.step_resume_breakpoint);
> 	    [...]
>             delete_step_resume_breakpoint (tp);
> 	    [...here bkpt has been removed from the parent...]

It hasn't here no.  This is all quite brittle.  :-/

I think we should instead make the clone be disabled to begin with,
so that it isn't marked as duplicate/inserted, and then re-enable it
in follow_inferior_reset_breakpoints, _after_ it's been re_set
to the child thread.

-- 
Pedro Alves