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

["Breazeal, Don" <donb at codesourcery dot com>]

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.

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.

>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...]
          }
	[...calls target_follow_fork...]
            /* If we followed the child, switch to it...  */
            if (follow_child)
              {
                switch_to_thread (child);

                /* ... and preserve the stepping state, in case the
                   user was stepping over the fork call.  */
                if (should_resume)
                  {
                    tp = inferior_thread ();
                    tp->control.step_resume_breakpoint
                      = step_resume_breakpoint;
		    [...this only affects the child process...]
                    if (tp->control.step_resume_breakpoint != NULL)
                      tp->control.step_resume_breakpoint->loc->inserted = 0;

Does that explanation make sense?

Luis suggests adding a comment that explains why the flag is cleared.
Would that be sufficient?

Thanks
--Don