gdb/

2009-05-17  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (handle_inferior_event): When handling a
	TARGET_WAITKIND_FORKED, detach breakpoints from the fork child
	immediatelly.
	* linux-nat.c (linux_child_follow_fork): Only detach breakpoint
	from the child if vforking.
	* inf-ptrace.c (inf_ptrace_follow_fork): No need to detach
	breakpoints from the child here.

gdb/testsuite/
2009-05-17  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/foll-fork.c: Include stdlib.h.  Add markers for
	`gdb_get_line_number'.  Call `callee' in both parent and child.
	* gdb.base/foll-fork.exp (catch_fork_child_follow): Use
	`gdb_get_line_number' instead of hardcoding line numbers.
	(catch_fork_unpatch_child): New procedure to test detaching
	breakpoints from child fork.
	(tcatch_fork_parent_follow): Use `gdb_get_line_number' instead of
	hardcoding line numbers.
	(do_fork_tests): Run `catch_fork_unpatch_child'.

7 files changed, 126 insertions, 18 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f35064c..f8a7287 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2009-05-17  Pedro Alves  <pedro@codesourcery.com>
 
+	* infrun.c (handle_inferior_event): When handling a
+	TARGET_WAITKIND_FORKED, detach breakpoints from the fork child
+	immediatelly.
+	* linux-nat.c (linux_child_follow_fork): Only detach breakpoint
+	from the child if vforking.
+	* inf-ptrace.c (inf_ptrace_follow_fork): No need to detach
+	breakpoints from the child here.
+
+2009-05-17  Pedro Alves  <pedro@codesourcery.com>
+
 	* infrun.c (pending_follow): Remove execd_pathname member.
 	(resume): No longer handle TARGET_WAITKIND_EXECD pending follow.
 	(handle_inferior_event): When handling a TARGET_WAITKIND_EXECD
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index f088ffd..7849f24 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -111,7 +111,9 @@ inf_ptrace_follow_fork (struct target_ops *ops, int follow_child)
   else
     {
       inferior_ptid = pid_to_ptid (pid);
-      detach_breakpoints (fpid);
+
+      /* Breakpoints have already been detached from the child by
+	 infrun.c.  */
 
       if (ptrace (PT_DETACH, fpid, (PTRACE_TYPE_ARG3)1, 0) == -1)
 	perror_with_name (("ptrace"));
diff --git a/gdb/infrun.c b/gdb/infrun.c
index dbaf02b..99873b1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2418,6 +2418,27 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  reinit_frame_cache ();
 	}
 
+      /* Immediately detach breakpoints from the child before there's
+	 any chance of letting the user delete breakpoints from the
+	 breakpoint lists.  If we don't do this early, it's easy to
+	 leave left over traps in the child, vis: "break foo; catch
+	 fork; c; <fork>; del; c; <child calls foo>".  We only follow
+	 the fork on the last `continue', and by that time the
+	 breakpoint at "foo" is long gone from the breakpoint table.
+	 If we vforked, then we don't need to unpatch here, since both
+	 parent and child are sharing the same memory pages; we'll
+	 need to unpatch at follow/detach time instead to be certain
+	 that new breakpoints added between catchpoint hit time and
+	 vfork follow are detached.  */
+      if (ecs->ws.kind != TARGET_WAITKIND_VFORKED)
+	{
+	  int child_pid = ptid_get_pid (ecs->ws.value.related_pid);
+
+	  /* This won't actually modify the breakpoint list, but will
+	     physically remove the breakpoints from the child.  */
+	  detach_breakpoints (child_pid);
+	}
+
       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
 
       ecs->event_thread->stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d67fcc3..beff012 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -593,11 +593,15 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child)
       /* We're already attached to the parent, by default. */
 
       /* Before detaching from the child, remove all breakpoints from
-         it.  (This won't actually modify the breakpoint list, but will
-         physically remove the breakpoints from the child.) */
-      /* If we vforked this will remove the breakpoints from the parent
-	 also, but they'll be reinserted below.  */
-      detach_breakpoints (child_pid);
+         it.  If we forked, then this has already been taken care of
+         by infrun.c.  If we vforked however, any breakpoint inserted
+         in the parent is visible in the child, even those added while
+         stopped in a vfork catchpoint.  This won't actually modify
+         the breakpoint list, but will physically remove the
+         breakpoints from the child.  This will remove the breakpoints
+         from the parent also, but they'll be reinserted below.  */
+      if (has_vforked)
+	detach_breakpoints (child_pid);
 
       /* Detach new forked process?  */
       if (detach_fork)
@@ -701,10 +705,6 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child)
 	 breakpoint.  */
       last_tp->step_resume_breakpoint = NULL;
 
-      /* Needed to keep the breakpoint lists in sync.  */
-      if (! has_vforked)
-	detach_breakpoints (child_pid);
-
       /* Before detaching from the parent, remove all breakpoints from it. */
       remove_breakpoints ();
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 54d1de6..1fb17ff 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,15 @@
+2009-05-17  Pedro Alves  <pedro@codesourcery.com>
+
+	* gdb.base/foll-fork.c: Include stdlib.h.  Add markers for
+	`gdb_get_line_number'.  Call `callee' in both parent and child.
+	* gdb.base/foll-fork.exp (catch_fork_child_follow): Use
+	`gdb_get_line_number' instead of hardcoding line numbers.
+	(catch_fork_unpatch_child): New procedure to test detaching
+	breakpoints from child fork.
+	(tcatch_fork_parent_follow): Use `gdb_get_line_number' instead of
+	hardcoding line numbers.
+	(do_fork_tests): Run `catch_fork_unpatch_child'.
+
 2009-05-17  Vladimir Prus  <vladimir@codesourcery.com>
 
 	* gdb.mi/mi-cmd-var.exp: Check that when varobj
diff --git a/gdb/testsuite/gdb.base/foll-fork.c b/gdb/testsuite/gdb.base/foll-fork.c
index 841258f..b7e69ab 100644
--- a/gdb/testsuite/gdb.base/foll-fork.c
+++ b/gdb/testsuite/gdb.base/foll-fork.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include <unistd.h>
+#include <stdlib.h>
 
 #ifdef PROTOTYPES
 void callee (int i)
@@ -21,14 +22,18 @@ main ()
   int  v = 5;
 
   pid = fork ();
-  if (pid == 0)
+  if (pid == 0) /* set breakpoint here */
     {
       v++;
       /* printf ("I'm the child!\n"); */
+      callee (getpid ());
     }
   else
     {
       v--;
       /* printf ("I'm the proud parent of child #%d!\n", pid); */
+      callee (getpid ());
     }
+
+  exit (0); /* at exit */
 }
diff --git a/gdb/testsuite/gdb.base/foll-fork.exp b/gdb/testsuite/gdb.base/foll-fork.exp
index 76475ad..08a0f49 100644
--- a/gdb/testsuite/gdb.base/foll-fork.exp
+++ b/gdb/testsuite/gdb.base/foll-fork.exp
@@ -147,6 +147,8 @@ proc catch_fork_child_follow {} {
    global gdb_prompt
    global srcfile
 
+   set bp_after_fork [gdb_get_line_number "set breakpoint here"]
+
    send_gdb "catch fork\n"
    gdb_expect {
       -re "Catchpoint .*(fork).*$gdb_prompt $"\
@@ -188,21 +190,21 @@ proc catch_fork_child_follow {} {
       -re "$gdb_prompt $" {pass "set follow child"}
       timeout         {fail "(timeout) set follow child"}
    }
-   send_gdb "tbreak ${srcfile}:24\n"
+   send_gdb "tbreak ${srcfile}:$bp_after_fork\n"
    gdb_expect {
-      -re "Temporary breakpoint.*, line 24.*$gdb_prompt $"\
+      -re "Temporary breakpoint.*, line $bp_after_fork.*$gdb_prompt $"\
                       {pass "set follow child, tbreak"}
       -re "$gdb_prompt $" {fail "set follow child, tbreak"}
       timeout         {fail "(timeout) set follow child, tbreak"}
    }
    send_gdb "continue\n"
    gdb_expect {
-      -re "Attaching after fork to.* at .*24.*$gdb_prompt $"\
+      -re "Attaching after fork to.* at .*$bp_after_fork.*$gdb_prompt $"\
                       {pass "set follow child, hit tbreak"}
       -re "$gdb_prompt $" {fail "set follow child, hit tbreak"}
       timeout         {fail "(timeout) set follow child, hit tbreak"}
    }
-   # The child has been detached; allow time for any output it might
+   # The parent 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.
    #
@@ -222,10 +224,61 @@ proc catch_fork_child_follow {} {
    }
 }
 
+proc catch_fork_unpatch_child {} {
+   global gdb_prompt
+   global srcfile
+
+   set bp_exit [gdb_get_line_number "at exit"]
+
+   gdb_test "break callee" "file .*$srcfile, line .*" "unpatch child, break at callee"
+   gdb_test "catch fork" "Catchpoint \[0-9\]* \\(fork\\)" "unpatch child, set catch fork"
+
+   gdb_test "continue" \
+       "Catchpoint.*\\(forked process.*\\).*,.*in .*(fork|__kernel_v?syscall).*" \
+       "unpatch child, catch fork"
+
+   # Delete all breakpoints and catchpoints.
+   delete_breakpoints
+
+   gdb_test "break $bp_exit" \
+       "Breakpoint .*file .*$srcfile, line .*" \
+       "unpatch child, breakpoint at exit call"
+
+   gdb_test "set follow child" "" "unpatch child, set follow child"
+
+   set test "unpatch child, unpatched parent breakpoints from child"
+   gdb_test_multiple "continue" $test {
+      -re "at exit.*$gdb_prompt $" {
+	  pass "$test"
+      }
+      -re "SIGTRAP.*$gdb_prompt $" {
+	  fail "$test"
+
+	  # Explicitly kill this child, so we can continue gracefully
+	  # with further testing...
+	  send_gdb "kill\n"
+	  gdb_expect {
+	      -re ".*Kill the program being debugged.*y or n. $" {
+		  send_gdb "y\n"
+		  gdb_expect -re "$gdb_prompt $" {}
+	      }
+	  }
+      }
+      -re ".*$gdb_prompt $" {
+	  fail "$test (unknown output)"
+      }
+      timeout {
+	  fail "$test (timeout)"
+      }
+   }
+}
+
 proc tcatch_fork_parent_follow {} {
    global gdb_prompt
    global srcfile
 
+   set bp_after_fork [gdb_get_line_number "set breakpoint here"]
+
    send_gdb "catch fork\n"
    gdb_expect {
       -re "Catchpoint .*(fork).*$gdb_prompt $"\
@@ -249,16 +302,16 @@ proc tcatch_fork_parent_follow {} {
       -re "$gdb_prompt $" {pass "set follow parent"}
       timeout         {fail "(timeout) set follow parent"}
    }
-   send_gdb "tbreak ${srcfile}:24\n"
+   send_gdb "tbreak ${srcfile}:$bp_after_fork\n"
    gdb_expect {
-      -re "Temporary breakpoint.*, line 24.*$gdb_prompt $"\
+      -re "Temporary breakpoint.*, line $bp_after_fork.*$gdb_prompt $"\
                       {pass "set follow parent, tbreak"}
       -re "$gdb_prompt $" {fail "set follow parent, tbreak"}
       timeout         {fail "(timeout) set follow child, tbreak"}
    }
    send_gdb "continue\n"
    gdb_expect {
-      -re ".*Detaching after fork from.* at .*24.*$gdb_prompt $"\
+      -re ".*Detaching after fork from.* at .*$bp_after_fork.*$gdb_prompt $"\
                       {pass "set follow parent, hit tbreak"}
       -re "$gdb_prompt $" {fail "set follow parent, hit tbreak"}
       timeout         {fail "(timeout) set follow parent, hit tbreak"}
@@ -362,6 +415,11 @@ By default, the debugger will follow the parent process..*$gdb_prompt $"\
    #
    if [runto_main] then { catch_fork_child_follow }
 
+   # Test that parent breakpoints are successfully detached from the
+   # child at fork time, even if the user removes them from the
+   # breakpoints list after stopping at a fork catchpoint.
+   if [runto_main] then { catch_fork_unpatch_child }
+
    # Test the ability to catch a fork, specify via a -do clause that
    # the parent be followed, and continue.  Make the catchpoint temporary.
    #