From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30206 invoked by alias); 12 Apr 2007 12:09:37 -0000 Received: (qmail 30194 invoked by uid 22791); 12 Apr 2007 12:09:35 -0000 X-Spam-Check-By: sourceware.org Received: from igw2.br.ibm.com (HELO igw2.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 12 Apr 2007 13:09:30 +0100 Received: from mailhub1.br.ibm.com (mailhub1 [9.18.232.109]) by igw2.br.ibm.com (Postfix) with ESMTP id 40E4E5BEE2 for ; Thu, 12 Apr 2007 09:02:55 -0300 (BRT) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.18.232.46]) by mailhub1.br.ibm.com (8.13.8/8.13.8/NCO v8.3) with ESMTP id l3CC9PdB1507500 for ; Thu, 12 Apr 2007 09:09:25 -0300 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id l3CC7OIU012297 for ; Thu, 12 Apr 2007 09:07:24 -0300 Received: from dyn531804.br.ibm.com (dyn531804.br.ibm.com [9.18.238.71]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id l3CC7Omx012292; Thu, 12 Apr 2007 09:07:24 -0300 Subject: Re: [patch] "single step" atomic instruction sequences as a whole. From: Luis Machado Reply-To: luisgpm@linux.vnet.ibm.com To: Daniel Jacobowitz Cc: gdb-patches@sourceware.org In-Reply-To: <20070410204010.GB2056@caradoc.them.org> References: <1173997454.4772.27.camel@localhost> <20070410204010.GB2056@caradoc.them.org> Content-Type: multipart/mixed; boundary="=-HO3zulTm2m7GAPp7zSmo" Date: Thu, 12 Apr 2007 12:09:00 -0000 Message-Id: <1176379764.4434.16.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.6.1 X-IsSubscribed: yes 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: 2007-04/txt/msg00160.txt.bz2 --=-HO3zulTm2m7GAPp7zSmo Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 1036 Daniel, Thanks for your reply. The syntax problem was corrected. I've ran the gdb_mbuild.sh script and had no problems that are directly related to my patch. I had a compilation problem with the spu-elf, but that seems to be in cvs-HEAD as well. I also changed the return type of the software single step method for spu-elf and the local return values from the method ("return" to "return 1"), since they were still void. The wince.c file modification was droped due to its removal from the tree. > Could you post a patch which changes the type of the > software_single_step gdbarch method to return int, updates infrun.c, > and nothing else? Then we can look at the PowerPC bits (which are > more interesting) separately. Attached follows the patch that only changes the infrun bits and also the return type of the software_single_step methods for various archs. > When you're editing a bunch of tdep files, please use the > gdb_mbuild.sh script to make sure they all still compile. Thanks for the hint on this. Regards, Luis --=-HO3zulTm2m7GAPp7zSmo Content-Disposition: attachment; filename=single_stepping.diff Content-Type: text/x-patch; name=single_stepping.diff; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 15467 2007-04-12 Luis Machado * gdbarch.sh: Change the return type of software_single_step from void to int and reformatted some comments to <= 80 columns. * gdbarch.c, gdbarch.h: Regenerated. * alpha-tdep.c (alpha_software_single_step): Change the return type from void to int and always return 1. * alpha-tdep.h: Change the return type of alpha_software_single_step from void to int. * arm-tdep.c (arm_software_single_step): Change the return type from void to int and always return 1. * cris-tdep.c (cris_software_single_step): Change the return type from void to int and always return 1. * mips-tdep.c (mips_software_single_step): Change the return type from void to int and always return 1. * mips-tdep.h: Change the return type of mips_software_single_step from void to int. * rs6000-tdep.c (rs6000_software_single_step): Change the return type from void to int and always return 1. *rs6000-tdep.h: Change the return type of rs6000_software_single_step from void to int. * sparc-tdep.c (sparc_software_single_step): Change the return type from void to int and always return 1. * sparc-tdep.h: Change the return type of sparc_software_single_step from void to int. * spu-tdep.c (spu_software_single_step): Change the return type from void to int and always return 1. infrun.c (resume): Check the return value from SOFTWARE_SINGLE_STEP and act accordingly. True means that the software_single_step breakpoints where inserted; false means they where not. Index: alpha-tdep.c =================================================================== --- alpha-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ alpha-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -1518,7 +1518,7 @@ return (pc + ALPHA_INSN_SIZE); } -void +int alpha_software_single_step (enum target_signal sig, int insert_breakpoints_p) { static CORE_ADDR next_pc; @@ -1536,6 +1536,7 @@ remove_single_step_breakpoints (); write_pc (next_pc); } + return 1; } Index: alpha-tdep.h =================================================================== --- alpha-tdep.h.orig 2007-04-11 07:20:42.000000000 -0700 +++ alpha-tdep.h 2007-04-11 07:20:44.000000000 -0700 @@ -107,7 +107,7 @@ }; extern unsigned int alpha_read_insn (CORE_ADDR pc); -extern void alpha_software_single_step (enum target_signal, int); +extern int alpha_software_single_step (enum target_signal, int); extern CORE_ADDR alpha_after_prologue (CORE_ADDR pc); extern void alpha_mdebug_init_abi (struct gdbarch_info, struct gdbarch *); Index: arm-tdep.c =================================================================== --- arm-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ arm-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -1907,7 +1907,7 @@ single_step() is also called just after the inferior stops. If we had set up a simulated single-step, we undo our damage. */ -static void +static int arm_software_single_step (enum target_signal sig, int insert_bpt) { /* NOTE: This may insert the wrong breakpoint instruction when @@ -1922,6 +1922,8 @@ } else remove_single_step_breakpoints (); + + return 1; } #include "bfd-in2.h" Index: cris-tdep.c =================================================================== --- cris-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ cris-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -2119,7 +2119,7 @@ digs through the opcodes in order to find all possible targets. Either one ordinary target or two targets for branches may be found. */ -static void +static int cris_software_single_step (enum target_signal ignore, int insert_breakpoints) { inst_env_type inst_env; @@ -2152,6 +2152,8 @@ } else remove_single_step_breakpoints (); + + return 1; } /* Calculates the prefix value for quick offset addressing mode. */ Index: gdbarch.c =================================================================== --- gdbarch.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ gdbarch.c 2007-04-11 07:20:44.000000000 -0700 @@ -3289,14 +3289,14 @@ return gdbarch->software_single_step != NULL; } -void +int gdbarch_software_single_step (struct gdbarch *gdbarch, enum target_signal sig, int insert_breakpoints_p) { gdb_assert (gdbarch != NULL); gdb_assert (gdbarch->software_single_step != NULL); if (gdbarch_debug >= 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_software_single_step called\n"); - gdbarch->software_single_step (sig, insert_breakpoints_p); + return gdbarch->software_single_step (sig, insert_breakpoints_p); } void Index: gdbarch.h =================================================================== --- gdbarch.h.orig 2007-04-11 07:20:42.000000000 -0700 +++ gdbarch.h 2007-04-11 07:20:44.000000000 -0700 @@ -1144,14 +1144,16 @@ #define SMASH_TEXT_ADDRESS(addr) (gdbarch_smash_text_address (current_gdbarch, addr)) #endif -/* FIXME/cagney/2001-01-18: This should be split in two. A target method that indicates if - the target needs software single step. An ISA method to implement it. +/* FIXME/cagney/2001-01-18: This should be split in two. A target method that + indicates if the target needs software single step. An ISA method to + implement it. - FIXME/cagney/2001-01-18: This should be replaced with something that inserts breakpoints - using the breakpoint system instead of blatting memory directly (as with rs6000). + FIXME/cagney/2001-01-18: This should be replaced with something that inserts + breakpoints using the breakpoint system instead of blatting memory directly + (as with rs6000). - FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the target can - single step. If not, then implement single step using breakpoints. */ + FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the + target can single step. If not, then implement single step using breakpoints. */ #if defined (SOFTWARE_SINGLE_STEP) /* Legacy for systems yet to multi-arch SOFTWARE_SINGLE_STEP */ @@ -1168,8 +1170,8 @@ #define SOFTWARE_SINGLE_STEP_P() (gdbarch_software_single_step_p (current_gdbarch)) #endif -typedef void (gdbarch_software_single_step_ftype) (enum target_signal sig, int insert_breakpoints_p); -extern void gdbarch_software_single_step (struct gdbarch *gdbarch, enum target_signal sig, int insert_breakpoints_p); +typedef int (gdbarch_software_single_step_ftype) (enum target_signal sig, int insert_breakpoints_p); +extern int gdbarch_software_single_step (struct gdbarch *gdbarch, enum target_signal sig, int insert_breakpoints_p); extern void set_gdbarch_software_single_step (struct gdbarch *gdbarch, gdbarch_software_single_step_ftype *software_single_step); #if !defined (GDB_TM_FILE) && defined (SOFTWARE_SINGLE_STEP) #error "Non multi-arch definition of SOFTWARE_SINGLE_STEP" Index: gdbarch.sh =================================================================== --- gdbarch.sh.orig 2007-04-11 07:20:42.000000000 -0700 +++ gdbarch.sh 2007-04-11 07:20:44.000000000 -0700 @@ -614,15 +614,19 @@ # It is not at all clear why SMASH_TEXT_ADDRESS is not folded into # ADDR_BITS_REMOVE. f:=:CORE_ADDR:smash_text_address:CORE_ADDR addr:addr::core_addr_identity::0 -# FIXME/cagney/2001-01-18: This should be split in two. A target method that indicates if -# the target needs software single step. An ISA method to implement it. + +# FIXME/cagney/2001-01-18: This should be split in two. A target method that +# indicates if the target needs software single step. An ISA method to +# implement it. # -# FIXME/cagney/2001-01-18: This should be replaced with something that inserts breakpoints -# using the breakpoint system instead of blatting memory directly (as with rs6000). +# FIXME/cagney/2001-01-18: This should be replaced with something that inserts +# breakpoints using the breakpoint system instead of blatting memory directly +# (as with rs6000). # -# FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the target can -# single step. If not, then implement single step using breakpoints. -F:=:void:software_single_step:enum target_signal sig, int insert_breakpoints_p:sig, insert_breakpoints_p +# FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the +# target can single step. If not, then implement single step using breakpoints. +F:=:int:software_single_step:enum target_signal sig, int insert_breakpoints_p:sig, insert_breakpoints_p + # Return non-zero if the processor is executing a delay slot and a # further single-step is needed before the instruction finishes. M::int:single_step_through_delay:struct frame_info *frame:frame Index: infrun.c =================================================================== --- infrun.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ infrun.c 2007-04-11 07:20:44.000000000 -0700 @@ -548,14 +548,16 @@ if (SOFTWARE_SINGLE_STEP_P () && step) { /* Do it the hard way, w/temp breakpoints */ - SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ ); - /* ...and don't ask hardware to do it. */ - step = 0; - /* and do not pull these breakpoints until after a `wait' in - `wait_for_inferior' */ - singlestep_breakpoints_inserted_p = 1; - singlestep_ptid = inferior_ptid; - singlestep_pc = read_pc (); + if (SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ )) + { + /* ...and don't ask hardware to do it. */ + step = 0; + /* and do not pull these breakpoints until after a `wait' in + `wait_for_inferior' */ + singlestep_breakpoints_inserted_p = 1; + singlestep_ptid = inferior_ptid; + singlestep_pc = read_pc (); + } } /* If there were any forks/vforks/execs that were caught and are @@ -1378,7 +1380,7 @@ (LONGEST) ecs->ws.value.integer)); gdb_flush (gdb_stdout); target_mourn_inferior (); - singlestep_breakpoints_inserted_p = 0; /*SOFTWARE_SINGLE_STEP_P() */ + singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */ stop_print_frame = 0; stop_stepping (ecs); return; @@ -1398,7 +1400,7 @@ target_mourn_inferior (); print_stop_reason (SIGNAL_EXITED, stop_signal); - singlestep_breakpoints_inserted_p = 0; /*SOFTWARE_SINGLE_STEP_P() */ + singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */ stop_stepping (ecs); return; @@ -1569,7 +1571,7 @@ if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: stepping_past_singlestep_breakpoint\n"); /* Pull the single step breakpoints out of the target. */ - SOFTWARE_SINGLE_STEP (0, 0); + (void) SOFTWARE_SINGLE_STEP (0, 0); singlestep_breakpoints_inserted_p = 0; ecs->random_signal = 0; @@ -1678,7 +1680,7 @@ if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) { /* Pull the single step breakpoints out of the target. */ - SOFTWARE_SINGLE_STEP (0, 0); + (void) SOFTWARE_SINGLE_STEP (0, 0); singlestep_breakpoints_inserted_p = 0; } @@ -1749,7 +1751,7 @@ if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p) { /* Pull the single step breakpoints out of the target. */ - SOFTWARE_SINGLE_STEP (0, 0); + (void) SOFTWARE_SINGLE_STEP (0, 0); singlestep_breakpoints_inserted_p = 0; } Index: mips-tdep.c =================================================================== --- mips-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ mips-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -2204,7 +2204,7 @@ single_step is also called just after the inferior stops. If we had set up a simulated single-step, we undo our damage. */ -void +int mips_software_single_step (enum target_signal sig, int insert_breakpoints_p) { CORE_ADDR pc, next_pc; @@ -2218,6 +2218,8 @@ } else remove_single_step_breakpoints (); + + return 1; } /* Test whether the PC points to the return instruction at the Index: mips-tdep.h =================================================================== --- mips-tdep.h.orig 2007-04-11 07:20:42.000000000 -0700 +++ mips-tdep.h 2007-04-11 07:20:44.000000000 -0700 @@ -100,7 +100,7 @@ }; /* Single step based on where the current instruction will take us. */ -extern void mips_software_single_step (enum target_signal, int); +extern int mips_software_single_step (enum target_signal, int); /* Tell if the program counter value in MEMADDR is in a MIPS16 function. */ Index: rs6000-tdep.h =================================================================== --- rs6000-tdep.h.orig 2007-04-11 07:20:42.000000000 -0700 +++ rs6000-tdep.h 2007-04-11 07:20:44.000000000 -0700 @@ -21,8 +21,8 @@ #include "defs.h" -extern void rs6000_software_single_step (enum target_signal signal, - int insert_breakpoints_p); +extern int rs6000_software_single_step (enum target_signal signal, + int insert_breakpoints_p); /* Hook in rs6000-tdep.c for determining the TOC address when calling functions in the inferior. */ Index: sparc-tdep.c =================================================================== --- sparc-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ sparc-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -1176,7 +1176,7 @@ return 0; } -void +int sparc_software_single_step (enum target_signal sig, int insert_breakpoints_p) { struct gdbarch *arch = current_gdbarch; @@ -1206,6 +1206,8 @@ } else remove_single_step_breakpoints (); + + return 1; } static void Index: sparc-tdep.h =================================================================== --- sparc-tdep.h.orig 2007-04-11 07:20:42.000000000 -0700 +++ sparc-tdep.h 2007-04-11 07:20:44.000000000 -0700 @@ -167,8 +167,8 @@ -extern void sparc_software_single_step (enum target_signal sig, - int insert_breakpoints_p); +extern int sparc_software_single_step (enum target_signal sig, + int insert_breakpoints_p); extern void sparc_supply_rwindow (struct regcache *regcache, CORE_ADDR sp, int regnum); Index: rs6000-tdep.c =================================================================== --- rs6000-tdep.c.orig 2007-04-11 07:20:42.000000000 -0700 +++ rs6000-tdep.c 2007-04-11 07:20:44.000000000 -0700 @@ -722,7 +722,7 @@ /* AIX does not support PT_STEP. Simulate it. */ -void +int rs6000_software_single_step (enum target_signal signal, int insert_breakpoints_p) { @@ -761,6 +761,7 @@ errno = 0; /* FIXME, don't ignore errors! */ /* What errors? {read,write}_memory call error(). */ + return 1; } Index: spu-tdep.c =================================================================== --- spu-tdep.c.orig 2007-04-11 12:53:45.000000000 -0700 +++ spu-tdep.c 2007-04-11 13:07:55.000000000 -0700 @@ -1078,7 +1078,7 @@ /* Software single-stepping support. */ -void +int spu_software_single_step (enum target_signal signal, int insert_breakpoints_p) { if (insert_breakpoints_p) @@ -1093,7 +1093,7 @@ pc = extract_unsigned_integer (buf, 4) & -4; if (target_read_memory (pc, buf, 4)) - return; + return 1; insn = extract_unsigned_integer (buf, 4); /* Next sequential instruction is at PC + 4, except if the current @@ -1125,6 +1125,8 @@ } else remove_single_step_breakpoints (); + + return 1; } --=-HO3zulTm2m7GAPp7zSmo--