This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc] Problems with software single-step and SPU breakpoints during fork
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: pedro at codesourcery dot com (Pedro Alves)
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 22 Jun 2010 17:22:37 +0200 (CEST)
- Subject: Re: [rfc] Problems with software single-step and SPU breakpoints during fork
Pedro Alves wrote:
> I think you should unpatch the single-step breakpoints from
> both parent and child. If you set detach-on-fork off, and then
> resume the child, won't it trip on the left over software single-step
> breakpoint? Oh, I guess that happens to not be a problem on the SPU,
> as both parent and child access the same SPU contexts, so removing
> the breakpoints from the parent also removed it from the child, though it
> looks like a problem on regular software single-step archs, like linux
> ARM or MIPS. Unfortunately, single step breakpoints are still using the
> deprecated_insert_raw_breakpoint mechanism and don't appear in the
> breakpoints/locations lists, otherwise, the "Immediately detach
> breakpoints" bit below would also take care of it ...
Good point. I've updated detach_breakpoints to also take care of
single-step breakpoints for now.
> Hmm, what is actually clearing single_step_breakpoints[0|1]?
> Are things just appearing to work correctly because
> the single_step_breakpoints[1] slot happens to be free on next
> single-step resume? (actually, don't we have the same problem
> with process exits while single-step breakpoints are inserted?
> that's a rare event, but possible.)
I've just copied this from process exit, but it seems you're right.
I've now added a new cancel_single_step_breakpoints routine that
can be called from infrun at those events.
> (software single-step and these related globals obviously
> need TLC for multi-process/non-stop)
That's certainly true.
> I think doing something clean would entail teaching gdb core
> about SPU/PPU's multiple address spaces.
That's not yet possible as it would require support for more than
one program / address space per inferior ... But long term this
seems the way to go.
> detach_breakpoints itself is a hack, so I don't have a problem
> with going with this approach meanwhile.
OK, thanks for the review and feedback!
Below is an updated patch with the changes described above.
Tested on powerpc64-linux (on Cell/B.E.).
Bye,
Ulrich
ChangeLog:
* infrun.c (handle_inferior_event): Handle presence of single-step
breakpoints for TARGET_WAITKIND_FORKED and TARGET_WAITKIND_VFORKED.
Cancel single-step breakpoints for TARGET_WAITKIND_EXITED,
TARGET_WAITKIND_SIGNALED, and TARGET_WAITKIND_EXECD.
* breakpoint.c (detach_single_step_breakpoints): New function.
(detach_breakpoints): Call it.
(cancel_single_step_breakpoints): New function.
* breakpoint.h (cancel_single_step_breakpoints): Add prototype.
* spu-tdep.c (spu_memory_remove_breakpoint): New function.
(spu_gdbarch_init): Install it.
testsuite/ChangeLog:
* gdb.cell/fork.exp: New file.
* gdb.cell/fork.c: Likewise.
* gdb.cell/fork-spu.c: Likewise.
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.490
diff -u -p -r1.490 breakpoint.c
--- gdb/breakpoint.c 16 Jun 2010 18:30:30 -0000 1.490
+++ gdb/breakpoint.c 22 Jun 2010 15:04:45 -0000
@@ -196,6 +196,8 @@ static void tcatch_command (char *arg, i
static void ep_skip_leading_whitespace (char **s);
+static void detach_single_step_breakpoints (void);
+
static int single_step_breakpoint_inserted_here_p (struct address_space *,
CORE_ADDR pc);
@@ -2383,6 +2385,10 @@ detach_breakpoints (int pid)
if (b->inserted)
val |= remove_breakpoint_1 (b, mark_inserted);
}
+
+ /* Detach single-step breakpoints as well. */
+ detach_single_step_breakpoints ();
+
do_cleanups (old_chain);
return val;
}
@@ -10491,6 +10497,39 @@ remove_single_step_breakpoints (void)
}
}
+/* Delete software single step breakpoints without removing them from
+ the inferior. This is intended to be used if the inferior's address
+ space where they were inserted is already gone, e.g. after exit or
+ exec. */
+
+void
+cancel_single_step_breakpoints (void)
+{
+ int i;
+
+ for (i = 0; i < 2; i++)
+ if (single_step_breakpoints[i])
+ {
+ xfree (single_step_breakpoints[i]);
+ single_step_breakpoints[i] = NULL;
+ single_step_gdbarch[i] = NULL;
+ }
+}
+
+/* Detach software single-step breakpoints from INFERIOR_PTID without
+ removing them. */
+
+static void
+detach_single_step_breakpoints (void)
+{
+ int i;
+
+ for (i = 0; i < 2; i++)
+ if (single_step_breakpoints[i])
+ target_remove_breakpoint (single_step_gdbarch[i],
+ single_step_breakpoints[i]);
+}
+
/* Check whether a software single-step breakpoint is inserted at PC. */
static int
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.119
diff -u -p -r1.119 breakpoint.h
--- gdb/breakpoint.h 7 Jun 2010 13:39:10 -0000 1.119
+++ gdb/breakpoint.h 22 Jun 2010 15:04:45 -0000
@@ -985,6 +985,7 @@ extern int remove_hw_watchpoints (void);
extern void insert_single_step_breakpoint (struct gdbarch *,
struct address_space *, CORE_ADDR);
extern void remove_single_step_breakpoints (void);
+extern void cancel_single_step_breakpoints (void);
/* Manage manual breakpoints, separate from the normal chain of
breakpoints. These functions are used in murky target-specific
Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.442
diff -u -p -r1.442 infrun.c
--- gdb/infrun.c 12 Jun 2010 00:05:21 -0000 1.442
+++ gdb/infrun.c 22 Jun 2010 15:04:47 -0000
@@ -3165,6 +3165,7 @@ handle_inferior_event (struct execution_
gdb_flush (gdb_stdout);
target_mourn_inferior ();
singlestep_breakpoints_inserted_p = 0;
+ cancel_single_step_breakpoints ();
stop_print_frame = 0;
stop_stepping (ecs);
return;
@@ -3188,6 +3189,7 @@ handle_inferior_event (struct execution_
print_stop_reason (SIGNAL_EXITED, ecs->ws.value.sig);
singlestep_breakpoints_inserted_p = 0;
+ cancel_single_step_breakpoints ();
stop_stepping (ecs);
return;
@@ -3225,6 +3227,13 @@ handle_inferior_event (struct execution_
detach_breakpoints (child_pid);
}
+ if (singlestep_breakpoints_inserted_p)
+ {
+ /* Pull the single step breakpoints out of the target. */
+ remove_single_step_breakpoints ();
+ singlestep_breakpoints_inserted_p = 0;
+ }
+
/* In case the event is caught by a catchpoint, remember that
the event is to be followed at the next resume of the thread,
and not immediately. */
@@ -3314,6 +3323,9 @@ handle_inferior_event (struct execution_
reinit_frame_cache ();
}
+ singlestep_breakpoints_inserted_p = 0;
+ cancel_single_step_breakpoints ();
+
stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
/* Do whatever is necessary to the parent branch of the vfork. */
Index: gdb/spu-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/spu-tdep.c,v
retrieving revision 1.60
diff -u -p -r1.60 spu-tdep.c
--- gdb/spu-tdep.c 19 Jun 2010 17:59:06 -0000 1.60
+++ gdb/spu-tdep.c 22 Jun 2010 15:04:47 -0000
@@ -1494,6 +1494,39 @@ spu_breakpoint_from_pc (struct gdbarch *
return breakpoint;
}
+static int
+spu_memory_remove_breakpoint (struct gdbarch *gdbarch,
+ struct bp_target_info *bp_tgt)
+{
+ /* We work around a problem in combined Cell/B.E. debugging here. Consider
+ that in a combined application, we have some breakpoints inserted in SPU
+ code, and now the application forks (on the PPU side). GDB common code
+ will assume that the fork system call copied all breakpoints into the new
+ process' address space, and that all those copies now need to be removed
+ (see breakpoint.c:detach_breakpoints).
+
+ While this is certainly true for PPU side breakpoints, it is not true
+ for SPU side breakpoints. fork will clone the SPU context file
+ descriptors, so that all the existing SPU contexts are in accessible
+ in the new process. However, the contents of the SPU contexts themselves
+ are *not* cloned. Therefore the effect of detach_breakpoints is to
+ remove SPU breakpoints from the *original* SPU context's local store
+ -- this is not the correct behaviour.
+
+ The workaround is to check whether the PID we are asked to remove this
+ breakpoint from (i.e. ptid_get_pid (inferior_ptid)) is different from the
+ PID of the current inferior (i.e. current_inferior ()->pid). This is only
+ true in the context of detach_breakpoints. If so, we simply do nothing.
+ [ Note that for the fork child process, it does not matter if breakpoints
+ remain inserted, because those SPU contexts are not runnable anyway --
+ the Linux kernel allows only the original process to invoke spu_run. */
+
+ if (ptid_get_pid (inferior_ptid) != current_inferior ()->pid)
+ return 0;
+
+ return default_memory_remove_breakpoint (gdbarch, bp_tgt);
+}
+
/* Software single-stepping support. */
@@ -2638,6 +2671,7 @@ spu_gdbarch_init (struct gdbarch_info in
/* Breakpoints. */
set_gdbarch_decr_pc_after_break (gdbarch, 4);
set_gdbarch_breakpoint_from_pc (gdbarch, spu_breakpoint_from_pc);
+ set_gdbarch_memory_remove_breakpoint (gdbarch, spu_memory_remove_breakpoint);
set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
set_gdbarch_software_single_step (gdbarch, spu_software_single_step);
set_gdbarch_get_longjmp_target (gdbarch, spu_get_longjmp_target);
--- /dev/null 2010-06-09 19:31:28.423437333 +0200
+++ gdb/testsuite/gdb.cell/fork.exp 2010-06-21 20:20:00.000000000 +0200
@@ -0,0 +1,85 @@
+# Copyright 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# Contributed by Ulrich Weigand <uweigand@de.ibm.com>.
+#
+# Testsuite for Cell Broadband Engine combined debugger
+# This testcases tests support for PPU-side fork during SPU debugging
+
+load_lib cell.exp
+
+set testfile "fork"
+set ppu_file "fork"
+set ppu_src ${srcdir}/${subdir}/${ppu_file}.c
+set ppu_bin ${objdir}/${subdir}/${ppu_file}
+set spu_file "fork-spu"
+set spu_src ${srcdir}/${subdir}/${spu_file}.c
+set spu_bin ${objdir}/${subdir}/${spu_file}
+
+if {[skip_cell_tests]} {
+ return 0
+}
+
+# Compile SPU binary.
+if { [gdb_compile_cell_spu $spu_src $spu_bin executable {debug}] != "" } {
+ unsupported "Compiling spu binary failed."
+ return -1
+}
+# Compile PPU binary.
+if { [gdb_cell_embedspu $spu_bin $spu_bin-embed.o {debug}] != "" } {
+ unsupported "Embedding spu binary failed."
+ return -1
+}
+if { [gdb_compile_cell_ppu [list $ppu_src $spu_bin-embed.o] $ppu_bin executable {debug}] != "" } {
+ unsupported "Compiling ppu binary failed."
+ return -1
+}
+
+if [get_compiler_info ${ppu_bin}] {
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${ppu_bin}
+
+if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+}
+
+gdb_test_no_output "set spu stop-on-load" "set spu stop-on-load"
+
+gdb_test "continue" "Continuing\\..*Temporary breakpoint \[0-9\]+, main \\(speid=.*, argp=.*, envp=.*\\) at .*$spu_file\\.c:.*spu_write_out_intr_mbox.*" \
+ "run until SPU main"
+
+gdb_test "break func" "Breakpoint \[0-9\]+ at.* file .*$spu_file.c, line \[0-9\]+\\." "break func"
+gdb_test "watch var" "Watchpoint \[0-9\]+: var" "watch var"
+
+gdb_test "continue" "Continuing\\..*Watchpoint.*Old value = 0.*New value = 1.*" \
+ "run until watchpoint hit"
+
+gdb_test_no_output "delete \$bpnum" "delete watchpoint"
+
+gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, func \\(\\) at .*$spu_file.c:.*" \
+ "run until breakpoint hit"
+
+gdb_test "continue" "Continuing\\..*Program exited normally.*" \
+ "run until end"
+
+gdb_exit
+
+return 0
--- /dev/null 2010-06-09 19:31:28.423437333 +0200
+++ gdb/testsuite/gdb.cell/fork.c 2010-06-21 19:32:02.000000000 +0200
@@ -0,0 +1,77 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+ Contributed by Ulrich Weigand <uweigand@de.ibm.com> */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <libspe2.h>
+#include <pthread.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+extern spe_program_handle_t fork_spu;
+
+void *
+spe_thread (void * arg)
+{
+ int flags = 0;
+ unsigned int entry = SPE_DEFAULT_ENTRY;
+ spe_context_ptr_t *ctx = (spe_context_ptr_t *) arg;
+
+ spe_program_load (*ctx, &fork_spu);
+ spe_context_run (*ctx, &entry, flags, NULL, NULL, NULL);
+
+ pthread_exit (NULL);
+}
+
+int
+main (void)
+{
+ pthread_t pts;
+ spe_context_ptr_t ctx;
+ unsigned int value;
+ unsigned int pid;
+
+ ctx = spe_context_create (0, NULL);
+ pthread_create (&pts, NULL, &spe_thread, &ctx);
+
+ /* Wait until the SPU thread is running. */
+ spe_out_intr_mbox_read (ctx, &value, 1, SPE_MBOX_ALL_BLOCKING);
+
+ pid = fork ();
+ if (pid == 0)
+ {
+ /* This is the child. Just exit immediately. */
+ exit (0);
+ }
+ else
+ {
+ /* This is the parent. Wait for the child to exit. */
+ waitpid (pid, NULL, 0);
+ }
+
+ /* Tell SPU to continue. */
+ spe_in_mbox_write (ctx, &value, 1, SPE_MBOX_ALL_BLOCKING);
+
+ pthread_join (pts, NULL);
+ spe_context_destroy (ctx);
+
+ return 0;
+}
+
--- /dev/null 2010-06-09 19:31:28.423437333 +0200
+++ gdb/testsuite/gdb.cell/fork-spu.c 2010-06-21 19:33:14.000000000 +0200
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+ Contributed by Ulrich Weigand <uweigand@de.ibm.com> */
+
+#include <spu_mfcio.h>
+
+int var;
+
+void
+func (void)
+{
+}
+
+int
+main (unsigned long long speid, unsigned long long argp,
+ unsigned long long envp)
+{
+ /* Signal to PPU side that it should fork now. */
+ spu_write_out_intr_mbox (0);
+
+ /* Wait until fork completed. */
+ spu_read_in_mbox ();
+
+ /* Trigger watchpoint. */
+ var = 1;
+
+ /* Now call some function to trigger breakpoint. */
+ func ();
+
+ return 0;
+}
+
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com