Re: [patch] "single step" atomic instruction sequences as a whole.

[Luis Machado <luisgpm@linux.vnet.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

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

[-- Attachment #2: single_stepping.diff --]
[-- Type: text/x-patch, Size: 15467 bytes --]

2007-04-12  Luis Machado  <luisgpm@br.ibm.com>

	* 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;
 }
 
 \f
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 @@
 
 \f
 
-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;
 }