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

[Emi SUZUKI <emi-suzuki@tjsys.co.jp>]

[-- Attachment #1: Type: Text/Plain, Size: 1443 bytes --]

Luis, 

Thank you for testing and sorry for being late to be back on this.  

From: Luis Machado <luisgpm at linux.vnet.ibm.com>
Subject: Re: [patch] "single step" atomic instruction sequences as a whole.
Date: Sat, 17 Feb 2007 00:23:19 -0200

> 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 have investigeted it and found that the code in my patch cannot
detect the region to skip from the sequence of instructions mentioned
above.  
But, in fact, I have diverted that part of codes from Paul's one :-(
I have been misguided by some useless codes in that patch... Yes, I
should have made myself understand more deeply about its behavior.  

Anyway, I have looked anew at that part of codes and improved it.  
Please try the attached version of patchset, if you don't mind.  
It's for CVS head.  

Best regards, 
-- 
Emi SUZUKI

[-- Attachment #2: rs6000-atomic-sequence_070227.diff --]
[-- Type: Text/Plain, Size: 7612 bytes --]

diff -ruN -x CVS src/gdb/config/rs6000/tm-rs6000.h gdb/gdb/config/rs6000/tm-rs6000.h
--- src/gdb/config/rs6000/tm-rs6000.h	2007-01-10 02:59:05.000000000 +0900
+++ gdb/gdb/config/rs6000/tm-rs6000.h	2007-02-27 21:52:46.000000000 +0900
@@ -90,3 +90,9 @@
 
 extern void (*rs6000_set_host_arch_hook) (int);
 
+extern int rs6000_software_single_step_p (void);
+#ifdef SOFTWARE_SINGLE_STEP_P
+#undef SOFTWARE_SINGLE_STEP_P
+#endif
+#define SOFTWARE_SINGLE_STEP_P() rs6000_software_single_step_p()
+
diff -ruN -x CVS src/gdb/infrun.c gdb/gdb/infrun.c
--- src/gdb/infrun.c	2007-02-27 21:34:23.000000000 +0900
+++ gdb/gdb/infrun.c	2007-02-27 21:28:54.000000000 +0900
@@ -556,15 +556,19 @@
   if (breakpoint_here_p (read_pc ()) == permanent_breakpoint_here)
     SKIP_PERMANENT_BREAKPOINT ();
 
-  if (SOFTWARE_SINGLE_STEP_P () && step)
+  if (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;
+      if (SOFTWARE_SINGLE_STEP_P ())
+	{
+	  /* 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 ();
     }
@@ -1566,8 +1570,6 @@
 
   if (stepping_past_singlestep_breakpoint)
     {
-      gdb_assert (SOFTWARE_SINGLE_STEP_P ()
-		  && singlestep_breakpoints_inserted_p);
       gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid));
       gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid));
 
@@ -1580,9 +1582,13 @@
 	{
 	  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);
-	  singlestep_breakpoints_inserted_p = 0;
+
+	  if (singlestep_breakpoints_inserted_p)
+	    {
+	      /* Pull the single step breakpoints out of the target.  */
+	      SOFTWARE_SINGLE_STEP (0, 0);
+	      singlestep_breakpoints_inserted_p = 0;
+	    }
 
 	  ecs->random_signal = 0;
 
@@ -1616,7 +1622,7 @@
 	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
 	    thread_hop_needed = 1;
 	}
-      else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+      else if (singlestep_breakpoints_inserted_p)
 	{
 	  /* We have not context switched yet, so this should be true
 	     no matter which thread hit the singlestep breakpoint.  */
@@ -1687,7 +1693,7 @@
 	  /* Saw a breakpoint, but it was hit by the wrong thread.
 	     Just continue. */
 
-	  if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+	  if (singlestep_breakpoints_inserted_p)
 	    {
 	      /* Pull the single step breakpoints out of the target. */
 	      SOFTWARE_SINGLE_STEP (0, 0);
@@ -1736,7 +1742,7 @@
 	      return;
 	    }
 	}
-      else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+      else if (singlestep_breakpoints_inserted_p)
 	{
 	  sw_single_step_trap_p = 1;
 	  ecs->random_signal = 0;
@@ -1758,7 +1764,7 @@
 	deprecated_context_hook (pid_to_thread_id (ecs->ptid));
     }
 
-  if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+  if (singlestep_breakpoints_inserted_p)
     {
       /* Pull the single step breakpoints out of the target. */
       SOFTWARE_SINGLE_STEP (0, 0);
diff -ruN -x CVS src/gdb/rs6000-tdep.c gdb/gdb/rs6000-tdep.c
--- src/gdb/rs6000-tdep.c	2007-02-21 12:01:58.000000000 +0900
+++ gdb/gdb/rs6000-tdep.c	2007-02-27 21:00:20.000000000 +0900
@@ -701,6 +701,64 @@
     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))
+
+CORE_ADDR 
+rs6000_deal_with_atomic_sequence (CORE_ADDR pc)
+{
+  CORE_ADDR loc = pc;
+  int insn = read_memory_integer (loc, PPC_INSN_SIZE);
+  int i;
+
+  /* Assume all atomic sequences start with an lwarx instruction. */
+  if ((insn & LWARX_MASK) != LWARX_INSTRUCTION 
+      && (insn & LWARX_MASK) != LDARX_INSTRUCTION)
+     return -1;
+
+  /* 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);
+
+      /* At most one conditional branch instuction appears between 
+	 the lwarx/ldarx and stwcx/stdcx instructions.  But its target 
+	 address should be where the second conditional branch goes 
+	 when the branch is not taken.  */
+      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)
+    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, PPC_INSN_SIZE);
+
+  if ((insn & BC_MASK) != BC_INSTRUCTION)
+    error (_("Tried to step over an atomic sequence of instructions but it did not end as expected."));
+
+  return loc;
+}
+
+/* SOFTWARE_SINGLE_STEP_P */
+int
+rs6000_software_single_step_p (void)
+{
+  return (rs6000_deal_with_atomic_sequence (read_pc ()) != -1);
+}
 
 /* AIX does not support PT_STEP. Simulate it. */
 
@@ -720,15 +778,29 @@
     {
       loc = read_pc ();
 
-      insn = read_memory_integer (loc, 4);
+      /* check if running on an atomic sequence of instructions */
+      breaks[0] = rs6000_deal_with_atomic_sequence (loc);
 
-      breaks[0] = loc + breakp_sz;
-      opcode = insn >> 26;
-      breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);
-
-      /* Don't put two breakpoints on the same address. */
-      if (breaks[1] == breaks[0])
-	breaks[1] = -1;
+      if (breaks[0] != -1)
+	{
+	  printf_unfiltered (_("Stepping over an atomic sequence of instructions. \n\
+Beginning at %s, break at %s next time.\n"),
+			     core_addr_to_string (loc), 
+			     core_addr_to_string (breaks[0]));
+	  gdb_flush (gdb_stdout);
+	  breaks[1] = -1;
+	}
+      else
+	{
+	  insn = read_memory_integer (loc, PPC_INSN_SIZE);
+	  breaks[0] = loc + breakp_sz;
+	  opcode = insn >> 26;
+	  breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);
+
+	  /* Don't put two breakpoints on the same address. */
+	  if (breaks[1] == breaks[0])
+	    breaks[1] = -1;
+	}
 
       for (ii = 0; ii < 2; ++ii)
 	{
@@ -3446,6 +3518,7 @@
 
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, rs6000_breakpoint_from_pc);
+  set_gdbarch_software_single_step (gdbarch, rs6000_software_single_step);
 
   /* Handle the 64-bit SVR4 minimal-symbol convention of using "FN"
      for the descriptor and ".FN" for the entry-point -- a user
@@ -3530,3 +3603,4 @@
   gdbarch_register (bfd_arch_rs6000, rs6000_gdbarch_init, rs6000_dump_tdep);
   gdbarch_register (bfd_arch_powerpc, rs6000_gdbarch_init, rs6000_dump_tdep);
 }
+