From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31092 invoked by alias); 22 Jun 2010 15:22:59 -0000 Received: (qmail 31067 invoked by uid 22791); 22 Jun 2010 15:22:53 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.17.166) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 Jun 2010 15:22:42 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.1/8.13.1) with ESMTP id o5MFMdCC028169 for ; Tue, 22 Jun 2010 15:22:39 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o5MFMcJq1740832 for ; Tue, 22 Jun 2010 17:22:38 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id o5MFMcD7013282 for ; Tue, 22 Jun 2010 17:22:38 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id o5MFMbsB013248; Tue, 22 Jun 2010 17:22:37 +0200 Message-Id: <201006221522.o5MFMbsB013248@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Tue, 22 Jun 2010 17:22:37 +0200 Subject: Re: [rfc] Problems with software single-step and SPU breakpoints during fork To: pedro@codesourcery.com (Pedro Alves) Date: Tue, 22 Jun 2010 15:22:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <201006220149.13756.pedro@codesourcery.com> from "Pedro Alves" at Jun 22, 2010 01:49:13 AM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-06/txt/msg00481.txt.bz2 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 . +# +# Contributed by Ulrich Weigand . +# +# 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 . + + Contributed by Ulrich Weigand */ + +#include +#include +#include +#include +#include +#include +#include + +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 . + + Contributed by Ulrich Weigand */ + +#include + +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