public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/4] Remove arm_override_mode
  2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
                   ` (2 preceding siblings ...)
  2016-05-12 16:18 ` [PATCH 2/4] Remove arm_insert_single_step_breakpoint Yao Qi
@ 2016-05-12 16:18 ` Yao Qi
  2016-06-23 13:20   ` Pedro Alves
  2016-06-21  7:34 ` [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
  4 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-05-12 16:18 UTC (permalink / raw)
  To: gdb-patches

This patch removes global variable arm_override_mode.  The rationale is
that if the address is the next address of current pc, we can get the
thumb/arm mode from dest address computed by software single step code.

gdb:

2016-05-12  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c: Include "gdbthread.h".
	(arm_override_mode): Remove.
	(arm_pc_is_thumb): Remove arm_override_mode.  If MEMADDR
	is the dest address of current pc, get thumb mode by dest address.
	(arm_insert_single_step_breakpoint): Remove arm_override_mode
	and cleanup.
---
 gdb/arm-tdep.c | 57 +++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0412f71..d529481 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -55,6 +55,7 @@
 #include "elf/arm.h"
 
 #include "vec.h"
+#include "gdbthread.h"
 
 #include "record.h"
 #include "record-full.h"
@@ -143,13 +144,6 @@ static const char *const arm_mode_strings[] =
 static const char *arm_fallback_mode_string = "auto";
 static const char *arm_force_mode_string = "auto";
 
-/* Internal override of the execution mode.  -1 means no override,
-   0 means override to ARM mode, 1 means override to Thumb mode.
-   The effect is the same as if arm_force_mode has been set by the
-   user (except the internal override has precedence over a user's
-   arm_force_mode override).  */
-static int arm_override_mode = -1;
-
 /* Number of different reg name sets (options).  */
 static int num_disassembly_options;
 
@@ -422,9 +416,46 @@ arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr)
   if (IS_THUMB_ADDR (memaddr))
     return 1;
 
-  /* Respect internal mode override if active.  */
-  if (arm_override_mode != -1)
-    return arm_override_mode;
+  if (target_has_execution && !is_executing (inferior_ptid))
+    {
+      gdb_byte buf[4];
+      struct regcache *regcache = get_current_regcache ();
+
+      /* Check the memory pointed by PC is readable.  */
+      if (target_read_memory (regcache_read_pc (regcache), buf, 4) == 0)
+	{
+	  struct arm_get_next_pcs next_pcs_ctx;
+	  CORE_ADDR pc;
+	  int i;
+	  VEC (CORE_ADDR) *next_pcs = NULL;
+	  struct cleanup *old_chain
+	    = make_cleanup (VEC_cleanup (CORE_ADDR), &next_pcs);
+
+	  arm_get_next_pcs_ctor (&next_pcs_ctx,
+				 &arm_get_next_pcs_ops,
+				 gdbarch_byte_order (gdbarch),
+				 gdbarch_byte_order_for_code (gdbarch),
+				 0,
+				 regcache);
+
+	  next_pcs = arm_get_next_pcs (&next_pcs_ctx);
+
+	  /* If MEMADDR is the next instruction of current pc, do the
+	     software single step computation, and get the thumb mode by
+	     the destination address.  */
+	  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
+	    {
+	      if (UNMAKE_THUMB_ADDR (pc) == memaddr)
+		{
+		  do_cleanups (old_chain);
+
+		  return IS_THUMB_ADDR (pc);
+		}
+	    }
+
+	  do_cleanups (old_chain);
+	}
+    }
 
   /* If the user wants to override the symbol table, let him.  */
   if (strcmp (arm_force_mode_string, "arm") == 0)
@@ -4205,15 +4236,9 @@ arm_insert_single_step_breakpoint (struct gdbarch *gdbarch,
 				   struct address_space *aspace,
 				   CORE_ADDR pc)
 {
-  struct cleanup *old_chain
-    = make_cleanup_restore_integer (&arm_override_mode);
-
-  arm_override_mode = IS_THUMB_ADDR (pc);
   pc = gdbarch_addr_bits_remove (gdbarch, pc);
 
   insert_single_step_breakpoint (gdbarch, aspace, pc);
-
-  do_cleanups (old_chain);
 }
 
 /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
  2016-05-12 16:18 ` [PATCH 3/4] Clear addr bit in next_pcs vector Yao Qi
@ 2016-05-12 16:18 ` Yao Qi
  2016-05-12 16:18 ` [PATCH 2/4] Remove arm_insert_single_step_breakpoint Yao Qi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-05-12 16:18 UTC (permalink / raw)
  To: gdb-patches

This patch changes gdbarch method software_single_step to return a
vector of addresses on which GDB should insert breakpoints, and don't
insert breakpoints.  Instead, the caller of
gdbarch_software_single_step inserts breakpoints if the returned
vector is not NULL.

gdb:

2016-05-12  Yao Qi  <yao.qi@linaro.org>

	* aarch64-tdep.c (aarch64_software_single_step): Return
	VEC (CORE_ADDR) *.  Return NULL instead of 0.  Don't call
	insert_single_step_breakpoint.
	* alpha-tdep.c (alpha_deal_with_atomic_sequence): Likewise.
	(alpha_software_single_step): Likewise.
	* alpha-tdep.h (alpha_software_single_step): Update declaration.
	* arm-linux-tdep.c (arm_linux_software_single_step): Return
	VEC (CORE_ADDR) *.  Return NULL instead of 0.
	* arm-tdep.c (arm_software_single_step): Return NULL instead of	0.
	* arm-tdep.h (arm_software_single_step): Update declaration.
	* breakpoint.c (insert_single_step_breakpoints): New function.
	* breakpoint.h (insert_single_step_breakpoints): Declare.
	* cris-tdep.c (cris_software_single_step): Return
	VEC (CORE_ADDR) *.  Don't call insert_single_step_breakpoint.
	* gdbarch.sh (software_single_step): Change it to return
	VEC (CORE_ADDR) *.
	* gdbarch.c, gdbarch.h: Regenerated.
	* infrun.c (maybe_software_singlestep): Adjust.
	* mips-tdep.c (mips_deal_with_atomic_sequence): Return
	VEC (CORE_ADDR) *.  Don't call insert_single_step_breakpoint.
	(micromips_deal_with_atomic_sequence): Likewise.
	(deal_with_atomic_sequence): Likewise.
	(mips_software_single_step): Likewise.
	* mips-tdep.h (mips_software_single_step): Update declaration.
	* moxie-tdep.c (moxie_software_single_step): Likewise.
	* nios2-tdep.c (nios2_software_single_step): Likewise.
	* ppc-tdep.h (ppc_deal_with_atomic_sequence): Update
	declaration.
	* record-full.c (record_full_resume): Adjust.
	(record_full_wait_1): Likewise.
	* rs6000-aix-tdep.c (rs6000_software_single_step): Return
	VEC (CORE_ADDR) *.  Don't call insert_single_step_breakpoint.
	* rs6000-tdep.c	(ppc_deal_with_atomic_sequence): Return
	VEC (CORE_ADDR) *.  Don't call insert_single_step_breakpoint.
	* s390-linux-tdep.c (s390_software_single_step): Likewise.
	* sparc-tdep.c (sparc_software_single_step): Likewise.
	* spu-tdep.c (spu_software_single_step): Likewise.
	* tic6x-tdep.c (tic6x_software_single_step): Likewise.
---
 gdb/aarch64-tdep.c    | 18 ++++++++--------
 gdb/alpha-tdep.c      | 28 ++++++++++++-------------
 gdb/alpha-tdep.h      |  2 +-
 gdb/arm-linux-tdep.c  | 12 ++++-------
 gdb/arm-tdep.c        | 10 +++------
 gdb/arm-tdep.h        |  2 +-
 gdb/breakpoint.c      | 27 ++++++++++++++++++++++++
 gdb/breakpoint.h      |  6 ++++++
 gdb/cris-tdep.c       | 13 ++++++------
 gdb/gdbarch.c         |  2 +-
 gdb/gdbarch.h         | 18 ++++++++--------
 gdb/gdbarch.sh        | 16 +++++++-------
 gdb/infrun.c          |  8 +++----
 gdb/mips-tdep.c       | 58 +++++++++++++++++++++++++--------------------------
 gdb/mips-tdep.h       |  2 +-
 gdb/moxie-tdep.c      | 34 +++++++++++++-----------------
 gdb/nios2-tdep.c      |  8 +++----
 gdb/ppc-tdep.h        |  2 +-
 gdb/record-full.c     | 18 ++++------------
 gdb/rs6000-aix-tdep.c | 12 ++++++-----
 gdb/rs6000-tdep.c     | 12 +++++------
 gdb/s390-linux-tdep.c | 16 +++++++-------
 gdb/sparc-tdep.c      |  9 ++++----
 gdb/spu-tdep.c        | 13 ++++++------
 gdb/tic6x-tdep.c      |  9 ++++----
 25 files changed, 182 insertions(+), 173 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 6f2e38e..52c9ea6 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2224,11 +2224,10 @@ value_of_aarch64_user_reg (struct frame_info *frame, const void *baton)
 /* Implement the "software_single_step" gdbarch method, needed to
    single step through atomic sequences on AArch64.  */
 
-static int
+static VEC (CORE_ADDR) *
 aarch64_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
   const int insn_size = 4;
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
@@ -2243,13 +2242,14 @@ aarch64_software_single_step (struct frame_info *frame)
   int bc_insn_count = 0; /* Conditional branch instruction count.  */
   int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */
   aarch64_inst inst;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   if (aarch64_decode_insn (insn, &inst, 1) != 0)
-    return 0;
+    return NULL;
 
   /* Look for a Load Exclusive instruction which begins the sequence.  */
   if (inst.opcode->iclass != ldstexcl || bit (insn, 22) == 0)
-    return 0;
+    return NULL;
 
   for (insn_count = 0; insn_count < atomic_sequence_length; ++insn_count)
     {
@@ -2258,14 +2258,14 @@ aarch64_software_single_step (struct frame_info *frame)
 					   byte_order_for_code);
 
       if (aarch64_decode_insn (insn, &inst, 1) != 0)
-	return 0;
+	return NULL;
       /* Check if the instruction is a conditional branch.  */
       if (inst.opcode->iclass == condbranch)
 	{
 	  gdb_assert (inst.operands[0].type == AARCH64_OPND_ADDR_PCREL19);
 
 	  if (bc_insn_count >= 1)
-	    return 0;
+	    return NULL;
 
 	  /* It is, so we'll try to set a breakpoint at the destination.  */
 	  breaks[1] = loc + inst.operands[0].imm.value;
@@ -2284,7 +2284,7 @@ aarch64_software_single_step (struct frame_info *frame)
 
   /* We didn't find a closing Store Exclusive instruction, fall back.  */
   if (!closing_insn)
-    return 0;
+    return NULL;
 
   /* Insert breakpoint after the end of the atomic sequence.  */
   breaks[0] = loc + insn_size;
@@ -2299,9 +2299,9 @@ aarch64_software_single_step (struct frame_info *frame)
   /* Insert the breakpoint at the end of the sequence, and one at the
      destination of the conditional branch, if it exists.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    VEC_safe_push (CORE_ADDR, next_pcs, breaks[index]);
 
-  return 1;
+  return next_pcs;
 }
 
 struct displaced_step_closure
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 0206214..c944b6a 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -768,11 +768,10 @@ static const int stq_c_opcode = 0x2f;
    is found, attempt to step through it.  A breakpoint is placed at the end of 
    the sequence.  */
 
-static int 
+static VEC (CORE_ADDR) *
 alpha_deal_with_atomic_sequence (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   CORE_ADDR pc = get_frame_pc (frame);
   CORE_ADDR breaks[2] = {-1, -1};
   CORE_ADDR loc = pc;
@@ -783,11 +782,12 @@ alpha_deal_with_atomic_sequence (struct frame_info *frame)
   int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */  
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
   int bc_insn_count = 0; /* Conditional branch instruction count.  */
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   /* Assume all atomic sequences start with a LDL_L/LDQ_L instruction.  */
   if (INSN_OPCODE (insn) != ldl_l_opcode
       && INSN_OPCODE (insn) != ldq_l_opcode)
-    return 0;
+    return NULL;
 
   /* Assume that no atomic sequence is longer than "atomic_sequence_length" 
      instructions.  */
@@ -806,8 +806,8 @@ alpha_deal_with_atomic_sequence (struct frame_info *frame)
 	  immediate = (immediate ^ 0x400000) - 0x400000;
 
 	  if (bc_insn_count >= 1)
-	    return 0; /* More than one branch found, fallback 
-			 to the standard single-step code.  */
+	    return NULL; /* More than one branch found, fallback 
+			    to the standard single-step code.  */
 
 	  breaks[1] = loc + ALPHA_INSN_SIZE + immediate;
 
@@ -823,7 +823,7 @@ alpha_deal_with_atomic_sequence (struct frame_info *frame)
   /* Assume that the atomic sequence ends with a STL_C/STQ_C instruction.  */
   if (INSN_OPCODE (insn) != stl_c_opcode
       && INSN_OPCODE (insn) != stq_c_opcode)
-    return 0;
+    return NULL;
 
   closing_insn = loc;
   loc += ALPHA_INSN_SIZE;
@@ -838,11 +838,10 @@ alpha_deal_with_atomic_sequence (struct frame_info *frame)
 	  || (breaks[1] >= pc && breaks[1] <= closing_insn)))
     last_breakpoint = 0;
 
-  /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    VEC_safe_push (CORE_ADDR, next_pcs, breaks[index]);
 
-  return 1;
+  return next_pcs;
 }
 
 \f
@@ -1721,18 +1720,17 @@ alpha_next_pc (struct frame_info *frame, CORE_ADDR pc)
   return (pc + ALPHA_INSN_SIZE);
 }
 
-int
+VEC (CORE_ADDR) *
 alpha_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
-  CORE_ADDR pc, next_pc;
+  CORE_ADDR pc;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   pc = get_frame_pc (frame);
-  next_pc = alpha_next_pc (frame, pc);
 
-  insert_single_step_breakpoint (gdbarch, aspace, next_pc);
-  return 1;
+  VEC_safe_push (CORE_ADDR, next_pcs, alpha_next_pc (frame, pc));
+  return next_pcs;
 }
 
 \f
diff --git a/gdb/alpha-tdep.h b/gdb/alpha-tdep.h
index abeb326..5b64861 100644
--- a/gdb/alpha-tdep.h
+++ b/gdb/alpha-tdep.h
@@ -103,7 +103,7 @@ struct gdbarch_tdep
 };
 
 extern unsigned int alpha_read_insn (struct gdbarch *gdbarch, CORE_ADDR pc);
-extern int alpha_software_single_step (struct frame_info *frame);
+extern VEC (CORE_ADDR) *alpha_software_single_step (struct frame_info *frame);
 extern CORE_ADDR alpha_after_prologue (CORE_ADDR pc);
 
 extern void alpha_mdebug_init_abi (struct gdbarch_info, struct gdbarch *);
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 4820c7d..80d5888 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -923,12 +923,11 @@ arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self)
 
 /* Insert a single step breakpoint at the next executed instruction.  */
 
-static int
+static VEC (CORE_ADDR) *
 arm_linux_software_single_step (struct frame_info *frame)
 {
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  struct address_space *aspace = get_regcache_aspace (regcache);
   struct arm_get_next_pcs next_pcs_ctx;
   CORE_ADDR pc;
   int i;
@@ -938,7 +937,7 @@ arm_linux_software_single_step (struct frame_info *frame)
   /* If the target does have hardware single step, GDB doesn't have
      to bother software single step.  */
   if (target_can_do_single_step () == 1)
-    return 0;
+    return NULL;
 
   old_chain = make_cleanup (VEC_cleanup (CORE_ADDR), &next_pcs);
 
@@ -957,12 +956,9 @@ arm_linux_software_single_step (struct frame_info *frame)
       VEC_replace (CORE_ADDR, next_pcs, i, pc);
     }
 
-  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
-    insert_single_step_breakpoint (gdbarch, aspace, pc);
-
-  do_cleanups (old_chain);
+  discard_cleanups (old_chain);
 
-  return 1;
+  return next_pcs;
 }
 
 /* Support for displaced stepping of Linux SVC instructions.  */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index fca9f52..3e7f5ca 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6278,12 +6278,11 @@ arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self)
    single-step support.  We find the target of the coming instructions
    and breakpoint them.  */
 
-int
+VEC (CORE_ADDR) *
 arm_software_single_step (struct frame_info *frame)
 {
   struct regcache *regcache = get_current_regcache ();
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  struct address_space *aspace = get_regcache_aspace (regcache);
   struct arm_get_next_pcs next_pcs_ctx;
   CORE_ADDR pc;
   int i;
@@ -6305,12 +6304,9 @@ arm_software_single_step (struct frame_info *frame)
       VEC_replace (CORE_ADDR, next_pcs, i, pc);
     }
 
-  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
-    insert_single_step_breakpoint (gdbarch, aspace, pc);
-
-  do_cleanups (old_chain);
+  discard_cleanups (old_chain);
 
-  return 1;
+  return next_pcs;
 }
 
 /* Cleanup/copy SVC (SWI) instructions.  These two functions are overridden
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 2eecfed..10ab742 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -259,7 +259,7 @@ CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 
 int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
 
-int arm_software_single_step (struct frame_info *);
+VEC (CORE_ADDR) *arm_software_single_step (struct frame_info *);
 int arm_is_thumb (struct regcache *regcache);
 int arm_frame_is_thumb (struct frame_info *frame);
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a39a15c..acddf27 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15186,6 +15186,33 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
   update_global_location_list (UGLL_INSERT);
 }
 
+/* Insert single step breakpoints according to the current state.  */
+
+int
+insert_single_step_breakpoints (struct gdbarch *gdbarch)
+{
+  struct frame_info *frame = get_current_frame ();
+  VEC (CORE_ADDR) * next_pcs;
+
+  next_pcs = gdbarch_software_single_step (gdbarch, frame);
+
+  if (next_pcs != NULL)
+    {
+      int i;
+      CORE_ADDR pc;
+      struct address_space *aspace = get_frame_address_space (frame);
+
+      for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
+	insert_single_step_breakpoint (gdbarch, aspace, pc);
+
+      VEC_free (CORE_ADDR, next_pcs);
+
+      return 1;
+    }
+  else
+    return 0;
+}
+
 /* See breakpoint.h.  */
 
 int
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 054eab4d..8985e07 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1522,6 +1522,12 @@ extern void delete_command (char *arg, int from_tty);
 extern void insert_single_step_breakpoint (struct gdbarch *,
 					   struct address_space *, 
 					   CORE_ADDR);
+
+/* Insert all software single step breakpoints for the current frame.
+   Return true if any software single step breakpoints are inserted,
+   otherwise, return false.  */
+extern int insert_single_step_breakpoints (struct gdbarch *);
+
 /* Check if any hardware watchpoints have triggered, according to the
    target.  */
 int watchpoints_triggered (struct target_waitstatus *);
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index d350ce8..97b9ee5 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -2068,12 +2068,12 @@ find_step_target (struct frame_info *frame, inst_env_type *inst_env)
    digs through the opcodes in order to find all possible targets.
    Either one ordinary target or two targets for branches may be found.  */
 
-static int
+static VEC (CORE_ADDR) *
 cris_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   inst_env_type inst_env;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   /* Analyse the present instruction environment and insert 
      breakpoints.  */
@@ -2090,18 +2090,19 @@ cris_software_single_step (struct frame_info *frame)
          and possibly another one for a branch, jump, etc.  */
       CORE_ADDR next_pc
 	= (CORE_ADDR) inst_env.reg[gdbarch_pc_regnum (gdbarch)];
-      insert_single_step_breakpoint (gdbarch, aspace, next_pc);
+
+      VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
       if (inst_env.branch_found 
 	  && (CORE_ADDR) inst_env.branch_break_address != next_pc)
 	{
 	  CORE_ADDR branch_target_address
 		= (CORE_ADDR) inst_env.branch_break_address;
-	  insert_single_step_breakpoint (gdbarch,
-					 aspace, branch_target_address);
+
+	  VEC_safe_push (CORE_ADDR, next_pcs, branch_target_address);
 	}
     }
 
-  return 1;
+  return next_pcs;
 }
 
 /* Calculates the prefix value for quick offset addressing mode.  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..e59a93d 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -3066,7 +3066,7 @@ gdbarch_software_single_step_p (struct gdbarch *gdbarch)
   return gdbarch->software_single_step != NULL;
 }
 
-int
+VEC (CORE_ADDR) *
 gdbarch_software_single_step (struct gdbarch *gdbarch, struct frame_info *frame)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 859ba85..e026c0e 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -649,18 +649,18 @@ extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_
    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.
   
-   A return value of 1 means that the software_single_step breakpoints
-   were inserted; 0 means they were not.  Multiple breakpoints may be
-   inserted for some instructions such as conditional branch.  However,
-   each implementation must always evaluate the condition and only put
-   the breakpoint at the branch destination if the condition is true, so
-   that we ensure forward progress when stepping past a conditional
-   branch to self. */
+   Return a vector of addresses on which the software single step
+   breakpoints are inserted.  NULL means software single step is not used.
+   Multiple breakpoints may be inserted for some instructions such as
+   conditional branch.  However, each implementation must always evaluate
+   the condition and only put the breakpoint at the branch destination if
+   the condition is true, so that we ensure forward progress when stepping
+   past a conditional branch to self. */
 
 extern int gdbarch_software_single_step_p (struct gdbarch *gdbarch);
 
-typedef int (gdbarch_software_single_step_ftype) (struct frame_info *frame);
-extern int gdbarch_software_single_step (struct gdbarch *gdbarch, struct frame_info *frame);
+typedef VEC (CORE_ADDR) * (gdbarch_software_single_step_ftype) (struct frame_info *frame);
+extern VEC (CORE_ADDR) * gdbarch_software_single_step (struct gdbarch *gdbarch, struct frame_info *frame);
 extern void set_gdbarch_software_single_step (struct gdbarch *gdbarch, gdbarch_software_single_step_ftype *software_single_step);
 
 /* Return non-zero if the processor is executing a delay slot and a
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index c8787c2..b144c04 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -608,14 +608,14 @@ m:CORE_ADDR:addr_bits_remove:CORE_ADDR addr:addr::core_addr_identity::0
 # 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.
 #
-# A return value of 1 means that the software_single_step breakpoints
-# were inserted; 0 means they were not.  Multiple breakpoints may be
-# inserted for some instructions such as conditional branch.  However,
-# each implementation must always evaluate the condition and only put
-# the breakpoint at the branch destination if the condition is true, so
-# that we ensure forward progress when stepping past a conditional
-# branch to self.
-F:int:software_single_step:struct frame_info *frame:frame
+# Return a vector of addresses on which the software single step
+# breakpoints are inserted.  NULL means software single step is not used.
+# Multiple breakpoints may be inserted for some instructions such as
+# conditional branch.  However, each implementation must always evaluate
+# the condition and only put the breakpoint at the branch destination if
+# the condition is true, so that we ensure forward progress when stepping
+# past a conditional branch to self.
+F:VEC (CORE_ADDR) *:software_single_step:struct frame_info *frame:frame
 
 # Return non-zero if the processor is executing a delay slot and a
 # further single-step is needed before the instruction finishes.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cfb1d06..9dc7917 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2263,11 +2263,9 @@ maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc)
   int hw_step = 1;
 
   if (execution_direction == EXEC_FORWARD
-      && gdbarch_software_single_step_p (gdbarch)
-      && gdbarch_software_single_step (gdbarch, get_current_frame ()))
-    {
-      hw_step = 0;
-    }
+      && gdbarch_software_single_step_p (gdbarch))
+    hw_step = !insert_single_step_breakpoints (gdbarch);
+
   return hw_step;
 }
 
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6098f71..8bb9cae 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -3883,9 +3883,8 @@ mips_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
 #define SC_OPCODE 0x38
 #define SCD_OPCODE 0x3c
 
-static int
-mips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
- 				struct address_space *aspace, CORE_ADDR pc)
+static VEC (CORE_ADDR) *
+mips_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
   CORE_ADDR breaks[2] = {-1, -1};
   CORE_ADDR loc = pc;
@@ -3895,11 +3894,12 @@ mips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
   int index;
   int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed).  */  
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   insn = mips_fetch_instruction (gdbarch, ISA_MIPS, loc, NULL);
   /* Assume all atomic sequences start with a ll/lld instruction.  */
   if (itype_op (insn) != LL_OPCODE && itype_op (insn) != LLD_OPCODE)
-    return 0;
+    return NULL;
 
   /* Assume that no atomic sequence is longer than "atomic_sequence_length" 
      instructions.  */
@@ -3963,7 +3963,7 @@ mips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
 
   /* Assume that the atomic sequence ends with a sc/scd instruction.  */
   if (itype_op (insn) != SC_OPCODE && itype_op (insn) != SCD_OPCODE)
-    return 0;
+    return NULL;
 
   loc += MIPS_INSN32_SIZE;
 
@@ -3977,14 +3977,13 @@ mips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
 
   /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    VEC_safe_push (CORE_ADDR, next_pcs, breaks[index]);
 
-  return 1;
+  return next_pcs;
 }
 
-static int
+static VEC (CORE_ADDR) *
 micromips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
-				     struct address_space *aspace,
 				     CORE_ADDR pc)
 {
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
@@ -3997,16 +3996,17 @@ micromips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
   ULONGEST insn;
   int insn_count;
   int index;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   /* Assume all atomic sequences start with a ll/lld instruction.  */
   insn = mips_fetch_instruction (gdbarch, ISA_MICROMIPS, loc, NULL);
   if (micromips_op (insn) != 0x18)	/* POOL32C: bits 011000 */
-    return 0;
+    return NULL;
   loc += MIPS_INSN16_SIZE;
   insn <<= 16;
   insn |= mips_fetch_instruction (gdbarch, ISA_MICROMIPS, loc, NULL);
   if ((b12s4_op (insn) & 0xb) != 0x3)	/* LL, LLD: bits 011000 0x11 */
-    return 0;
+    return NULL;
   loc += MIPS_INSN16_SIZE;
 
   /* Assume all atomic sequences end with an sc/scd instruction.  Assume
@@ -4103,24 +4103,24 @@ micromips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
 	          && b5s5_op (insn) != 0x18)
 				/* JRADDIUSP: bits 010001 11000 */
 	        break;
-	      return 0; /* Fall back to the standard single-step code. */
+	      return NULL; /* Fall back to the standard single-step code. */
 
 	    case 0x33: /* B16: bits 110011 */
-	      return 0; /* Fall back to the standard single-step code. */
+	      return NULL; /* Fall back to the standard single-step code. */
 	    }
 	  break;
 	}
       if (is_branch)
 	{
 	  if (last_breakpoint >= 1)
-	    return 0; /* More than one branch found, fallback to the
+	    return NULL; /* More than one branch found, fallback to the
 			 standard single-step code.  */
 	  breaks[1] = branch_bp;
 	  last_breakpoint++;
 	}
     }
   if (!sc_found)
-    return 0;
+    return NULL;
 
   /* Insert a breakpoint right after the end of the atomic sequence.  */
   breaks[0] = loc;
@@ -4132,21 +4132,20 @@ micromips_deal_with_atomic_sequence (struct gdbarch *gdbarch,
 
   /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    VEC_safe_push (CORE_ADDR, next_pcs, breaks[index]);
 
-  return 1;
+  return next_pcs;
 }
 
-static int
-deal_with_atomic_sequence (struct gdbarch *gdbarch,
-			   struct address_space *aspace, CORE_ADDR pc)
+static VEC (CORE_ADDR) *
+deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
   if (mips_pc_is_mips (pc))
-    return mips_deal_with_atomic_sequence (gdbarch, aspace, pc);
+    return mips_deal_with_atomic_sequence (gdbarch, pc);
   else if (mips_pc_is_micromips (gdbarch, pc))
-    return micromips_deal_with_atomic_sequence (gdbarch, aspace, pc);
+    return micromips_deal_with_atomic_sequence (gdbarch, pc);
   else
-    return 0;
+    return NULL;
 }
 
 /* mips_software_single_step() is called just before we want to resume
@@ -4154,21 +4153,22 @@ deal_with_atomic_sequence (struct gdbarch *gdbarch,
    or kernel single-step support (MIPS on GNU/Linux for example).  We find
    the target of the coming instruction and breakpoint it.  */
 
-int
+VEC (CORE_ADDR) *
 mips_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   CORE_ADDR pc, next_pc;
+  VEC (CORE_ADDR) *next_pcs;
 
   pc = get_frame_pc (frame);
-  if (deal_with_atomic_sequence (gdbarch, aspace, pc))
-    return 1;
+  next_pcs = deal_with_atomic_sequence (gdbarch, pc);
+  if (next_pcs != NULL)
+    return next_pcs;
 
   next_pc = mips_next_pc (frame, pc);
 
-  insert_single_step_breakpoint (gdbarch, aspace, next_pc);
-  return 1;
+  VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
+  return next_pcs;
 }
 
 /* Test whether the PC points to the return instruction at the
diff --git a/gdb/mips-tdep.h b/gdb/mips-tdep.h
index 4e547b4..8a870aa 100644
--- a/gdb/mips-tdep.h
+++ b/gdb/mips-tdep.h
@@ -154,7 +154,7 @@ enum
 };
 
 /* Single step based on where the current instruction will take us.  */
-extern int mips_software_single_step (struct frame_info *frame);
+extern VEC (CORE_ADDR) *mips_software_single_step (struct frame_info *frame);
 
 /* Strip the ISA (compression) bit off from ADDR.  */
 extern CORE_ADDR mips_unmake_compact_addr (CORE_ADDR addr);
diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c
index 714734d..b7f3e41 100644
--- a/gdb/moxie-tdep.c
+++ b/gdb/moxie-tdep.c
@@ -306,11 +306,10 @@ moxie_process_readu (CORE_ADDR addr, gdb_byte *buf,
 
 /* Insert a single step breakpoint.  */
 
-static int
+static VEC (CORE_ADDR) *
 moxie_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   CORE_ADDR addr;
   gdb_byte buf[4];
   uint16_t inst;
@@ -318,6 +317,7 @@ moxie_software_single_step (struct frame_info *frame)
   ULONGEST fp;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct regcache *regcache = get_current_regcache ();
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   addr = get_frame_pc (frame);
 
@@ -345,8 +345,9 @@ moxie_software_single_step (struct frame_info *frame)
 	    case 0x09: /* bleu */
 	      /* Insert breaks on both branches, because we can't currently tell
 		 which way things will go.  */
-	      insert_single_step_breakpoint (gdbarch, aspace, addr + 2);
-	      insert_single_step_breakpoint (gdbarch, aspace, addr + 2 + INST2OFFSET(inst));
+	      VEC_safe_push (CORE_ADDR, next_pcs, addr + 2);
+	      VEC_safe_push (CORE_ADDR, next_pcs,
+			     addr + 2 + INST2OFFSET(inst));
 	      break;
 	    default:
 	      {
@@ -358,7 +359,7 @@ moxie_software_single_step (struct frame_info *frame)
       else
 	{
 	  /* This is a Form 2 instruction.  They are all 16 bits.  */
-	  insert_single_step_breakpoint (gdbarch, aspace, addr + 2);
+	  VEC_safe_push (CORE_ADDR, next_pcs, addr + 2);
 	}
     }
   else
@@ -405,7 +406,7 @@ moxie_software_single_step (struct frame_info *frame)
 	case 0x32: /* udiv.l */
 	case 0x33: /* mod.l */
 	case 0x34: /* umod.l */
-	  insert_single_step_breakpoint (gdbarch, aspace, addr + 2);
+	  VEC_safe_push (CORE_ADDR, next_pcs, addr + 2);
 	  break;
 
 	  /* 32-bit instructions.  */
@@ -415,7 +416,7 @@ moxie_software_single_step (struct frame_info *frame)
 	case 0x37: /* sto.b */
 	case 0x38: /* ldo.s */
 	case 0x39: /* sto.s */
-	  insert_single_step_breakpoint (gdbarch, aspace, addr + 4);
+	  VEC_safe_push (CORE_ADDR, next_pcs, addr + 4);
 	  break;
 
 	  /* 48-bit instructions.  */
@@ -428,32 +429,27 @@ moxie_software_single_step (struct frame_info *frame)
 	case 0x20: /* ldi.s (immediate) */
 	case 0x22: /* lda.s */
 	case 0x24: /* sta.s */
-	  insert_single_step_breakpoint (gdbarch, aspace, addr + 6);
+	  VEC_safe_push (CORE_ADDR, next_pcs, addr + 6);
 	  break;
 
 	  /* Control flow instructions.	 */
 	case 0x03: /* jsra */
 	case 0x1a: /* jmpa */
-	  insert_single_step_breakpoint (gdbarch, aspace,
-					 moxie_process_readu (addr + 2,
-							      buf, 4,
-							      byte_order));
+	  VEC_safe_push (CORE_ADDR, next_pcs,
+			 moxie_process_readu (addr + 2, buf, 4, byte_order));
 	  break;
 
 	case 0x04: /* ret */
 	  regcache_cooked_read_unsigned (regcache, MOXIE_FP_REGNUM, &fp);
-	  insert_single_step_breakpoint (gdbarch, aspace,
-					 moxie_process_readu (fp + 4,
-							      buf, 4,
-							      byte_order));
+	  VEC_safe_push (CORE_ADDR, next_pcs,
+			 moxie_process_readu (fp + 4, buf, 4, byte_order));
 	  break;
 
 	case 0x19: /* jsr */
 	case 0x25: /* jmp */
 	  regcache_raw_read (regcache,
 			     (inst >> 4) & 0xf, (gdb_byte *) & tmpu32);
-	  insert_single_step_breakpoint (gdbarch, aspace,
-					 tmpu32);
+	  VEC_safe_push (CORE_ADDR, next_pcs, tmpu32);
 	  break;
 
 	case 0x30: /* swi */
@@ -463,7 +459,7 @@ moxie_software_single_step (struct frame_info *frame)
 	}
     }
 
-  return 1;
+  return next_pcs;
 }
 
 /* Implement the "read_pc" gdbarch method.  */
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 9d92c55..ce23caf 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -2205,16 +2205,16 @@ nios2_get_next_pc (struct frame_info *frame, CORE_ADDR pc)
 
 /* Implement the software_single_step gdbarch method.  */
 
-static int
+static VEC (CORE_ADDR) *
 nios2_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   CORE_ADDR next_pc = nios2_get_next_pc (frame, get_frame_pc (frame));
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
-  insert_single_step_breakpoint (gdbarch, aspace, next_pc);
+  VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
 
-  return 1;
+  return next_pcs;
 }
 
 /* Implement the get_longjump_target gdbarch method.  */
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index 4a1cb0f..0249456 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -76,7 +76,7 @@ int ppc_altivec_support_p (struct gdbarch *gdbarch);
 /* Return non-zero if the architecture described by GDBARCH has
    VSX registers (vsr0 --- vsr63).  */
 int vsx_support_p (struct gdbarch *gdbarch);
-int ppc_deal_with_atomic_sequence (struct frame_info *frame);
+VEC (CORE_ADDR) *ppc_deal_with_atomic_sequence (struct frame_info *frame);
 
 
 /* Register set description.  */
diff --git a/gdb/record-full.c b/gdb/record-full.c
index f307b48..fc282a2 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -976,17 +976,7 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
                   record_full_resume_step = 1;
                 }
               else
-                {
-                  /* This is a continue.
-                     Try to insert a soft single step breakpoint.  */
-                  if (!gdbarch_software_single_step (gdbarch,
-                                                     get_current_frame ()))
-                    {
-                      /* This system don't want use soft single step.
-                         Use hard sigle step.  */
-                      step = 1;
-                    }
-                }
+		step = !insert_single_step_breakpoints (gdbarch);
             }
         }
 
@@ -1159,9 +1149,9 @@ record_full_wait_1 (struct target_ops *ops,
 			     If insert success, set step to 0.  */
 			  set_executing (inferior_ptid, 0);
 			  reinit_frame_cache ();
-			  if (gdbarch_software_single_step (gdbarch,
-                                                            get_current_frame ()))
-			    step = 0;
+
+			  step = !insert_single_step_breakpoints (gdbarch);
+
 			  set_executing (inferior_ptid, 1);
 			}
 
diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c
index f5e5489..6578e7a 100644
--- a/gdb/rs6000-aix-tdep.c
+++ b/gdb/rs6000-aix-tdep.c
@@ -671,7 +671,7 @@ branch_dest (struct frame_info *frame, int opcode, int instr,
 
 /* AIX does not support PT_STEP.  Simulate it.  */
 
-static int
+static VEC (CORE_ADDR) *
 rs6000_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
@@ -681,13 +681,15 @@ rs6000_software_single_step (struct frame_info *frame)
   CORE_ADDR loc;
   CORE_ADDR breaks[2];
   int opcode;
+  VEC (CORE_ADDR) *next_pcs;
 
   loc = get_frame_pc (frame);
 
   insn = read_memory_integer (loc, 4, byte_order);
 
-  if (ppc_deal_with_atomic_sequence (frame))
-    return 1;
+  next_pcs = ppc_deal_with_atomic_sequence (frame);
+  if (next_pcs != NULL)
+    return next_pcs;
   
   breaks[0] = loc + PPC_INSN_SIZE;
   opcode = insn >> 26;
@@ -702,12 +704,12 @@ rs6000_software_single_step (struct frame_info *frame)
       /* ignore invalid breakpoint.  */
       if (breaks[ii] == -1)
 	continue;
-      insert_single_step_breakpoint (gdbarch, aspace, breaks[ii]);
+      VEC_safe_push (CORE_ADDR, next_pcs, breaks[ii]);
     }
 
   errno = 0;			/* FIXME, don't ignore errors!  */
   /* What errors?  {read,write}_memory call error().  */
-  return 1;
+  return next_pcs;
 }
 
 /* Implement the "auto_wide_charset" gdbarch method for this platform.  */
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 2460eb5..26c8ed9 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1150,7 +1150,7 @@ ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
    is found, attempt to step through it.  A breakpoint is placed at the end of 
    the sequence.  */
 
-int 
+VEC (CORE_ADDR) *
 ppc_deal_with_atomic_sequence (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
@@ -1167,11 +1167,12 @@ ppc_deal_with_atomic_sequence (struct frame_info *frame)
   const int atomic_sequence_length = 16; /* Instruction sequence length.  */
   int opcode; /* Branch instruction's OPcode.  */
   int bc_insn_count = 0; /* Conditional branch instruction count.  */
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   /* Assume all atomic sequences start with a lwarx/ldarx instruction.  */
   if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
       && (insn & LWARX_MASK) != LDARX_INSTRUCTION)
-    return 0;
+    return NULL;
 
   /* Assume that no atomic sequence is longer than "atomic_sequence_length" 
      instructions.  */
@@ -1209,7 +1210,7 @@ ppc_deal_with_atomic_sequence (struct frame_info *frame)
   /* Assume that the atomic sequence ends with a stwcx/stdcx instruction.  */
   if ((insn & STWCX_MASK) != STWCX_INSTRUCTION
       && (insn & STWCX_MASK) != STDCX_INSTRUCTION)
-    return 0;
+    return NULL;
 
   closing_insn = loc;
   loc += PPC_INSN_SIZE;
@@ -1225,11 +1226,10 @@ ppc_deal_with_atomic_sequence (struct frame_info *frame)
 	  || (breaks[1] >= pc && breaks[1] <= closing_insn)))
     last_breakpoint = 0;
 
-  /* Effectively inserts the breakpoints.  */
   for (index = 0; index <= last_breakpoint; index++)
-    insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
+    VEC_safe_push (CORE_ADDR, next_pcs, breaks[index]);
 
-  return 1;
+  return next_pcs;
 }
 
 
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index fc57592..dd26ba6 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -721,7 +721,7 @@ s390_is_partial_instruction (struct gdbarch *gdbarch, CORE_ADDR loc, int *len)
    process about 4kiB of it each time, leading to O(n**2) memory and time
    complexity.  */
 
-static int
+static VEC (CORE_ADDR) *
 s390_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
@@ -730,33 +730,33 @@ s390_software_single_step (struct frame_info *frame)
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int len;
   uint16_t insn;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   /* Special handling only if recording.  */
   if (!record_full_is_used ())
-    return 0;
+    return NULL;
 
   /* First, match a partial instruction.  */
   if (!s390_is_partial_instruction (gdbarch, loc, &len))
-    return 0;
+    return NULL;
 
   loc += len;
 
   /* Second, look for a branch back to it.  */
   insn = read_memory_integer (loc, 2, byte_order);
   if (insn != 0xa714) /* BRC with mask 1 */
-    return 0;
+    return NULL;
 
   insn = read_memory_integer (loc + 2, 2, byte_order);
   if (insn != (uint16_t) -(len / 2))
-    return 0;
+    return NULL;
 
   loc += 4;
 
   /* Found it, step past the whole thing.  */
+  VEC_safe_push (CORE_ADDR, next_pcs, loc);
 
-  insert_single_step_breakpoint (gdbarch, aspace, loc);
-
-  return 1;
+  return next_pcs;
 }
 
 static int
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 863ef8f..27a70fc 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1605,7 +1605,7 @@ sparc_step_trap (struct frame_info *frame, unsigned long insn)
   return 0;
 }
 
-static int
+static VEC (CORE_ADDR) *
 sparc_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *arch = get_frame_arch (frame);
@@ -1614,6 +1614,7 @@ sparc_software_single_step (struct frame_info *frame)
   CORE_ADDR npc, nnpc;
 
   CORE_ADDR pc, orig_npc;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   pc = get_frame_register_unsigned (frame, tdep->pc_regnum);
   orig_npc = npc = get_frame_register_unsigned (frame, tdep->npc_regnum);
@@ -1621,10 +1622,10 @@ sparc_software_single_step (struct frame_info *frame)
   /* Analyze the instruction at PC.  */
   nnpc = sparc_analyze_control_transfer (frame, pc, &npc);
   if (npc != 0)
-    insert_single_step_breakpoint (arch, aspace, npc);
+    VEC_safe_push (CORE_ADDR, next_pcs, npc);
 
   if (nnpc != 0)
-    insert_single_step_breakpoint (arch, aspace, nnpc);
+    VEC_safe_push (CORE_ADDR, next_pcs, nnpc);
 
   /* Assert that we have set at least one breakpoint, and that
      they're not set at the same spot - unless we're going
@@ -1632,7 +1633,7 @@ sparc_software_single_step (struct frame_info *frame)
   gdb_assert (npc != 0 || nnpc != 0 || orig_npc == 0);
   gdb_assert (nnpc != npc || orig_npc == 0);
 
-  return 1;
+  return next_pcs;
 }
 
 static void
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 8dad5c3..ac8fac8 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1617,17 +1617,17 @@ spu_memory_remove_breakpoint (struct gdbarch *gdbarch,
 
 /* Software single-stepping support.  */
 
-static int
+static VEC (CORE_ADDR) *
 spu_software_single_step (struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR pc, next_pc;
   unsigned int insn;
   int offset, reg;
   gdb_byte buf[4];
   ULONGEST lslr;
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
   pc = get_frame_pc (frame);
 
@@ -1650,8 +1650,7 @@ spu_software_single_step (struct frame_info *frame)
   else
     next_pc = (SPUADDR_ADDR (pc) + 4) & lslr;
 
-  insert_single_step_breakpoint (gdbarch,
-				 aspace, SPUADDR (SPUADDR_SPU (pc), next_pc));
+  VEC_safe_push (CORE_ADDR, next_pcs, SPUADDR (SPUADDR_SPU (pc), next_pc));
 
   if (is_branch (insn, &offset, &reg))
     {
@@ -1681,11 +1680,11 @@ spu_software_single_step (struct frame_info *frame)
 
       target = target & lslr;
       if (target != next_pc)
-	insert_single_step_breakpoint (gdbarch, aspace,
-				       SPUADDR (SPUADDR_SPU (pc), target));
+	VEC_safe_push (CORE_ADDR, next_pcs, SPUADDR (SPUADDR_SPU (pc),
+						     target));
     }
 
-  return 1;
+  return next_pcs;
 }
 
 
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index 4779bf0..54a6515 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -694,16 +694,15 @@ tic6x_get_next_pc (struct frame_info *frame, CORE_ADDR pc)
 
 /* This is the implementation of gdbarch method software_single_step.  */
 
-static int
+static VEC (CORE_ADDR) *
 tic6x_software_single_step (struct frame_info *frame)
 {
-  struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct address_space *aspace = get_frame_address_space (frame);
   CORE_ADDR next_pc = tic6x_get_next_pc (frame, get_frame_pc (frame));
+  VEC (CORE_ADDR) *next_pcs = NULL;
 
-  insert_single_step_breakpoint (gdbarch, aspace, next_pc);
+  VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
 
-  return 1;
+  return next_pcs;
 }
 
 /* This is the implementation of gdbarch method frame_align.  */
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] Remove arm_insert_single_step_breakpoint
  2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
  2016-05-12 16:18 ` [PATCH 3/4] Clear addr bit in next_pcs vector Yao Qi
  2016-05-12 16:18 ` [PATCH 4/4] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
@ 2016-05-12 16:18 ` Yao Qi
  2016-05-12 16:18 ` [PATCH 1/4] Remove arm_override_mode Yao Qi
  2016-06-21  7:34 ` [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
  4 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-05-12 16:18 UTC (permalink / raw)
  To: gdb-patches

This patch is to remove arm_insert_single_step_breakpoint.

gdb:

2016-05-12  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-tdep.c (arm_linux_software_single_step): Don't
	call arm_insert_single_step_breakpoint, call
	insert_single_step_breakpoint instead.
	* arm-tdep.c (arm_insert_single_step_breakpoint): Remove.
	(arm_software_single_step): Don't call
	arm_insert_single_step_breakpoint, call
	insert_single_step_breakpoint instead.
	* arm-tdep.h (arm_insert_single_step_breakpoint): Remove
	declaration.
---
 gdb/arm-linux-tdep.c |  5 ++++-
 gdb/arm-tdep.c       | 19 ++++---------------
 gdb/arm-tdep.h       |  2 --
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 9b68315..0e7cad5 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -952,7 +952,10 @@ arm_linux_software_single_step (struct frame_info *frame)
   next_pcs = arm_get_next_pcs (&next_pcs_ctx);
 
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
-    arm_insert_single_step_breakpoint (gdbarch, aspace, pc);
+    {
+      pc = gdbarch_addr_bits_remove (gdbarch, pc);
+      insert_single_step_breakpoint (gdbarch, aspace, pc);
+    }
 
   do_cleanups (old_chain);
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d529481..c027ad5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4227,20 +4227,6 @@ convert_to_extended (const struct floatformat *fmt, void *dbl, const void *ptr,
 			       &d, dbl);
 }
 
-/* Like insert_single_step_breakpoint, but make sure we use a breakpoint
-   of the appropriate mode (as encoded in the PC value), even if this
-   differs from what would be expected according to the symbol tables.  */
-
-void
-arm_insert_single_step_breakpoint (struct gdbarch *gdbarch,
-				   struct address_space *aspace,
-				   CORE_ADDR pc)
-{
-  pc = gdbarch_addr_bits_remove (gdbarch, pc);
-
-  insert_single_step_breakpoint (gdbarch, aspace, pc);
-}
-
 /* Given BUF, which is OLD_LEN bytes ending at ENDADDR, expand
    the buffer to be NEW_LEN bytes ending at ENDADDR.  Return
    NULL if an error occurs.  BUF is freed.  */
@@ -6314,7 +6300,10 @@ arm_software_single_step (struct frame_info *frame)
   next_pcs = arm_get_next_pcs (&next_pcs_ctx);
 
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
-    arm_insert_single_step_breakpoint (gdbarch, aspace, pc);
+    {
+      pc = gdbarch_addr_bits_remove (gdbarch, pc);
+      insert_single_step_breakpoint (gdbarch, aspace, pc);
+    }
 
   do_cleanups (old_chain);
 
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index e5d13bb..2eecfed 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -259,8 +259,6 @@ CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
 
 int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self);
 
-void arm_insert_single_step_breakpoint (struct gdbarch *,
-					struct address_space *, CORE_ADDR);
 int arm_software_single_step (struct frame_info *);
 int arm_is_thumb (struct regcache *regcache);
 int arm_frame_is_thumb (struct frame_info *frame);
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] Clear addr bit in next_pcs vector
  2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
@ 2016-05-12 16:18 ` Yao Qi
  2016-05-12 16:18 ` [PATCH 4/4] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-05-12 16:18 UTC (permalink / raw)
  To: gdb-patches

This patch is to split the loop of calling gdbarch_addr_bits_remove
and insert_single_step_breakpoint into two loops.

gdb:

2016-05-12  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-tdep.c (arm_linux_software_single_step): Write
	adjusted address back to vector.  Call insert_single_step_breakpoint
	in a new loop.
	* arm-tdep.c (arm_software_single_step): Likewise.
---
 gdb/arm-linux-tdep.c | 5 ++++-
 gdb/arm-tdep.c       | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 0e7cad5..4820c7d 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -954,9 +954,12 @@ arm_linux_software_single_step (struct frame_info *frame)
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
     {
       pc = gdbarch_addr_bits_remove (gdbarch, pc);
-      insert_single_step_breakpoint (gdbarch, aspace, pc);
+      VEC_replace (CORE_ADDR, next_pcs, i, pc);
     }
 
+  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
+    insert_single_step_breakpoint (gdbarch, aspace, pc);
+
   do_cleanups (old_chain);
 
   return 1;
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index c027ad5..fca9f52 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6302,9 +6302,12 @@ arm_software_single_step (struct frame_info *frame)
   for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
     {
       pc = gdbarch_addr_bits_remove (gdbarch, pc);
-      insert_single_step_breakpoint (gdbarch, aspace, pc);
+      VEC_replace (CORE_ADDR, next_pcs, i, pc);
     }
 
+  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
+    insert_single_step_breakpoint (gdbarch, aspace, pc);
+
   do_cleanups (old_chain);
 
   return 1;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) *
@ 2016-05-12 16:18 Yao Qi
  2016-05-12 16:18 ` [PATCH 3/4] Clear addr bit in next_pcs vector Yao Qi
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Yao Qi @ 2016-05-12 16:18 UTC (permalink / raw)
  To: gdb-patches

This is a step toward changing gdbarch software_single_step to
"F:VEC (CORE_ADDR) *:software_single_step:struct regcache *regcache:regcache"
as I mentioned https://sourceware.org/ml/gdb-patches/2015-11/msg00591.html

It isn't trivial to replace frame_info with regcache because it may
break the target that frames have different gdbarch, such as spu, so
I still think about it.  In the mean time, I post this series, to let
software_single_step returns VEC (CORE_ADDR) *, and
displaced_step_hw_singlestep can be removed.

V1 can be found https://sourceware.org/ml/gdb-patches/2016-03/msg00466.html
The main change, compared with V1, is that we finally remove the global
variable arm_override_mode.  During the review to V1, there are two
suggested ways, but they need big changes on gdb target/gdbarch
interfaces.  Finally, I get a simpler change in patch 1.  The rest of
patches are straightforward.

I don't include patch "Remove gdbarch method displaced_step_hw_singlestep"
in this series, because it causes regression on aarch64-linux doing
multi-arch debugging, and I am looking at it.

Regression tested on arm-linux, x86_64-linux and aarch64-linux.

*** BLURB HERE ***

Yao Qi (4):
  Remove arm_override_mode
  Remove arm_insert_single_step_breakpoint
  Clear addr bit in next_pcs vector
  gdbarch software_single_step returns VEC (CORE_ADDR) *

 gdb/aarch64-tdep.c    | 18 +++++------
 gdb/alpha-tdep.c      | 28 ++++++++---------
 gdb/alpha-tdep.h      |  2 +-
 gdb/arm-linux-tdep.c  | 14 +++++----
 gdb/arm-tdep.c        | 83 +++++++++++++++++++++++++++++----------------------
 gdb/arm-tdep.h        |  4 +--
 gdb/breakpoint.c      | 27 +++++++++++++++++
 gdb/breakpoint.h      |  6 ++++
 gdb/cris-tdep.c       | 13 ++++----
 gdb/gdbarch.c         |  2 +-
 gdb/gdbarch.h         | 18 +++++------
 gdb/gdbarch.sh        | 16 +++++-----
 gdb/infrun.c          |  8 ++---
 gdb/mips-tdep.c       | 58 +++++++++++++++++------------------
 gdb/mips-tdep.h       |  2 +-
 gdb/moxie-tdep.c      | 34 ++++++++++-----------
 gdb/nios2-tdep.c      |  8 ++---
 gdb/ppc-tdep.h        |  2 +-
 gdb/record-full.c     | 18 +++--------
 gdb/rs6000-aix-tdep.c | 12 ++++----
 gdb/rs6000-tdep.c     | 12 ++++----
 gdb/s390-linux-tdep.c | 16 +++++-----
 gdb/sparc-tdep.c      |  9 +++---
 gdb/spu-tdep.c        | 13 ++++----
 gdb/tic6x-tdep.c      |  9 +++---
 25 files changed, 231 insertions(+), 201 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
                   ` (3 preceding siblings ...)
  2016-05-12 16:18 ` [PATCH 1/4] Remove arm_override_mode Yao Qi
@ 2016-06-21  7:34 ` Yao Qi
  4 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-06-21  7:34 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

Yao Qi <qiyaoltc@gmail.com> writes:

Hi Pedro,
Your comments to V1 are addressed in V2.  Can you take a look?

> This is a step toward changing gdbarch software_single_step to
> "F:VEC (CORE_ADDR) *:software_single_step:struct regcache *regcache:regcache"
> as I mentioned https://sourceware.org/ml/gdb-patches/2015-11/msg00591.html
>
> It isn't trivial to replace frame_info with regcache because it may
> break the target that frames have different gdbarch, such as spu, so
> I still think about it.  In the mean time, I post this series, to let
> software_single_step returns VEC (CORE_ADDR) *, and
> displaced_step_hw_singlestep can be removed.
>
> V1 can be found https://sourceware.org/ml/gdb-patches/2016-03/msg00466.html
> The main change, compared with V1, is that we finally remove the global
> variable arm_override_mode.  During the review to V1, there are two
> suggested ways, but they need big changes on gdb target/gdbarch
> interfaces.  Finally, I get a simpler change in patch 1.  The rest of
> patches are straightforward.
>
> I don't include patch "Remove gdbarch method displaced_step_hw_singlestep"
> in this series, because it causes regression on aarch64-linux doing
> multi-arch debugging, and I am looking at it.
>
> Regression tested on arm-linux, x86_64-linux and aarch64-linux.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Remove arm_override_mode
  2016-05-12 16:18 ` [PATCH 1/4] Remove arm_override_mode Yao Qi
@ 2016-06-23 13:20   ` Pedro Alves
  2016-07-20 10:17     ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-06-23 13:20 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 05/12/2016 05:18 PM, Yao Qi wrote:
> This patch removes global variable arm_override_mode.  The rationale is
> that if the address is the next address of current pc, we can get the
> thumb/arm mode from dest address computed by software single step code.
> 

If we're still doing something special inside arm_pc_is_thumb
for a particular caller, how's this better than the global?
It looks way more complicated, and fragile.  It looks like the sort
of thing that could break easily, since the function now
behaves differently depending on the currently select thread.  :-/

I liked the plan of going the gdbserver direction of storing the
breakpoint's "len/kind" in the breakpoint location, as a
separate field, instead of encoding it in the address.  Did you
find a problem with that approach?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Remove arm_override_mode
  2016-06-23 13:20   ` Pedro Alves
@ 2016-07-20 10:17     ` Yao Qi
  2016-07-22 10:15       ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-07-20 10:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Thu, Jun 23, 2016 at 2:20 PM, Pedro Alves <palves@redhat.com> wrote:
> On 05/12/2016 05:18 PM, Yao Qi wrote:
>> This patch removes global variable arm_override_mode.  The rationale is
>> that if the address is the next address of current pc, we can get the
>> thumb/arm mode from dest address computed by software single step code.
>>
>
> If we're still doing something special inside arm_pc_is_thumb
> for a particular caller, how's this better than the global?
> It looks way more complicated, and fragile.  It looks like the sort
> of thing that could break easily, since the function now
> behaves differently depending on the currently select thread.  :-/

The reason that I choose this approach is that the alternative (see below)
is quite expensive.

>
> I liked the plan of going the gdbserver direction of storing the
> breakpoint's "len/kind" in the breakpoint location, as a
> separate field, instead of encoding it in the address.  Did you
> find a problem with that approach?
>

If we store the breakpoint's "len/kind" in bp_target_info, we need to compute
"kind" and store it before calling target_insert_breakpoint.  The
"kind" of single
step breakpoint is computed by pc and current state, while the "kind" of other
breakpoints is computed by pc.  It is difficult to get "kind" of
breakpoint for ia64,
but we can get store length of breakpoint to "kind" for ia64,

- call gdbarch_breakpoint_kind_from_current_state or
gdbarch_breakpoint_kind_from_pc to get bl->target_info.placed_size,
- call target_insert_breakpoint
                 |
                +---> remote_insert_breakpoint [1]
                +---> memory_insert_breakpoint
                                 |
                                  '---> gdbarch_memory_insert_breakpoint
                                                 |
                                                +--->
ia64_memory_insert_breakpoint
                                                 |
                                                 +--->
default_memory_insert_breakpoint [2]
                                                              |
                                                              '-->
gdbarch_sw_breakpoint_from_kind

in this way, three gdbarch methods, breakpoint_kind_from_pc,
sw_breakpoint_from_kind, breakpoint_kind_from_current_state, and one
gdbarch method remote_breakpoint_from_pc is removed.  Note that
target_info.placed_size ("kind") is used in [1] and [2].

Further, we can simplify gdbarch method breakpoint_from_pc for most archs,
which can be the default one, while ia64 still uses its own implementation.

const unsigned char *
default_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
int *lenptr)
{
  int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, *pcptr);

  return gdbarch_sw_breakpoint_from_kind (gdbarch, kind, lenptr);
}

Of course, we need to define breakpoint_kind_from_pc and
sw_breakpoint_from_kind for many archs, and switch breakpoint_from_pc to
the default one.

What do you think?  I've already got arm working in this way, but still need
to convert other archs.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Remove arm_override_mode
  2016-07-20 10:17     ` Yao Qi
@ 2016-07-22 10:15       ` Pedro Alves
  2016-07-22 18:17         ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-07-22 10:15 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 07/20/2016 11:17 AM, Yao Qi wrote:
> On Thu, Jun 23, 2016 at 2:20 PM, Pedro Alves <palves@redhat.com> wrote:

>>
>> I liked the plan of going the gdbserver direction of storing the
>> breakpoint's "len/kind" in the breakpoint location, as a
>> separate field, instead of encoding it in the address.  Did you
>> find a problem with that approach?
>>
> 
> If we store the breakpoint's "len/kind" in bp_target_info, we need to compute
> "kind" and store it before calling target_insert_breakpoint.  The
> "kind" of single
> step breakpoint is computed by pc and current state, while the "kind" of other
> breakpoints is computed by pc.  It is difficult to get "kind" of
> breakpoint for ia64,
> but we can get store length of breakpoint to "kind" for ia64,
> 

Now clear to me that if ia64 wanted to support Z/z packets, that it'd
need different breakpoint kinds.  Seems like bp_tgt->shadow_len has the
needed info, and bp_tgt->placed_size (the kind) could be
constant (meaning only one kind).


> - call gdbarch_breakpoint_kind_from_current_state or
> gdbarch_breakpoint_kind_from_pc to get bl->target_info.placed_size,
> - call target_insert_breakpoint
>                  |
>                 +---> remote_insert_breakpoint [1]
>                 +---> memory_insert_breakpoint
>                                  |
>                                   '---> gdbarch_memory_insert_breakpoint
>                                                  |
>                                                 +--->
> ia64_memory_insert_breakpoint
>                                                  |
>                                                  +--->
> default_memory_insert_breakpoint [2]
>                                                               |
>                                                               '-->
> gdbarch_sw_breakpoint_from_kind
> 
> in this way, three gdbarch methods, breakpoint_kind_from_pc,
> sw_breakpoint_from_kind, breakpoint_kind_from_current_state, and one
> gdbarch method remote_breakpoint_from_pc is removed.  Note that
> target_info.placed_size ("kind") is used in [1] and [2].
> 
> Further, we can simplify gdbarch method breakpoint_from_pc for most archs,
> which can be the default one, while ia64 still uses its own implementation.
> 
> const unsigned char *
> default_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
> int *lenptr)
> {
>   int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, *pcptr);
> 
>   return gdbarch_sw_breakpoint_from_kind (gdbarch, kind, lenptr);
> }
> 
> Of course, we need to define breakpoint_kind_from_pc and
> sw_breakpoint_from_kind for many archs, and switch breakpoint_from_pc to
> the default one.
> 
> What do you think?  I've already got arm working in this way, but still need
> to convert other archs.
> 

That sounds good.  If it makes it easier, feel free to post a patch with
only ARM, ia64 and couple other archs, so we can see/discuss the core parts,
before doing the rest of the (seemingly) mechanical arch conversions.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Remove arm_override_mode
  2016-07-22 10:15       ` Pedro Alves
@ 2016-07-22 18:17         ` Pedro Alves
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2016-07-22 18:17 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 07/22/2016 11:15 AM, Pedro Alves wrote:

> Now clear to me that if ia64 wanted to support Z/z packets, that it'd
> need different breakpoint kinds.  Seems like bp_tgt->shadow_len has the
> needed info, and bp_tgt->placed_size (the kind) could be
> constant (meaning only one kind).

Happened to re-read this back now and noticed a typo that changes
meaning...  FAOD, I meant:

 NOT clear to me that if ia64 wanted to support Z/z packets, that it'd
 need different breakpoint kinds.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-22 18:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-12 16:18 [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
2016-05-12 16:18 ` [PATCH 3/4] Clear addr bit in next_pcs vector Yao Qi
2016-05-12 16:18 ` [PATCH 4/4] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
2016-05-12 16:18 ` [PATCH 2/4] Remove arm_insert_single_step_breakpoint Yao Qi
2016-05-12 16:18 ` [PATCH 1/4] Remove arm_override_mode Yao Qi
2016-06-23 13:20   ` Pedro Alves
2016-07-20 10:17     ` Yao Qi
2016-07-22 10:15       ` Pedro Alves
2016-07-22 18:17         ` Pedro Alves
2016-06-21  7:34 ` [PATCH 0/4 V2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).