Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: [patch] "single step" atomic instruction sequences as a whole.
@ 2007-03-15 22:24 Luis Machado
  2007-04-10 20:40 ` Daniel Jacobowitz
  0 siblings, 1 reply; 44+ messages in thread
From: Luis Machado @ 2007-03-15 22:24 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

This is a modified version of Paul's patch for the single stepping over
atomic instruction sequences.

The main modification is the removal of the portion of code that was
previously added to ppc-linux-tdep.* files. As some people on the
mailing list noticed, it was not really needed. The contents of that
file were transferred to the rs6000-tdep.c file. 

rs6000-tdep.c now handles all the functionality for the single stepping
over locking instruction sets, and a suggested modification was included
in the code in order to fix possible internal errors due to inconsistent
handling of breakpoint creation/removal.

Follows the testsuite runs with and without the patch for HEAD.

Testsuite Summary with the patch applied

# of expected passes            11071
# of unexpected failures        356
# of expected failures          42
# of known failures             41
# of unresolved testcases       3
# of untested testcases         11
# of unsupported tests          18

Testsuite Summary without the patch

# of expected passes            11071
# of unexpected failures        357
# of expected failures          42
# of known failures             41
# of unresolved testcases       3
# of untested testcases         10
# of unsupported tests          18

It appears to have no regressions.

I am aware that Emi is currently working on a similar patch, based on
Paul's also. But since this is an issue that's been rolling for long and
that negatively impacts on more specific debugging activities, wouldn't
it be possible to include this patch as a fix while we wait for Emi's
copyright assignment to be ready? After his copyright assignment is
ready, it would be possible for him to incorporate additional changes
that he wrote on top of this one.

Regards,
Luis

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

20006-06-22  Paul Gilliam  <pgilliam@us.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.
	* wince.c (wince_software_single_step {three times}): 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.

20006-06-22  Paul Gilliam  <pgilliam@us.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.
	* wince.c (wince_software_single_step {three times}): 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
+++ alpha-tdep.c
@@ -1518,7 +1518,7 @@ alpha_next_pc (CORE_ADDR pc)
   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 @@ alpha_software_single_step (enum target_
       remove_single_step_breakpoints ();
       write_pc (next_pc);
     }
+  return 1;
 }
 
 \f
Index: alpha-tdep.h
===================================================================
--- alpha-tdep.h.orig
+++ alpha-tdep.h
@@ -107,7 +107,7 @@ struct gdbarch_tdep
 };
 
 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
+++ arm-tdep.c
@@ -1907,7 +1907,7 @@ arm_get_next_pc (CORE_ADDR pc)
    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 @@ arm_software_single_step (enum target_si
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1
 }
 
 #include "bfd-in2.h"
Index: cris-tdep.c
===================================================================
--- cris-tdep.c.orig
+++ cris-tdep.c
@@ -2119,7 +2119,7 @@ find_step_target (inst_env_type *inst_en
    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 @@ cris_software_single_step (enum target_s
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 /* Calculates the prefix value for quick offset addressing mode.  */
Index: gdbarch.c
===================================================================
--- gdbarch.c.orig
+++ gdbarch.c
@@ -3289,14 +3289,14 @@ gdbarch_software_single_step_p (struct g
   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
+++ gdbarch.h
@@ -1144,14 +1144,16 @@ extern void set_gdbarch_smash_text_addre
 #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 @@ extern int gdbarch_software_single_step_
 #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
+++ gdbarch.sh
@@ -614,15 +614,19 @@ f:=:CORE_ADDR:addr_bits_remove:CORE_ADDR
 # 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
+++ infrun.c
@@ -555,14 +555,16 @@ resume (int step, enum target_signal sig
   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
@@ -1385,7 +1387,7 @@ handle_inferior_event (struct execution_
 					   (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;
@@ -1405,7 +1407,7 @@ handle_inferior_event (struct execution_
       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;
 
@@ -1576,7 +1578,7 @@ handle_inferior_event (struct execution_
 	  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;
@@ -1685,7 +1687,7 @@ handle_inferior_event (struct execution_
 	  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;
 	    }
 
@@ -1756,7 +1758,7 @@ handle_inferior_event (struct execution_
   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
+++ mips-tdep.c
@@ -2204,7 +2204,7 @@ mips_addr_bits_remove (CORE_ADDR addr)
    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 @@ mips_software_single_step (enum target_s
     }
   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
+++ mips-tdep.h
@@ -103,7 +103,7 @@ enum
 };
 
 /* 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.c
===================================================================
--- rs6000-tdep.c.orig
+++ rs6000-tdep.c
@@ -718,10 +718,98 @@ rs6000_breakpoint_from_pc (CORE_ADDR *bp
     return little_breakpoint;
 }
 
+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define LDARX_INSTRUCTION 0x7C000108
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define STDCX_INSTRUCTION 0x7c0001ad
+#define BC_MASK 0xfc000000
+#define BC_INSTRUCTION 0x40000000
+#define IMMEDIATE_PART(insn)  (((insn & ~3) << 16) >> 16)
+#define ABSOLUTE_P(insn) ((int) ((insn >> 1) & 1))
+
+static int 
+deal_with_atomic_sequence (enum target_signal sig, int insert_breakpoints_p)
+{
+  CORE_ADDR pc = read_pc ();
+  CORE_ADDR breaks[2] = {-1, -1};
+  CORE_ADDR loc = pc;
+  int insn = read_memory_integer (loc, PPC_INSN_SIZE);
+  int last_break = 0;
+  int i;
 
-/* AIX does not support PT_STEP. Simulate it. */
+  if (insert_breakpoints_p)
+    {
+        
+  /* Assume all atomic sequences start with an lwarx or ldarx instruction. */
+  if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
+   && (insn & LWARX_MASK) != LDARX_INSTRUCTION)
+     return 0;
+
+  /* Assume that no atomic sequence is longer than 6 instructions. */
+  for (i = 1; i < 5; ++i)
+    {
+      loc += PPC_INSN_SIZE;
+      insn = read_memory_integer (loc, PPC_INSN_SIZE);
+
+      /* Assume at most one conditional branch instruction between
+ 	  the lwarx and stwcx instructions.*/
+      if ((insn & BC_MASK) == BC_INSTRUCTION)
+	    {
+	    last_break = 1;
+	    breaks[1] = IMMEDIATE_PART (insn);
+	    if ( ! ABSOLUTE_P(insn))
+	      breaks[1] += loc;
+	      continue;
+	    }
 
-void
+        if ((insn & STWCX_MASK) == STWCX_INSTRUCTION
+         || (insn & STWCX_MASK) == STDCX_INSTRUCTION)
+	      break;
+    }
+
+  /* Assume that the atomic sequence ends with a stwcx instruction
+       followed by a conditional branch instruction. */
+  if ((insn & STWCX_MASK) != STWCX_INSTRUCTION
+   && (insn & STWCX_MASK) != STDCX_INSTRUCTION)
+    warning (_("Tried to step over an atomic sequence of instructions at %s\n \
+                but could not find the end of the sequence."),
+            core_addr_to_string(pc));
+
+  loc += PPC_INSN_SIZE;
+  insn = read_memory_integer (loc, PPC_INSN_SIZE);
+
+  if ((insn & BC_MASK) != BC_INSTRUCTION)
+    warning (_("Tried to step over an atomic sequence of instructions at %s\n \
+                but the instruction sequence ended in an unexpected way."),
+            core_addr_to_string(pc));
+
+  breaks[0] = loc;
+
+  /* This should never happen, but make sure we don't put
+     two breakpoints on the same address. */
+  if (last_break && breaks[1] == breaks[0])
+    last_break = 0;
+
+  for (i = 0; i <= last_break; ++i)
+    insert_single_step_breakpoint (breaks[i]);
+
+  printf_unfiltered (_("Stepping over an atomic sequence of instructions beginning at %s\n"),
+		     core_addr_to_string (pc));
+
+  gdb_flush (gdb_stdout);
+    }
+  else
+      remove_single_step_breakpoints();
+
+  return 1;
+}
+
+/* AIX does not support PT_STEP. Simulate it, dealing with any sequence of
+   instructions that must be atomic. */
+
+int
 rs6000_software_single_step (enum target_signal signal,
 			     int insert_breakpoints_p)
 {
@@ -739,6 +827,9 @@ rs6000_software_single_step (enum target
 
       insn = read_memory_integer (loc, 4);
 
+      if (deal_with_atomic_sequence (signal, insert_breakpoints_p))
+	return 1;
+
       breaks[0] = loc + breakp_sz;
       opcode = insn >> 26;
       breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);
@@ -760,6 +851,8 @@ rs6000_software_single_step (enum target
 
   errno = 0;			/* FIXME, don't ignore errors! */
   /* What errors?  {read,write}_memory call error().  */
+
+  return 1;
 }
 
 
@@ -3400,6 +3493,9 @@ rs6000_gdbarch_init (struct gdbarch_info
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, rs6000_breakpoint_from_pc);
 
+  /* Watch for locking instruction sequences during single stepping */
+  set_gdbarch_software_single_step(gdbarch, deal_with_atomic_sequence);
+
   /* Handle the 64-bit SVR4 minimal-symbol convention of using "FN"
      for the descriptor and ".FN" for the entry-point -- a user
      specifying "break FN" will unexpectedly end up with a breakpoint
Index: rs6000-tdep.h
===================================================================
--- rs6000-tdep.h.orig
+++ rs6000-tdep.h
@@ -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
+++ sparc-tdep.c
@@ -1176,7 +1176,7 @@ sparc_step_trap (unsigned long insn)
   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 @@ sparc_software_single_step (enum target_
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 static void
Index: sparc-tdep.h
===================================================================
--- sparc-tdep.h.orig
+++ sparc-tdep.h
@@ -167,8 +167,8 @@ extern struct sparc_frame_cache *
 
 \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: wince.c
===================================================================
--- wince.c.orig
+++ wince.c
@@ -839,7 +839,7 @@ undoSStep (thread_info * th)
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -851,14 +851,15 @@ wince_software_single_step (enum target_
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = mips_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif SHx
 /* Renesas SH architecture instruction encoding masks */
@@ -980,7 +981,7 @@ undoSStep (thread_info * th)
    instruction and setting a breakpoint on the "next" instruction
    which would be executed.  This code hails from sh-stub.c.
  */
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -990,13 +991,14 @@ wince_software_single_step (enum target_
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   th->step_pc = sh_get_next_pc (&th->context);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif defined (ARM)
 #undef check_for_step
@@ -1027,7 +1029,7 @@ undoSStep (thread_info * th)
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -1039,14 +1041,15 @@ wince_software_single_step (enum target_
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = arm_get_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #endif
 

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [patch] "single step" atomic instruction sequences as a whole.
@ 2007-02-17  2:24 Luis Machado
  2007-02-27 13:00 ` Emi SUZUKI
  0 siblings, 1 reply; 44+ messages in thread
From: Luis Machado @ 2007-02-17  2:24 UTC (permalink / raw)
  To: gdb-patches

Emi,

I tried your patch against the CVS head. GDB seems to handle this set of
instructions just fine:

0x40000119a00 <._IO_puts+80>:   beq-    cr7,0x40000119a3c <._IO_puts
+140>
0x40000119a04 <._IO_puts+84>:   li      r0,1
0x40000119a08 <._IO_puts+88>:   lwarx   r11,0,r3
0x40000119a0c <._IO_puts+92>:   cmpw    r11,r9
0x40000119a10 <._IO_puts+96>:   bne-    0x40000119a1c <._IO_puts+108>
0x40000119a14 <._IO_puts+100>:  stwcx.  r0,0,r3
0x40000119a18 <._IO_puts+104>:  bne+    0x40000119a08 <._IO_puts+88>
0x40000119a1c <._IO_puts+108>:  isync
0x40000119a20 <._IO_puts+112>:  cmpwi   cr7,r11,0
0x40000119a24 <._IO_puts+116>:  bne-    cr7,0x40000119b84 <._IO_puts
+468>

If i step through "lwarx", i end up with this:

Stepping over an atomic sequence of instructions.
Beginning at 0x0000040000119a08, break at 0x0000040000119a18 next time.
0x0000040000119a18 in ._IO_puts () from /lib64/power5/libc.so.6

So far so good. But if we continue executing the program we will
eventually reach a set of instructions like this one:

0x40000119ae8 <._IO_puts+312>:  lwarx   r9,0,r3
0x40000119aec <._IO_puts+316>:  stwcx.  r0,0,r3
0x40000119af0 <._IO_puts+320>:  bne+    0x40000119ae8 <._IO_puts+312>

At this point, GDB keeps stepping through the lwarx and stwcx
instructions endlessly. If i hit continue right after the first
breakpoint (0x40000119a08), the program ends normally

I tested the same instruction block with Paul's patch and this did not
happen, stating that it skipped the atomic instruction set as usual, and
ending the program.

I'm trying to figure out what is going on exactly.

Regards,
Luis


^ permalink raw reply	[flat|nested] 44+ messages in thread
* [patch] "single step" atomic instruction sequences as a whole.
@ 2007-02-06 11:02 Luis Machado
  2007-02-06 12:11 ` Emi SUZUKI
  0 siblings, 1 reply; 44+ messages in thread
From: Luis Machado @ 2007-02-06 11:02 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

Some months ago Paul Gilliam submitted a set of three patches to fix a single 
stepping problem through atomic instruction sequences on a POWER 
architecture. The problem caused GDB to hang when one tried to single step 
through these instructions. Suppose that you have the following block of 
instructions:

        L1:   lwarx   r11,0,r3
              cmpw    r11,r9
              bne-    L2
              stwcx.  r0,0,r3
              bne-    L1
        L2:   isync

It's not possible to do a single step through each one of these instructions 
since the 'reserve' made by the lwarx instruction will always be lost by the
the time the stwcx instruction has executed.

The patch treats the whole L1/L2 interval as a single instruction, avoiding a 
single step through each one of the instructions contained in the L1/L2 
interval, which would lead GDB to a hang

More details regarding this patch can be found at the original messages.
 
* Patches 1 and 2: http://sourceware.org/ml/gdb-patches/2006-06/msg00339.html
* Patch 3: http://sourceware.org/ml/gdb-patches/2006-06/msg00341.html

I'm re-submitting this patch in order for it to be reviewed. Could this patch 
be applied?

Regards,
Luis

[-- Attachment #2: ppc-atomic.single-step.diff --]
[-- Type: text/x-diff, Size: 3488 bytes --]

20006-06-22  Paul Gilliam  <pgilliam@us.ibm.com>

	* ppc-linux-tdep.c (ppc_atomic_single_step): New function.
	(ppc_linux_init_abi): Set software_single_step member of the gdbarch
	vector to the new ppc_atomic_single_step function.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	22 Jun 2006 18:26:16 -0000
@@ -927,6 +927,84 @@
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 
+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define BC_MASK 0xfc000000
+#define BC_INSTRUCTION 0x40000000
+#define IMMEDIATE_PART(insn)  (((insn & ~3) << 16) >> 16)
+#define ABSOLUTE_P(insn) ((int) ((insn >> 1) & 1))
+
+static int 
+ppc_atomic_single_step (enum target_signal sig, int insert_breakpoints_p)
+{
+  if (insert_breakpoints_p)
+    {
+      CORE_ADDR pc = read_pc ();
+      CORE_ADDR breaks[2] = {-1, -1};
+      CORE_ADDR loc = pc;
+      int insn = read_insn (loc);
+      int last_break = 0;
+      int i;
+
+
+      /* Assume all atomic sequences start with an lwarx instruction. */
+      if ((insn & LWARX_MASK) != LWARX_INSTRUCTION)
+         return 0;
+
+      /* Assume that no atomic sequence is longer than 6 instructions. */
+      for (i= 1; i < 5; ++i)
+	{
+	  loc += PPC_INSN_SIZE;
+	  insn = read_insn (loc);
+
+	  /* Assume at most one conditional branch instruction between
+	     the lwarx and stwcx instructions.*/
+	  if ((insn & BC_MASK) == BC_INSTRUCTION)
+	    {
+	      last_break = 1;
+	      breaks[1] = IMMEDIATE_PART (insn);
+	      if ( ! ABSOLUTE_P(insn))
+		breaks[1] += loc;
+	      continue;
+	    }
+
+	  if ((insn & STWCX_MASK) == STWCX_INSTRUCTION)
+	    break;
+	}
+
+      /* Assume that the atomic sequence ends with a stwcx instruction
+         followed by a conditional branch instruction. */
+      if ((insn & STWCX_MASK) != STWCX_INSTRUCTION)
+	error (_("Tried to step over an atomic sequence of instructions but could not find the end of the sequence."));
+
+      loc += PPC_INSN_SIZE;
+      insn = read_insn (loc);
+
+      if ((insn & BC_MASK) != BC_INSTRUCTION)
+	error (_("Tried to step over an atomic sequence of instructions but it did not end as expected."));
+
+      breaks[0] = loc;
+
+      /* This should never happen, but make sure we don't but
+	 two breakpoints on the same address. */
+      if (last_break && breaks[1] == breaks[0])
+	last_break = 0;
+
+      for (i= 0; i < last_break; ++i)
+	insert_single_step_breakpoint (breaks[i]);
+
+      printf_unfiltered (_("Stepping over an atomic sequence of instructions beginning at %s\n"),
+			 core_addr_to_string (pc));
+      gdb_flush (gdb_stdout);
+    }
+  else
+    remove_single_step_breakpoints ();
+
+  return 1;
+}
+
 static void
 ppc32_linux_sigaction_cache_init (const struct tramp_frame *self,
 				  struct frame_info *next_frame,
@@ -1080,6 +1158,10 @@
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  /* Enable software_single_step in case someone tries to sngle step a
+     sequence of instructions that should be atomic. */
+  set_gdbarch_software_single_step (gdbarch, ppc_atomic_single_step);
 }
 
 void


[-- Attachment #3: rs6000-atomic-single-step.diff --]
[-- Type: text/x-diff, Size: 2855 bytes --]

--- old_rs6000-tdep.c	2006-06-22 13:16:03.000000000 -0700
+++ rs6000-tdep.c	2006-06-22 13:41:10.000000000 -0700
@@ -701,8 +701,81 @@
     return little_breakpoint;
 }
 
+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define BC_MASK 0xfc000000
+#define BC_INSTRUCTION 0x40000000
+#define IMMEDIATE_PART(insn)  (((insn & ~3) << 16) >> 16)
+#define ABSOLUTE_P(insn) ((int) ((insn >> 1) & 1))
+
+static int 
+deal_with_atomic_sequence (enum target_signal sig)
+{
+  CORE_ADDR pc = read_pc ();
+  CORE_ADDR breaks[2] = {-1, -1};
+  CORE_ADDR loc = pc;
+  int insn = read_memory_integer (loc, 4);
+  int last_break = 0;
+  int i;
+
+
+  /* Assume all atomic sequences start with an lwarx instruction. */
+  if ((insn & LWARX_MASK) != LWARX_INSTRUCTION)
+     return 0;
 
-/* AIX does not support PT_STEP. Simulate it. */
+  /* Assume that no atomic sequence is longer than 6 instructions. */
+  for (i= 1; i < 5; ++i)
+    {
+      loc += PPC_INSN_SIZE;
+      insn = read_memory_integer (loc, 4);
+
+      /* Assume at most one conditional branch instruction between
+ 	 the lwarx and stwcx instructions.*/
+      if ((insn & BC_MASK) == BC_INSTRUCTION)
+	{
+	  last_break = 1;
+	  breaks[1] = IMMEDIATE_PART (insn);
+	  if ( ! ABSOLUTE_P(insn))
+	    breaks[1] += loc;
+	    continue;
+	}
+
+      if ((insn & STWCX_MASK) == STWCX_INSTRUCTION)
+	break;
+    }
+
+  /* Assume that the atomic sequence ends with a stwcx instruction
+       followed by a conditional branch instruction. */
+  if ((insn & STWCX_MASK) != STWCX_INSTRUCTION)
+    error (_("Tried to step over an atomic sequence of instructions but could not find the end of the sequence."));
+
+  loc += PPC_INSN_SIZE;
+  insn = read_memory_integer (loc, 4);
+
+  if ((insn & BC_MASK) != BC_INSTRUCTION)
+    error (_("Tried to step over an atomic sequence of instructions but it did not end as expected."));
+
+  breaks[0] = loc;
+
+  /* This should never happen, but make sure we don't but
+     two breakpoints on the same address. */
+  if (last_break && breaks[1] == breaks[0])
+    last_break = 0;
+
+  for (i= 0; i < last_break; ++i)
+    insert_single_step_breakpoint (breaks[i]);
+
+  printf_unfiltered (_("Stepping over an atomic sequence of instructions beginning at %s\n"),
+		     core_addr_to_string (pc));
+  gdb_flush (gdb_stdout);
+
+  return 1;
+}
+
+/* AIX does not support PT_STEP. Simulate it, dealing with any sequence of
+   instructions that must be atomic. */
 
 int
 rs6000_software_single_step (enum target_signal signal,
@@ -722,6 +795,9 @@
 
       insn = read_memory_integer (loc, 4);
 
+      if (deal_with_atomic_sequence (signal))
+	return 1;
+
       breaks[0] = loc + breakp_sz;
       opcode = insn >> 26;
       breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);


[-- Attachment #4: change-software-single-step.diff --]
[-- Type: text/x-diff, Size: 17363 bytes --]

20006-06-22  Paul Gilliam  <pgilliam@us.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.
	* wince.c (wince_software_single_step {three times}): 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
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.c,v
retrieving revision 1.154
diff -a -u -r1.154 alpha-tdep.c
--- alpha-tdep.c	18 Apr 2006 19:20:05 -0000	1.154
+++ alpha-tdep.c	22 Jun 2006 17:49:27 -0000
@@ -1489,7 +1489,7 @@
   return (pc + 4);
 }
 
-void
+int
 alpha_software_single_step (enum target_signal sig, int insert_breakpoints_p)
 {
   static CORE_ADDR next_pc;
@@ -1507,6 +1507,7 @@
       remove_single_step_breakpoints ();
       write_pc (next_pc);
     }
+  return 1;
 }
 
 \f
Index: alpha-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.h,v
retrieving revision 1.23
diff -a -u -r1.23 alpha-tdep.h
--- alpha-tdep.h	17 Dec 2005 22:33:59 -0000	1.23
+++ alpha-tdep.h	22 Jun 2006 17:49:27 -0000
@@ -100,7 +100,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
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.209
diff -a -u -r1.209 arm-tdep.c
--- arm-tdep.c	17 May 2006 14:40:39 -0000	1.209
+++ arm-tdep.c	22 Jun 2006 17:49:27 -0000
@@ -1846,7 +1846,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
@@ -1861,6 +1861,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1
 }
 
 #include "bfd-in2.h"
Index: cris-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/cris-tdep.c,v
retrieving revision 1.136
diff -a -u -r1.136 cris-tdep.c
--- cris-tdep.c	18 Apr 2006 19:20:06 -0000	1.136
+++ cris-tdep.c	22 Jun 2006 17:49:28 -0000
@@ -2117,7 +2117,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;
@@ -2150,6 +2150,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 /* Calculates the prefix value for quick offset addressing mode.  */
Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.329
diff -a -u -r1.329 gdbarch.c
--- gdbarch.c	18 Apr 2006 19:20:06 -0000	1.329
+++ gdbarch.c	22 Jun 2006 17:49:28 -0000
@@ -3318,14 +3318,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
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.285
diff -a -u -r1.285 gdbarch.h
--- gdbarch.h	18 Apr 2006 19:20:06 -0000	1.285
+++ gdbarch.h	22 Jun 2006 17:49:28 -0000
@@ -1165,14 +1165,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 */
@@ -1189,8 +1191,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
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.364
diff -a -u -r1.364 gdbarch.sh
--- gdbarch.sh	18 Apr 2006 19:20:06 -0000	1.364
+++ gdbarch.sh	22 Jun 2006 17:49:28 -0000
@@ -602,15 +602,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
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.211
diff -a -u -r1.211 infrun.c
--- infrun.c	16 Jun 2006 01:12:58 -0000	1.211
+++ infrun.c	22 Jun 2006 17:49:28 -0000
@@ -556,13 +556,15 @@
   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;
+      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;
+	}
     }
 
   /* If there were any forks/vforks/execs that were caught and are
@@ -1375,7 +1377,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;
@@ -1395,7 +1397,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;
 
@@ -1557,7 +1559,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;
@@ -1621,7 +1623,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;
 	    }
 
@@ -1694,7 +1696,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
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.396
diff -a -u -r1.396 mips-tdep.c
--- mips-tdep.c	19 Jun 2006 18:50:09 -0000	1.396
+++ mips-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -2185,7 +2185,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;
@@ -2199,6 +2199,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 /* Test whether the PC points to the return instruction at the
Index: mips-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.h,v
retrieving revision 1.18
diff -a -u -r1.18 mips-tdep.h
--- mips-tdep.h	17 Dec 2005 22:34:01 -0000	1.18
+++ mips-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -103,7 +103,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.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.258
diff -a -u -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -704,7 +704,7 @@
 
 /* AIX does not support PT_STEP. Simulate it. */
 
-void
+int
 rs6000_software_single_step (enum target_signal signal,
 			     int insert_breakpoints_p)
 {
@@ -743,6 +743,8 @@
 
   errno = 0;			/* FIXME, don't ignore errors! */
   /* What errors?  {read,write}_memory call error().  */
+
+  return 1;
 }
 
 
Index: rs6000-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.h,v
retrieving revision 1.1
diff -a -u -r1.1 rs6000-tdep.h
--- rs6000-tdep.h	10 Feb 2006 20:56:14 -0000	1.1
+++ rs6000-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -21,6 +21,6 @@
 
 #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);
 
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.172
diff -a -u -r1.172 sparc-tdep.c
--- sparc-tdep.c	18 Apr 2006 19:20:06 -0000	1.172
+++ sparc-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -1131,7 +1131,7 @@
   return 0;
 }
 
-void
+int
 sparc_software_single_step (enum target_signal sig, int insert_breakpoints_p)
 {
   struct gdbarch *arch = current_gdbarch;
@@ -1161,6 +1161,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 static void
Index: sparc-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.h,v
retrieving revision 1.11
diff -a -u -r1.11 sparc-tdep.h
--- sparc-tdep.h	22 Jan 2006 20:07:38 -0000	1.11
+++ sparc-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -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: wince.c
===================================================================
RCS file: /cvs/src/src/gdb/wince.c,v
retrieving revision 1.45
diff -a -u -r1.45 wince.c
--- wince.c	18 Apr 2006 19:20:06 -0000	1.45
+++ wince.c	22 Jun 2006 17:49:29 -0000
@@ -838,7 +838,7 @@
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -850,14 +850,15 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = mips_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif SHx
 /* Renesas SH architecture instruction encoding masks */
@@ -979,7 +980,7 @@
    instruction and setting a breakpoint on the "next" instruction
    which would be executed.  This code hails from sh-stub.c.
  */
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -989,13 +990,14 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   th->step_pc = sh_get_next_pc (&th->context);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif defined (ARM)
 #undef check_for_step
@@ -1026,7 +1028,7 @@
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -1038,14 +1040,15 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = arm_get_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #endif
 

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re:[patch] "single step" atomic instruction sequences as a whole.
@ 2006-09-18 11:59 emin ak
  2006-11-09 13:07 ` [patch] " emin ak
  0 siblings, 1 reply; 44+ messages in thread
From: emin ak @ 2006-09-18 11:59 UTC (permalink / raw)
  To: gdb-patches

Hi All;
Is there aynone can succesfully apply or care about this patch. I have
the same problem with my powerpc.
I have checked the latest cvs gdb and I have found this patch did'nt
merge into mainline. Will it commit later?
Thanks. Best Regards.
Emin Ak
>Sorry forgot the logs.
>--------------------test function.
>#include <stdio.h>
>#define __KERNEL__
>#include <asm/atomic.h>
>
>atomic_t i;
>int main(void)
>{
>printf("atomic_step_test\n");
>atomic_set(&i,5);
>printf("i=%d\n",atomic_read(&i));
>atomic_dec(&i);
>printf("i=%d\n",atomic_read(&i));
>
>
>}
>----------- gdb logs-----------
>Breakpoint 2, main () at test.c:8
>8       printf("atomic_step_test\n");
>(gdb) next
>9       atomic_set(&i,5);
>(gdb) next
>10      printf("i=%d\n",atomic_read(&i));
>(gdb) next
>151                     :"=m" (v->counter), "=qm" (c)
>(gdb) next
>Stepping over an atomic sequence of instructions beginning at 0x10000484
>breakpoint.c:7730: internal-error: remove_single_step_breakpoints:
>Assertion `single_step_breakpoints[0] != NULL' failed.
>A problem internal to GDB has been detected,
>further debugging may prove unreliable.
>Quit this debugging session? (y or n)
>--------------------------
>Regards.
>Emre
> Hello Mr Gilliam;
> Firstly thank you very much for your patch about single stepping problem
> on atomic instructions. In my opinion, this bug is very critical for
> powerpc arch (especially while kernel debugging). We have experienced this
> problem  with both with h/w probe (BDI2000) and KGDB while debugging
> kernel. I have applied your patch to latest gdb (20060911 taken from cvs)
> and after correctting some fail warnings and test it on a PPC8540. It can
> successfullt detects 'lwarx' instruction but can't step over the loop and
> a little moment later, it gives an error message like 'breakpoint.c:7730:
> internal-error: remove_single_step_breakpoints: Assertion
> `single_step_breakpoints[0] != NULL' failed.'
> I have corrected some patch error, I dont know, maybe this is the reason
> of the problem. Here is the output of gdb and my test program that include
> atomic_dec function.
> Where can be the problem? And thank alot again, because atomic functions
> are everywhere on kernel waiting to cause infinite loops..
> Regards.
> Emre KARA
>
>
>


^ permalink raw reply	[flat|nested] 44+ messages in thread
* [patch] "single step" atomic instruction sequences as a whole.
@ 2006-06-22 20:56 PAUL GILLIAM
  2006-06-22 21:53 ` PAUL GILLIAM
  2006-11-10 21:18 ` Daniel Jacobowitz
  0 siblings, 2 replies; 44+ messages in thread
From: PAUL GILLIAM @ 2006-06-22 20:56 UTC (permalink / raw)
  To: gdb-patches

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

This fixes a problem noticed on ppc64-linux: automatically stepping out
of the 'puts' library function (because it had no line number
information) would cause an endless loop.  This happened because of the
following sequence of instructions:

        L1:   lwarx   r11,0,r3
              cmpw    r11,r9
              bne-    L2
              stwcx.  r0,0,r3
              bne-    L1
        L2:   isync

GDB can not single step this sequence instruction by instruction because
the 'reserve' made by the lwarx instruction will always be lost by the
the time the stwcx instruction has executed.

Other architectures may have similar instruction sequences which must
not be single stepped, one at a time.

To fix this, we must 'single step' these sequences as a whole, using the
same mechanism used by software single step.

Toward that end, I changed the existing software_single_step so that it
would return 1 if it handled the step and 0 if it did not.  All the
existing versions where changed to always return 1.

Then I added a software_single_step routine for ppc-linux.  This
platform did not previous use this function because it has a hardware
single step.  Now, this routine ('ppc_atomic_single_step') checks to see
if the instruction about to be stepped is an stwcx instruction.  If not,
it returns 0 to indicate that the single step has not been handled and
the regular hardware single step should be used.  If the instruction
about to be single stepped was an stwcx instruction, then the code is
scanned looking for the corresponding stwcx instruction, after which a
breakpoint is placed.  If a branch is detected while scanning, a
breakpoint is also placed at the target of that branch.  Then flags are
set as they where for the old software single step and 1 is returned to
indicate that the step has been handled.

I have attached two patches.  'change-software-single-step.diff' makes
the generic changes to all the existing software single step routines:
changing their type from void to int and always returning a 1.

'ppc-atomic-single-step.diff' adds the new ppc_atomic_single_step
routine to ppc-linux-tdep.c and updates the ppc_linux_init_abi routine
to use set_gdbarch_software_single_step() to plug the new routine into
the architecture vector.

You may ask "but what if an architecture needs the old
software_single_step functionality *and* has sequences of instructions
that need to be atomic?"  The answer is easy: do the test for the start
of an atomic sequence: if yes, then scan for its end as above and set
one or two breakpoints; if no, then examine the instruction to see if
it's a branch, setting a breakpoint at the target of the branch if it
is, and setting a breakpoint after the instruction if it is not or if
it's conditional.  In either event the step was handled, so return 1.

You may ask "isn't this going to take a long time, examining every
instruction before it is stepped?"  Well, a little.  But no more then an
architecture without a hardware supported single step which has to
examine every instruction to see if it's a branch.  I did some timing
analysis and the difference was minor.

This was first discussed on the gdb mailing list here:
        http://sourceware.org/ml/gdb/2006-06/msg00048.html
        
OK to commit?

-=# Paul #=- 

[-- Attachment #2: change-software-single-step.diff --]
[-- Type: text/x-patch, Size: 17363 bytes --]

20006-06-22  Paul Gilliam  <pgilliam@us.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.
	* wince.c (wince_software_single_step {three times}): 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
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.c,v
retrieving revision 1.154
diff -a -u -r1.154 alpha-tdep.c
--- alpha-tdep.c	18 Apr 2006 19:20:05 -0000	1.154
+++ alpha-tdep.c	22 Jun 2006 17:49:27 -0000
@@ -1489,7 +1489,7 @@
   return (pc + 4);
 }
 
-void
+int
 alpha_software_single_step (enum target_signal sig, int insert_breakpoints_p)
 {
   static CORE_ADDR next_pc;
@@ -1507,6 +1507,7 @@
       remove_single_step_breakpoints ();
       write_pc (next_pc);
     }
+  return 1;
 }
 
 \f
Index: alpha-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/alpha-tdep.h,v
retrieving revision 1.23
diff -a -u -r1.23 alpha-tdep.h
--- alpha-tdep.h	17 Dec 2005 22:33:59 -0000	1.23
+++ alpha-tdep.h	22 Jun 2006 17:49:27 -0000
@@ -100,7 +100,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
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.209
diff -a -u -r1.209 arm-tdep.c
--- arm-tdep.c	17 May 2006 14:40:39 -0000	1.209
+++ arm-tdep.c	22 Jun 2006 17:49:27 -0000
@@ -1846,7 +1846,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
@@ -1861,6 +1861,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1
 }
 
 #include "bfd-in2.h"
Index: cris-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/cris-tdep.c,v
retrieving revision 1.136
diff -a -u -r1.136 cris-tdep.c
--- cris-tdep.c	18 Apr 2006 19:20:06 -0000	1.136
+++ cris-tdep.c	22 Jun 2006 17:49:28 -0000
@@ -2117,7 +2117,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;
@@ -2150,6 +2150,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 /* Calculates the prefix value for quick offset addressing mode.  */
Index: gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.329
diff -a -u -r1.329 gdbarch.c
--- gdbarch.c	18 Apr 2006 19:20:06 -0000	1.329
+++ gdbarch.c	22 Jun 2006 17:49:28 -0000
@@ -3318,14 +3318,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
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.285
diff -a -u -r1.285 gdbarch.h
--- gdbarch.h	18 Apr 2006 19:20:06 -0000	1.285
+++ gdbarch.h	22 Jun 2006 17:49:28 -0000
@@ -1165,14 +1165,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 */
@@ -1189,8 +1191,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
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.364
diff -a -u -r1.364 gdbarch.sh
--- gdbarch.sh	18 Apr 2006 19:20:06 -0000	1.364
+++ gdbarch.sh	22 Jun 2006 17:49:28 -0000
@@ -602,15 +602,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
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.211
diff -a -u -r1.211 infrun.c
--- infrun.c	16 Jun 2006 01:12:58 -0000	1.211
+++ infrun.c	22 Jun 2006 17:49:28 -0000
@@ -556,13 +556,15 @@
   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;
+      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;
+	}
     }
 
   /* If there were any forks/vforks/execs that were caught and are
@@ -1375,7 +1377,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;
@@ -1395,7 +1397,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;
 
@@ -1557,7 +1559,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;
@@ -1621,7 +1623,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;
 	    }
 
@@ -1694,7 +1696,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
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.396
diff -a -u -r1.396 mips-tdep.c
--- mips-tdep.c	19 Jun 2006 18:50:09 -0000	1.396
+++ mips-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -2185,7 +2185,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;
@@ -2199,6 +2199,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 /* Test whether the PC points to the return instruction at the
Index: mips-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.h,v
retrieving revision 1.18
diff -a -u -r1.18 mips-tdep.h
--- mips-tdep.h	17 Dec 2005 22:34:01 -0000	1.18
+++ mips-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -103,7 +103,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.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.258
diff -a -u -r1.258 rs6000-tdep.c
--- rs6000-tdep.c	23 Apr 2006 14:15:01 -0000	1.258
+++ rs6000-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -704,7 +704,7 @@
 
 /* AIX does not support PT_STEP. Simulate it. */
 
-void
+int
 rs6000_software_single_step (enum target_signal signal,
 			     int insert_breakpoints_p)
 {
@@ -743,6 +743,8 @@
 
   errno = 0;			/* FIXME, don't ignore errors! */
   /* What errors?  {read,write}_memory call error().  */
+
+  return 1;
 }
 
 
Index: rs6000-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.h,v
retrieving revision 1.1
diff -a -u -r1.1 rs6000-tdep.h
--- rs6000-tdep.h	10 Feb 2006 20:56:14 -0000	1.1
+++ rs6000-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -21,6 +21,6 @@
 
 #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);
 
Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.172
diff -a -u -r1.172 sparc-tdep.c
--- sparc-tdep.c	18 Apr 2006 19:20:06 -0000	1.172
+++ sparc-tdep.c	22 Jun 2006 17:49:29 -0000
@@ -1131,7 +1131,7 @@
   return 0;
 }
 
-void
+int
 sparc_software_single_step (enum target_signal sig, int insert_breakpoints_p)
 {
   struct gdbarch *arch = current_gdbarch;
@@ -1161,6 +1161,8 @@
     }
   else
     remove_single_step_breakpoints ();
+
+  return 1;
 }
 
 static void
Index: sparc-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.h,v
retrieving revision 1.11
diff -a -u -r1.11 sparc-tdep.h
--- sparc-tdep.h	22 Jan 2006 20:07:38 -0000	1.11
+++ sparc-tdep.h	22 Jun 2006 17:49:29 -0000
@@ -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: wince.c
===================================================================
RCS file: /cvs/src/src/gdb/wince.c,v
retrieving revision 1.45
diff -a -u -r1.45 wince.c
--- wince.c	18 Apr 2006 19:20:06 -0000	1.45
+++ wince.c	22 Jun 2006 17:49:29 -0000
@@ -838,7 +838,7 @@
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -850,14 +850,15 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = mips_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif SHx
 /* Renesas SH architecture instruction encoding masks */
@@ -979,7 +980,7 @@
    instruction and setting a breakpoint on the "next" instruction
    which would be executed.  This code hails from sh-stub.c.
  */
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -989,13 +990,14 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   th->step_pc = sh_get_next_pc (&th->context);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #elif defined (ARM)
 #undef check_for_step
@@ -1026,7 +1028,7 @@
     }
 }
 
-void
+int
 wince_software_single_step (enum target_signal ignore,
 			    int insert_breakpoints_p)
 {
@@ -1038,14 +1040,15 @@
   if (!insert_breakpoints_p)
     {
       undoSStep (th);
-      return;
+      return 1;
     }
 
   th->stepped = 1;
   pc = read_register (PC_REGNUM);
   th->step_pc = arm_get_next_pc (pc);
   insert_single_step_breakpoint (th->step_pc);
-  return;
+
+  return 1;
 }
 #endif
 

[-- Attachment #3: ppc-atomic-single-step.diff --]
[-- Type: text/x-patch, Size: 3487 bytes --]

20006-06-22  Paul Gilliam  <pgilliam@us.ibm.com>

	* ppc-linux-tdep.c (ppc_atomic_single_step): New function.
	(ppc_linux_init_abi): Set software_single_step member of the gdbarch
	vector to the new ppc_atomic_single_step function.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	22 Jun 2006 18:26:16 -0000
@@ -927,6 +927,84 @@
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 
+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define BC_MASK 0xfc000000
+#define BC_INSTRUCTION 0x40000000
+#define IMMEDIATE_PART(insn)  (((insn & ~3) << 16) >> 16)
+#define ABSOLUTE_P(insn) ((int) ((insn >> 1) & 1))
+
+static int 
+ppc_atomic_single_step (enum target_signal sig, int insert_breakpoints_p)
+{
+  if (insert_breakpoints_p)
+    {
+      CORE_ADDR pc = read_pc ();
+      CORE_ADDR breaks[2] = {-1, -1};
+      CORE_ADDR loc = pc;
+      int insn = read_insn (loc);
+      int last_break = 0;
+      int i;
+
+
+      /* Assume all atomic sequences start with an lwarx instruction. */
+      if ((insn & LWARX_MASK) != LWARX_INSTRUCTION)
+         return 0;
+
+      /* Assume that no atomic sequence is longer than 6 instructions. */
+      for (i= 1; i < 5; ++i)
+	{
+	  loc += PPC_INSN_SIZE;
+	  insn = read_insn (loc);
+
+	  /* Assume at most one conditional branch instruction between
+	     the lwarx and stwcx instructions.*/
+	  if ((insn & BC_MASK) == BC_INSTRUCTION)
+	    {
+	      last_break = 1;
+	      breaks[1] = IMMEDIATE_PART (insn);
+	      if ( ! ABSOLUTE_P(insn))
+		breaks[1] += loc;
+	      continue;
+	    }
+
+	  if ((insn & STWCX_MASK) == STWCX_INSTRUCTION)
+	    break;
+	}
+
+      /* Assume that the atomic sequence ends with a stwcx instruction
+         followed by a conditional branch instruction. */
+      if ((insn & STWCX_MASK) != STWCX_INSTRUCTION)
+	error (_("Tried to step over an atomic sequence of instructions but could not find the end of the sequence."));
+
+      loc += PPC_INSN_SIZE;
+      insn = read_insn (loc);
+
+      if ((insn & BC_MASK) != BC_INSTRUCTION)
+	error (_("Tried to step over an atomic sequence of instructions but it did not end as expected."));
+
+      breaks[0] = loc;
+
+      /* This should never happen, but make sure we don't but
+	 two breakpoints on the same address. */
+      if (last_break && breaks[1] == breaks[0])
+	last_break = 0;
+
+      for (i= 0; i < last_break; ++i)
+	insert_single_step_breakpoint (breaks[i]);
+
+      printf_unfiltered (_("Stepping over an atomic sequence of instructions beginning at %s\n"),
+			 core_addr_to_string (pc));
+      gdb_flush (gdb_stdout);
+    }
+  else
+    remove_single_step_breakpoints ();
+
+  return 1;
+}
+
 static void
 ppc32_linux_sigaction_cache_init (const struct tramp_frame *self,
 				  struct frame_info *next_frame,
@@ -1080,6 +1158,10 @@
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
                                              svr4_fetch_objfile_link_map);
+
+  /* Enable software_single_step in case someone tries to sngle step a
+     sequence of instructions that should be atomic. */
+  set_gdbarch_software_single_step (gdbarch, ppc_atomic_single_step);
 }
 
 void

^ permalink raw reply	[flat|nested] 44+ messages in thread
end of thread, other threads:[~2007-04-14 18:13 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-15 22:24 [patch] "single step" atomic instruction sequences as a whole Luis Machado
2007-04-10 20:40 ` Daniel Jacobowitz
2007-04-12 12:09   ` Luis Machado
2007-04-12 12:15     ` Daniel Jacobowitz
2007-04-12 12:54       ` Luis Machado
2007-04-12 12:58         ` Daniel Jacobowitz
2007-04-12 13:30           ` Luis Machado
2007-04-12 13:35             ` Daniel Jacobowitz
2007-04-12 14:58               ` Ulrich Weigand
2007-04-12 15:33                 ` Daniel Jacobowitz
2007-04-12 17:16                   ` Ulrich Weigand
2007-04-12 18:25                     ` Daniel Jacobowitz
2007-04-12 20:09                       ` Ulrich Weigand
2007-04-12 20:16                         ` Mark Kettenis
2007-04-12 20:43                           ` Ulrich Weigand
2007-04-14 15:20                             ` Mark Kettenis
2007-04-14 18:13                               ` Ulrich Weigand
2007-04-12 20:49                           ` Daniel Jacobowitz
2007-04-12 20:48                         ` Daniel Jacobowitz
2007-04-14 18:50                           ` [commit] Update software_single_step arguments Ulrich Weigand
2007-04-12 14:32     ` [patch] "single step" atomic instruction sequences as a whole Ulrich Weigand
2007-04-12 14:47       ` Luis Machado
2007-04-12 15:00         ` Ulrich Weigand
  -- strict thread matches above, loose matches on Subject: below --
2007-02-17  2:24 Luis Machado
2007-02-27 13:00 ` Emi SUZUKI
2007-02-27 13:17   ` Daniel Jacobowitz
2007-02-28  8:08     ` Emi SUZUKI
2007-02-28 11:46       ` Daniel Jacobowitz
2007-02-28 16:09         ` Luis Machado
2007-03-02 12:47           ` Emi SUZUKI
2007-03-06 11:00             ` Andreas Schwab
2007-03-06 12:24               ` Daniel Jacobowitz
2007-03-08  8:50                 ` Emi SUZUKI
2007-03-08 16:15                   ` Ulrich Weigand
2007-03-13  6:12                     ` SUZUKI Emi
2007-02-06 11:02 Luis Machado
2007-02-06 12:11 ` Emi SUZUKI
2007-02-07 13:10   ` Luis Machado
2007-02-08 13:00     ` Emi SUZUKI
2006-09-18 11:59 emin ak
2006-11-09 13:07 ` [patch] " emin ak
2006-06-22 20:56 PAUL GILLIAM
2006-06-22 21:53 ` PAUL GILLIAM
2006-06-22 22:20   ` PAUL GILLIAM
2006-11-10 21:18 ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox