This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
unpatch breakpoints from the fork child sooner.
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Sun, 17 May 2009 20:11:05 +0100
- Subject: unpatch breakpoints from the fork child sooner.
We currently only detach breakpoints from a fork child at
follow fork time, but this is too late. Consider:
The user installs a breakpoint at `foo' in the parent
before it forks, catches the fork with "catch fork", removes
the breakpoint before continuing, and now resumes, the
trap at `foo' is left behind in the child by mistake. Vis:
>./gdb ./testsuite/gdb.base/foll-fork
GNU gdb (GDB) 6.8.50.20090517-cvs
:
(gdb) b callee
Breakpoint 1 at 0x4005c3: file ../../../src/gdb/testsuite/gdb.base/foll-fork.c, line 12.
(gdb) catch fork
Catchpoint 2 (fork)
(gdb) r
Starting program: /home/pedro/gdb/mainline/build/gdb/testsuite/gdb.base/foll-fork
Catchpoint 2 (forked process 24262), 0x00007ffff789ac4b in fork () from /lib/libc.so.6
(gdb) del
Delete all breakpoints? (y or n) y
(gdb) set follow-fork-mode child
(gdb) c
Continuing.
callee: 24259
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00000000004005c4 in callee (i=
During symbol reading, incomplete CFI data; unspecified registers (e.g., rax) at 0x4005bc.
24262) at ../../../src/gdb/testsuite/gdb.base/foll-fork.c:12
12 printf("callee: %d\n", i);
(gdb)
I'm applying these patches below to fix this, and to add a
new testcase to foll-fork.exp to make sure we don't regress.
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'.
Tested on x86_64-linux with no regressions.
--
Pedro Alves
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/inf-ptrace.c | 4 +++-
gdb/infrun.c | 21 +++++++++++++++++++++
gdb/linux-nat.c | 18 +++++++++---------
3 files changed, 33 insertions(+), 10 deletions(-)
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2009-05-17 19:50:03.000000000 +0100
+++ src/gdb/infrun.c 2009-05-17 19:51:08.000000000 +0100
@@ -2418,6 +2418,27 @@ handle_inferior_event (struct execution_
reinit_frame_cache ();
}
+ /* Immediately detach breakpoints from the child before there's
+ any chance of letting the user deleting 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);
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c 2009-05-17 19:50:03.000000000 +0100
+++ src/gdb/linux-nat.c 2009-05-17 19:51:08.000000000 +0100
@@ -593,11 +593,15 @@ linux_child_follow_fork (struct target_o
/* 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_o
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 ();
Index: src/gdb/inf-ptrace.c
===================================================================
--- src.orig/gdb/inf-ptrace.c 2009-05-17 19:50:03.000000000 +0100
+++ src/gdb/inf-ptrace.c 2009-05-17 19:51:08.000000000 +0100
@@ -111,7 +111,9 @@ inf_ptrace_follow_fork (struct target_op
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"));
-------------------------------------------------------------------
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'.
---
gdb/testsuite/gdb.base/foll-fork.c | 7 ++
gdb/testsuite/gdb.base/foll-fork.exp | 82 ++++++++++++++++++++++++++++++++---
2 files changed, 81 insertions(+), 8 deletions(-)
Index: src/gdb/testsuite/gdb.base/foll-fork.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/foll-fork.c 2009-05-17 18:30:59.000000000 +0100
+++ src/gdb/testsuite/gdb.base/foll-fork.c 2009-05-17 19:39:55.000000000 +0100
@@ -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 */
}
Index: src/gdb/testsuite/gdb.base/foll-fork.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/foll-fork.exp 2009-05-17 18:32:07.000000000 +0100
+++ src/gdb/testsuite/gdb.base/foll-fork.exp 2009-05-17 19:49:51.000000000 +0100
@@ -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,71 @@ 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"
+
+ # Verify that the catchpoint is mentioned in an "info breakpoints",
+ # and further that the catchpoint mentions no process id.
+ #
+ set test_name "info shows catchpoint without pid"
+ gdb_test_multiple "info breakpoints" "$test_name" {
+ -re ".*catchpoint.*keep y.*fork\[\r\n\]+$gdb_prompt $" {
+ pass "$test_name"
+ }
+ }
+
+ 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, unpatch 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 +312,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 +425,11 @@ By default, the debugger will follow the
#
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.
#