Symptom:

Using the test program gdb.base/foll-fork.c, with follow-fork-mode set to
"child" and detach-on-fork set to "off", stepping or running past the fork
call results in the child process running to completion, when it should
just finish the single step.  In addition, the breakpoint is not removed
from the parent process, so if it is resumed it receives a SIGTRAP.

Cause:

No matter what the setting for detach-on-fork, when stepping past a fork,
the single-step breakpoint (step_resume_breakpoint) is not handled
correctly in the parent.  The SR breakpoint is cloned for the child
process, but before the clone is associated with the child it is treated as
a duplicate of the original, associated wth the parent.  This results in
the insertion state of the original SR breakpoint and the clone being
"swapped" by breakpoint.c:update_global_location_list, so that the clone is
marked as inserted.

In the case where the parent is not detached, the two breakpoints remain in
that state.  The breakpoint is never inserted in the child, because
although the cloned SR breakpoint is associated with the child, it is
marked as inserted.  When the child is resumed, it runs to completion.  The
breakpoint is never removed from the parent, so that if it is resumed after
the child exits, it gets a SIGTRAP.

Here is the sequence of events:

1) handle_inferior_event: FORK event is recognized.

2) handle_inferior_event: detach_breakpoints removes all breakpoints
from the child.

3) follow_fork: the parent SR breakpoint is cloned.  Part of this procedure
is to call update_global_location_list, which swaps the insertion state of
the original and cloned SR breakpoints as part of ensuring that duplicate
breakpoints are only inserted once.  At this point the original SR
breakpoint is not marked as inserted, and the clone is.  The breakpoint is
actually inserted in the parent but not the child.

4) follow_fork: the original breakpoint is deleted by calling
delete_step_resume_breakpoint.  Since the original is not marked as
inserted, the actual breakpoint remains in the parent process.
update_global_location_list is called again as part of the deletion.  The
clone is still associated with the parent, but since it is marked as
enabled and inserted, the breakpoint is left in the parent.

5) follow_fork: if detach-on-fork is 'on', the actual breakpoint will be
removed from the parent in target_detach, based on the cloned breakpoint
still associated with the parent.  Then the clone is no longer marked as
inserted.  In follow_inferior_reset_breakpoints the clone is associated
with the child, and can be inserted.

If detach-on-fork is 'off', the actual breakpoint in the parent is never
removed (although the breakpoint had been deleted from the list).  Since
the clone continues to be marked 'inserted', the SR breakpoint is never
inserted in the child.

Fix:

Set the cloned breakpoint as disabled from the moment it is created.  This
is done by modifying clone_momentary_breakpoint to take an additional
argument, LOC_ENABLED, which is used as the value of the
bp_location->enabled member.  The clone must be disabled at that point
because clone_momentary_breakpoint calls update_global_location_list, which
will swap treat the clone as a duplicate of the original breakpoint if it
is enabled.

All the calls to clone_momentary_breakpoint had to be modified to pass '1'
or '0'.  I looked at implementing an enum for the enabled member, but
concluded that readability would suffer because there are so many places it
is used as a boolean, e.g. "if (bl->enabled)".

In follow_inferior_reset_breakpoints the clone is set to enabled once it
has been associated with the child process.  With this, the bp_location
'inserted' member is maintained correctly throughout the follow-fork
procedure and the behavior is as expected.

The same treatment is given to the exception_resume_breakpoint when
following a fork.

Testing:

Ran 'make check' on Linux x64.

Along with the fix above, the coverage of the follow-fork test
gdb.base/foll-fork.exp was expanded to:

1) cover all the combinations of values for
   follow-fork-mode and detach-on-fork

2) make sure that both user breakpoints and
   single-step breakpoints are propagated
   correctly to the child

3) check that the inferior list has the
   expected contents after following the fork.

4) check that unfollowed, undetached inferiors
   can be resumed.

gdb/

2014-06-18  Don Breazeal  <donb@codesourcery.com>

	* breakpoint.c (set_longjmp_breakpoint): Call
	momentary_breakpoint_from_master with additional argument.
	(set_longjmp_breakpoint_for_call_dummy): Call
	momentary_breakpoint_from_master with additional argument.
	(set_std_terminate_breakpoint): Call
	momentary_breakpoint_from_master with additional argument.
	(momentary_breakpoint_from_master): Add argument to function
	definition and use it to initialize structure member flag.
	(clone_momentary_breakpoint): Call
	momentary_breakpoint_from_master with additional argument.
	* infrun.c (follow_inferior_reset_breakpoints): Clear structure
	member flags set in momentary_breakpoint_from_master.

gdb/testsuite/

2014-06-18  Don Breazeal  <donb@codesourcery.com>

	* gdb.base/foll-fork.exp (default_fork_parent_follow):
	Deleted procedure.
	(explicit_fork_parent_follow): Deleted procedure.
	(explicit_fork_child_follow): Deleted procedure.
	(test_follow_fork): New procedure.
	(do_fork_tests): Replace calls to deleted procedures with
	calls to test_follow_fork and reset GDB for subsequent
	procedure calls.

5 files changed, 182 insertions, 72 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 21bcb7f..04ee3ef 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2014-06-18  Don Breazeal  <donb@codesourcery.com>
+
+	* breakpoint.c (set_longjmp_breakpoint): Call
+	momentary_breakpoint_from_master with additional argument.
+	(set_longjmp_breakpoint_for_call_dummy): Call
+	momentary_breakpoint_from_master with additional argument.
+	(set_std_terminate_breakpoint): Call
+	momentary_breakpoint_from_master with additional argument.
+	(momentary_breakpoint_from_master): Add argument to function
+	definition and use it to initialize structure member flag.
+	(clone_momentary_breakpoint): Call 
+	momentary_breakpoint_from_master with additional argument.
+	* infrun.c (follow_inferior_reset_breakpoints): Clear structure
+	member flags set in momentary_breakpoint_from_master.
+
 2014-06-18  Gary Benson  <gbenson@redhat.com>
 
 	* i386-nat.c (i386_show_dr): Renamed to
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 2240f08..f3e6283 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -154,7 +154,8 @@ struct breakpoint *set_raw_breakpoint (struct gdbarch *gdbarch,
 static struct breakpoint *
   momentary_breakpoint_from_master (struct breakpoint *orig,
 				    enum bptype type,
-				    const struct breakpoint_ops *ops);
+				    const struct breakpoint_ops *ops,
+				    int loc_enabled);
 
 static void breakpoint_adjustment_warning (CORE_ADDR, CORE_ADDR, int, int);
 
@@ -7390,7 +7391,7 @@ set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame)
 	/* longjmp_breakpoint_ops ensures INITIATING_FRAME is cleared again
 	   after their removal.  */
 	clone = momentary_breakpoint_from_master (b, type,
-						  &longjmp_breakpoint_ops);
+						  &longjmp_breakpoint_ops, 1);
 	clone->thread = thread;
       }
 
@@ -7440,7 +7441,8 @@ set_longjmp_breakpoint_for_call_dummy (void)
 	struct breakpoint *new_b;
 
 	new_b = momentary_breakpoint_from_master (b, bp_longjmp_call_dummy,
-						  &momentary_breakpoint_ops);
+						  &momentary_breakpoint_ops,
+						  1);
 	new_b->thread = pid_to_thread_id (inferior_ptid);
 
 	/* Link NEW_B into the chain of RETVAL breakpoints.  */
@@ -7533,7 +7535,7 @@ set_std_terminate_breakpoint (void)
 	&& b->type == bp_std_terminate_master)
       {
 	momentary_breakpoint_from_master (b, bp_std_terminate,
-					  &momentary_breakpoint_ops);
+					  &momentary_breakpoint_ops, 1);
       }
 }
 
@@ -9052,13 +9054,14 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
 }
 
 /* Make a momentary breakpoint based on the master breakpoint ORIG.
-   The new breakpoint will have type TYPE, and use OPS as it
-   breakpoint_ops.  */
+   The new breakpoint will have type TYPE, use OPS as its
+   breakpoint_ops, and will set enabled to LOC_ENABLED.  */
 
 static struct breakpoint *
 momentary_breakpoint_from_master (struct breakpoint *orig,
 				  enum bptype type,
-				  const struct breakpoint_ops *ops)
+				  const struct breakpoint_ops *ops,
+				  int loc_enabled)
 {
   struct breakpoint *copy;
 
@@ -9074,6 +9077,7 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
   copy->loc->probe = orig->loc->probe;
   copy->loc->line_number = orig->loc->line_number;
   copy->loc->symtab = orig->loc->symtab;
+  copy->loc->enabled = loc_enabled;
   copy->frame_id = orig->frame_id;
   copy->thread = orig->thread;
   copy->pspace = orig->pspace;
@@ -9096,7 +9100,7 @@ clone_momentary_breakpoint (struct breakpoint *orig)
   if (orig == NULL)
     return NULL;
 
-  return momentary_breakpoint_from_master (orig, orig->type, orig->ops);
+  return momentary_breakpoint_from_master (orig, orig->type, orig->ops, 0);
 }
 
 struct breakpoint *
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 47604c7..4e808d9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -569,7 +569,9 @@ follow_inferior_reset_breakpoints (void)
 
   /* Was there a step_resume breakpoint?  (There was if the user
      did a "next" at the fork() call.)  If so, explicitly reset its
-     thread number.
+     thread number.  Cloned step_resume breakpoints are disabled on
+     creation, so enable it here now that it is associated with the
+     correct thread.
 
      step_resumes are a form of bp that are made to be per-thread.
      Since we created the step_resume bp when the parent process
@@ -579,10 +581,17 @@ follow_inferior_reset_breakpoints (void)
      it is for, or it'll be ignored when it triggers.  */
 
   if (tp->control.step_resume_breakpoint)
-    breakpoint_re_set_thread (tp->control.step_resume_breakpoint);
+    {
+      breakpoint_re_set_thread (tp->control.step_resume_breakpoint);
+      tp->control.step_resume_breakpoint->loc->enabled = 1;
+    }
 
+  /* Treat exception_resume breakpoints like step_resume breakpoints.  */
   if (tp->control.exception_resume_breakpoint)
-    breakpoint_re_set_thread (tp->control.exception_resume_breakpoint);
+    {
+      breakpoint_re_set_thread (tp->control.exception_resume_breakpoint);
+      tp->control.exception_resume_breakpoint->loc->enabled = 1;
+    }
 
   /* Reinsert all breakpoints in the child.  The user may have set
      breakpoints after catching the fork, in which case those
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c6385cd..bbd3243 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2014-06-18  Don Breazeal  <donb@codesourcery.com>
+
+	* gdb.base/foll-fork.exp (default_fork_parent_follow):
+	Deleted procedure.
+	(explicit_fork_parent_follow): Deleted procedure.
+	(explicit_fork_child_follow): Deleted procedure.
+	(test_follow_fork): New procedure.
+	(do_fork_tests): Replace calls to deleted procedures with
+	calls to test_follow_fork and reset GDB for subsequent 
+	procedure calls.
+
 2014-06-17  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.base/wchar.exp: Set $cent to \u00A2 if "host-charset" is
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index e1201d7..ad8b750 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -53,60 +53,130 @@ proc check_fork_catchpoints {} {
   }
 }
 
-proc default_fork_parent_follow {} {
+# Test follow-fork to ensure that the correct process is followed, that
+# the followed process stops where it is expected to stop, that processes
+# are detached (or not) as expected, and that the inferior list has the
+# expected contents after following the fork.  WHO is the argument to
+# the 'set follow-fork-mode' command, DETACH is the argument to the 
+# 'set detach-on-fork' command, and CMD is the GDB command used to 
+# execute the program past the fork.  If the value of WHO or DETACH is
+# 'default', the corresponding GDB command is skipped for that test.
+# The value of CMD must be either 'next 2' or 'continue'.
+proc test_follow_fork { who detach cmd } {
     global gdb_prompt
+    global srcfile
+    global testfile
 
-    gdb_test "show follow-fork" \
-	"Debugger response to a program call of fork or vfork is \"parent\".*" \
-	"default show parent follow, no catchpoints"
+    with_test_prefix "follow $who, detach $detach, command \"$cmd\"" {
 
-    gdb_test "next 2" \
-	"Detaching after fork from.*" \
-	"default parent follow, no catchpoints"
+	# Start a new debugger session each time so defaults are legitimate.
+	clean_restart $testfile
 
-    # The child has been detached; allow time for any output it might
-    # generate to arrive, so that output doesn't get confused with
-    # any expected debugger output from a subsequent testpoint.
-    #
-    exec sleep 1
-}
+	if ![runto_main] {
+	    untested "could not run to main"
+	    return -1
+	}
 
-proc explicit_fork_parent_follow {} {
-    global gdb_prompt
+	# The "Detaching..." and "Attaching..." messages may be hidden by
+	# default.
+	gdb_test_no_output "set verbose"
 
-    gdb_test_no_output "set follow-fork parent"
+	# Set follow-fork-mode if we aren't using the default.
+	if {$who == "default"} {
+	    set who "parent"
+	} else {
+	    gdb_test_no_output "set follow-fork $who"
+	}
 
-    gdb_test "show follow-fork" \
-	"Debugger response to a program call of fork or vfork is \"parent\"." \
-	"explicit show parent follow, no catchpoints"
+	gdb_test "show follow-fork" \
+	"Debugger response to a program call of fork or vfork is \"$who\"." \
+	"show follow-fork"
 
-    gdb_test "next 2" "Detaching after fork from.*" \
-	"explicit parent follow, no catchpoints"
+	# Set detach-on-fork mode if we aren't using the default.
+	if {$detach == "default"} {
+	    set detach "on"
+	} else {
+	    gdb_test_no_output "set detach-on-fork $detach"
+	}
 
-    # The child has been detached; allow time for any output it might
-    # generate to arrive, so that output doesn't get confused with
-    # any expected debugger output from a subsequent testpoint.
-    #
-    exec sleep 1
-}
+	gdb_test "show detach-on-fork" \
+		 "Whether gdb will detach.* fork is $detach." \
+		 "show detach-on-fork"
+
+	# Set a breakpoint after the fork if we aren't single-stepping
+	# past the fork.
+	if {$cmd == "continue"} {
+	    set bp_after_fork [gdb_get_line_number "set breakpoint here"]
+	    gdb_test "break ${srcfile}:$bp_after_fork" \
+		     "Breakpoint.*, line $bp_after_fork.*" \
+		     "set breakpoint after fork"
+	}
 
-proc explicit_fork_child_follow {} {
-    global gdb_prompt
+	# Set up the output we expect to see after we run.
+	set expected_re ""
+	if {$who == "child"} {
+	    set expected_re "Attaching after.* fork to.*set breakpoint here.*"
+	} elseif {$who == "parent" && $detach == "on"} {
+	    set expected_re "Detaching after fork from .*set breakpoint here.*"
+	} else {
+	    set expected_re ".*set breakpoint here.*"
+	}
 
-    gdb_test_no_output "set follow-fork child"
+	# Test running past and following the fork, using the parameters
+	# set above.
+	gdb_test $cmd $expected_re "$cmd past fork"
 
-    gdb_test "show follow-fork" \
-	"Debugger response to a program call of fork or vfork is \"child\"." \
-	"explicit show child follow, no catchpoints"
+	# Check that we have the inferiors arranged correctly after 
+	# following the fork.
+	set resume_unfollowed 0
+	if {$who == "parent" && $detach == "on"} {
 
-    gdb_test "next 2" "Attaching after.* fork to.*" \
-	"explicit child follow, no catchpoints"
+	    # Follow parent / detach child: the only inferior is the parent.
+	    gdb_test "info inferiors" "\\* 1 .* process.*" \
+		     "info inferiors"
 
-    # The child has been detached; allow time for any output it might
-    # generate to arrive, so that output doesn't get confused with
-    # any gdb_expected debugger output from a subsequent testpoint.
-    #
-    exec sleep 1
+	} elseif {$who == "parent" && $detach == "off"} {
+
+	    # Follow parent / keep child: two inferiors under debug, the
+	    # parent is the current inferior.
+	    gdb_test "info inferiors" " 2 .*process.*\\* 1 .*process.*" \
+		     "info inferiors"
+
+	    gdb_test "inferior 2" "Switching to inferior 2 .*"
+	    set resume_unfollowed 1
+
+	} elseif {$who == "child" && $detach == "on"} {
+
+	    # Follow child / detach parent: the child is under debug and is
+	    # the current inferior.  The parent is listed but is not under
+	    # debug.
+	    gdb_test "info inferiors" "\\* 2 .*process.* 1 .*<null>.*" \
+		     "info inferiors"
+
+	} elseif {$who == "child" && $detach == "off"} {
+
+	    # Follow child / keep parent: two inferiors under debug, the
+	    # child is the current inferior.
+	    gdb_test "info inferiors" "\\* 2 .*process.* 1 .*process.*" \
+		     "info inferiors"
+
+	    gdb_test "inferior 1" "Switching to inferior 1 .*"
+	    set resume_unfollowed 1
+	}
+
+	if {$resume_unfollowed == 1} {
+	    if {$cmd == "next 2"} {
+
+		gdb_continue_to_end "continue unfollowed inferior to end"
+
+	    } elseif {$cmd == "continue"} {
+
+		gdb_continue_to_breakpoint \
+		    "continue unfollowed inferior to bp" \
+		    ".* set breakpoint here.*"
+	    }
+	}
+    }
 }
 
 proc catch_fork_child_follow {} {
@@ -254,6 +324,7 @@ proc tcatch_fork_parent_follow {} {
 
 proc do_fork_tests {} {
     global gdb_prompt
+    global testfile
 
     # Verify that help is available for "set follow-fork-mode".
     #
@@ -286,31 +357,31 @@ By default, the debugger will follow the parent process..*" \
     # fork-following is supported.
     if [runto_main] then { check_fork_catchpoints }
 
-    # Test the default behaviour, which is to follow the parent of a
-    # fork, and detach from the child.  Do this without catchpoints.
+    # Test the basic follow-fork functionality using all combinations of
+    # values for follow-fork-mode and detach-on-fork, using either a
+    # breakpoint or single-step to execute past the fork.
     #
-    if [runto_main] then { default_fork_parent_follow }
-
-    # Test the ability to explicitly follow the parent of a fork, and
-    # detach from the child.  Do this without catchpoints.
-    #
-    if [runto_main] then { explicit_fork_parent_follow }
+    # The first loop should be sufficient to test the defaults.  There
+    # is no need to test using the defaults in other permutations (e.g.
+    # "default" "on", "parent" "default", etc.).
+    foreach cmd {"next 2" "continue"} {
+        test_follow_fork "default" "default" $cmd
+    }
 
-    # Test the ability to follow the child of a fork, and detach from
-    # the parent.  Do this without catchpoints.
-    #
-    if [runto_main] then { explicit_fork_child_follow }
+    # Now test all explicit permutations.
+    foreach who {"parent" "child"} {
+	foreach detach {"on" "off"} {
+	    foreach cmd {"next 2" "continue"} {
+		test_follow_fork $who $detach $cmd
+	    }
+	}
+    }
 
-    # Test the ability to follow both child and parent of a fork.  Do
-    # this without catchpoints.
-    # ??rehrauer: NYI.  Will add testpoints here when implemented.
-    #
+    # Catchpoint tests.
 
-    # Test the ability to have the debugger ask the user at fork-time
-    # whether to follow the parent, child or both.  Do this without
-    # catchpoints.
-    # ??rehrauer: NYI.  Will add testpoints here when implemented.
-    #
+    # Restart to eliminate any effects of the follow-fork tests.
+    clean_restart $testfile
+    gdb_test_no_output "set verbose"
 
     # Test the ability to catch a fork, specify that the child be
     # followed, and continue.  Make the catchpoint permanent.