public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
@ 2016-03-23 14:10 Yao Qi
  2016-03-23 14:10 ` [PATCH 1/2] " Yao Qi
  2016-03-23 14:10 ` [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep Yao Qi
  0 siblings, 2 replies; 10+ messages in thread
From: Yao Qi @ 2016-03-23 14:10 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 (in patch 2).

The whole series are tested on {x86_64, ppc64p7, arm, aarch64}-linux.

*** BLURB HERE ***

Yao Qi (2):
  gdbarch software_single_step returns VEC (CORE_ADDR) *
  Remove gdbarch method displaced_step_hw_singlestep

 gdb/aarch64-linux-tdep.c |  2 --
 gdb/aarch64-tdep.c       | 27 ++++++++--------------
 gdb/alpha-tdep.c         | 28 +++++++++++------------
 gdb/alpha-tdep.h         |  2 +-
 gdb/arch-utils.c         |  7 ------
 gdb/arch-utils.h         |  5 -----
 gdb/arm-linux-tdep.c     | 14 ++++--------
 gdb/arm-tdep.c           | 18 ++++++---------
 gdb/arm-tdep.h           |  4 +---
 gdb/cris-tdep.c          | 13 ++++++-----
 gdb/gdbarch.c            | 48 +++++++++++++++++++--------------------
 gdb/gdbarch.h            | 26 +++++++---------------
 gdb/gdbarch.sh           | 19 +++++-----------
 gdb/infrun.c             | 44 +++++++++++++++++++++++++++++++-----
 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        | 46 +++++++++++++++++++++++++++++++++-----
 gdb/rs6000-aix-tdep.c    | 12 +++++-----
 gdb/rs6000-tdep.c        | 24 ++++++--------------
 gdb/s390-linux-tdep.c    | 25 +++++++--------------
 gdb/sparc-tdep.c         |  9 ++++----
 gdb/spu-tdep.c           | 13 +++++------
 gdb/tic6x-tdep.c         |  9 ++++----
 26 files changed, 246 insertions(+), 253 deletions(-)

-- 
1.9.1

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

* [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-03-23 14:10 [PATCH 0/2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
@ 2016-03-23 14:10 ` Yao Qi
  2016-03-23 17:25   ` Pedro Alves
  2016-03-23 14:10 ` [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep Yao Qi
  1 sibling, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-03-23 14:10 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.

However, breakpoint insertion in arm is a little different, which
uses arm_insert_single_step_breakpoint, and it updates a global
variable arm_override_mode, so that arm_pc_is_thumb can get the
right arm thumb mode even the program is wrong (see
gdb.arch/thumb-singlestep.exp).  I failed to remove global variable
arm_override_mode, so have to add a new gdbarch method
insert_single_step_breakpoint, which is in default
insert_single_step_breakpoint for all targets except arm.

gdb:

2016-03-23  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.  Don't call
	insert_single_step_breakpoint.
	* arm-tdep.c (arm_insert_single_step_breakpoint): Make it
	static.
	(arm_software_single_step): Return NULL instead of 0.  Don't
	call arm_insert_single_step_breakpoint.
	(arm_gdbarch_init): Install gdbarch method
	insert_single_step_breakpoint.
	* arm-tdep.h (arm_insert_single_step_breakpoint): Remove
	declaration.
	(arm_software_single_step): Update declaration.
	* 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) *.
	(insert_single_step_breakpoint): New.
	* 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  | 14 ++++---------
 gdb/arm-tdep.c        | 18 +++++++---------
 gdb/arm-tdep.h        |  4 +---
 gdb/cris-tdep.c       | 13 ++++++------
 gdb/gdbarch.c         | 25 +++++++++++++++++++++-
 gdb/gdbarch.h         | 12 +++++++----
 gdb/gdbarch.sh        |  8 ++++---
 gdb/infrun.c          | 23 +++++++++++++++++---
 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     | 46 +++++++++++++++++++++++++++++++++++-----
 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 ++++----
 23 files changed, 226 insertions(+), 160 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 77155ef..1df6267 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 9b68315..a919358 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -923,22 +923,19 @@ 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;
   VEC (CORE_ADDR) *next_pcs = NULL;
   struct cleanup *old_chain;
 
   /* 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);
 
@@ -951,12 +948,9 @@ 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);
-
-  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 ad69834..d4d17f3 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -4089,7 +4089,7 @@ convert_to_extended (const struct floatformat *fmt, void *dbl, const void *ptr,
    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
+static void
 arm_insert_single_step_breakpoint (struct gdbarch *gdbarch,
 				   struct address_space *aspace,
 				   CORE_ADDR pc)
@@ -6156,15 +6156,12 @@ 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;
   VEC (CORE_ADDR) *next_pcs = NULL;
   struct cleanup *old_chain = make_cleanup (VEC_cleanup (CORE_ADDR), &next_pcs);
 
@@ -6177,12 +6174,8 @@ 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);
-
-  do_cleanups (old_chain);
-
-  return 1;
+  discard_cleanups (old_chain);
+  return next_pcs;
 }
 
 /* Cleanup/copy SVC (SWI) instructions.  These two functions are overridden
@@ -9274,6 +9267,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   dwarf2_frame_set_init_reg (gdbarch, arm_dwarf2_frame_init_reg);
 
+  set_gdbarch_insert_single_step_breakpoint (gdbarch,
+					     arm_insert_single_step_breakpoint);
+
   /* Add some default predicates.  */
   if (is_m)
     frame_unwind_append_unwinder (gdbarch, &arm_m_exception_unwind);
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index e5d13bb..10ab742 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -259,9 +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);
 
-void arm_insert_single_step_breakpoint (struct gdbarch *,
-					struct address_space *, CORE_ADDR);
-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/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..04e5a48 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -245,6 +245,7 @@ struct gdbarch
   gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
   gdbarch_addr_bits_remove_ftype *addr_bits_remove;
   gdbarch_software_single_step_ftype *software_single_step;
+  gdbarch_insert_single_step_breakpoint_ftype *insert_single_step_breakpoint;
   gdbarch_single_step_through_delay_ftype *single_step_through_delay;
   gdbarch_print_insn_ftype *print_insn;
   gdbarch_skip_trampoline_code_ftype *skip_trampoline_code;
@@ -404,6 +405,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->stabs_argument_has_addr = default_stabs_argument_has_addr;
   gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
   gdbarch->addr_bits_remove = core_addr_identity;
+  gdbarch->insert_single_step_breakpoint = insert_single_step_breakpoint;
   gdbarch->skip_trampoline_code = generic_skip_trampoline_code;
   gdbarch->skip_solib_resolver = generic_skip_solib_resolver;
   gdbarch->in_solib_return_trampoline = generic_in_solib_return_trampoline;
@@ -591,6 +593,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0 */
   /* Skip verify of addr_bits_remove, invalid_p == 0 */
   /* Skip verify of software_single_step, has predicate.  */
+  /* Skip verify of insert_single_step_breakpoint, invalid_p == 0 */
   /* Skip verify of single_step_through_delay, has predicate.  */
   if (gdbarch->print_insn == 0)
     fprintf_unfiltered (log, "\n\tprint_insn");
@@ -1077,6 +1080,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: inner_than = <%s>\n",
                       host_address_to_string (gdbarch->inner_than));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: insert_single_step_breakpoint = <%s>\n",
+                      host_address_to_string (gdbarch->insert_single_step_breakpoint));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: insn_is_call = <%s>\n",
                       host_address_to_string (gdbarch->insn_is_call));
   fprintf_unfiltered (file,
@@ -3066,7 +3072,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);
@@ -3083,6 +3089,23 @@ set_gdbarch_software_single_step (struct gdbarch *gdbarch,
   gdbarch->software_single_step = software_single_step;
 }
 
+void
+gdbarch_insert_single_step_breakpoint (struct gdbarch *gdbarch, struct address_space *aspace, CORE_ADDR pc)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->insert_single_step_breakpoint != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_insert_single_step_breakpoint called\n");
+  gdbarch->insert_single_step_breakpoint (gdbarch, aspace, pc);
+}
+
+void
+set_gdbarch_insert_single_step_breakpoint (struct gdbarch *gdbarch,
+                                           gdbarch_insert_single_step_breakpoint_ftype insert_single_step_breakpoint)
+{
+  gdbarch->insert_single_step_breakpoint = insert_single_step_breakpoint;
+}
+
 int
 gdbarch_single_step_through_delay_p (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 252fc4b..18da3ce 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -649,15 +649,19 @@ 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. */
+   Return a vector of addresses on which the software single step
+   breakpoints are inserted.  NULL means software single step is not used. */
 
 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);
 
+typedef void (gdbarch_insert_single_step_breakpoint_ftype) (struct gdbarch *gdbarch, struct address_space *aspace, CORE_ADDR pc);
+extern void gdbarch_insert_single_step_breakpoint (struct gdbarch *gdbarch, struct address_space *aspace, CORE_ADDR pc);
+extern void set_gdbarch_insert_single_step_breakpoint (struct gdbarch *gdbarch, gdbarch_insert_single_step_breakpoint_ftype *insert_single_step_breakpoint);
+
 /* 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/gdbarch.sh b/gdb/gdbarch.sh
index 37f59b7..667c3f1 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -608,9 +608,11 @@ 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.
-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.
+F:VEC (CORE_ADDR) *:software_single_step:struct frame_info *frame:frame
+
+m:void:insert_single_step_breakpoint:struct address_space *aspace, CORE_ADDR pc:aspace, pc::insert_single_step_breakpoint::0
 
 # 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 696105d..5dbcf7a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2248,11 +2248,28 @@ 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 ()))
+      && gdbarch_software_single_step_p (gdbarch))
     {
-      hw_step = 0;
+      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);
+
+	  hw_step = 0;
+
+	  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
+	    gdbarch_insert_single_step_breakpoint (gdbarch, aspace, pc);
+
+	  VEC_free (CORE_ADDR, next_pcs);
+	}
     }
+
   return hw_step;
 }
 
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 178a163..0d115f4 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 2e4d194..df99c80 100644
--- a/gdb/mips-tdep.h
+++ b/gdb/mips-tdep.h
@@ -157,7 +157,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 f6023bf..c3c3add 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -985,15 +985,32 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
                 }
               else
                 {
+		  struct frame_info *frame = get_current_frame ();
+		  VEC (CORE_ADDR) * next_pcs;
+
+		  next_pcs = gdbarch_software_single_step (gdbarch, frame);
+
                   /* This is a continue.
                      Try to insert a soft single step breakpoint.  */
-                  if (!gdbarch_software_single_step (gdbarch,
-                                                     get_current_frame ()))
+                  if (next_pcs == NULL)
                     {
                       /* This system don't want use soft single step.
                          Use hard sigle step.  */
                       step = 1;
                     }
+		  else
+		    {
+		      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);
+		    }
                 }
             }
         }
@@ -1163,13 +1180,32 @@ record_full_wait_1 (struct target_ops *ops,
 
                       if (gdbarch_software_single_step_p (gdbarch))
 			{
+			  VEC (CORE_ADDR) *next_pcs;
+			  struct frame_info *frame;
+
 			  /* Try to insert the software single step breakpoint.
 			     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;
+			  frame = get_current_frame ();
+			  next_pcs
+			    = gdbarch_software_single_step (gdbarch, frame);
+			  if (next_pcs != NULL)
+			    {
+			      int i;
+			      CORE_ADDR pc;
+			      struct address_space *aspace;
+
+			      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);
+			      step = 0;
+			    }
 			  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/2] Remove gdbarch method displaced_step_hw_singlestep
  2016-03-23 14:10 [PATCH 0/2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
  2016-03-23 14:10 ` [PATCH 1/2] " Yao Qi
@ 2016-03-23 14:10 ` Yao Qi
  2016-03-23 17:26   ` Pedro Alves
  1 sibling, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-03-23 14:10 UTC (permalink / raw)
  To: gdb-patches

displaced_step_hw_singlestep was added for some targets, which can do
hardware single step, but need software single step for some special
instructions.  After we change gdbarch method software_single_step,
displaced_step_hw_singlestep is no longer necessary, because we can
get the same information from software_single_step.  This patch is
to remove displaced_step_hw_singlestep.

gdb:

2016-03-22  Yao Qi  <yao.qi@linaro.org>

	* aarch64-linux-tdep.c (aarch64_linux_init_abi): Don't call
	set_gdbarch_displaced_step_hw_singlestep.
	* aarch64-tdep.c (aarch64_displaced_step_hw_singlestep): Remove.
	* arch-utils.c (default_displaced_step_hw_singlestep): Remove.
	* arch-utils.h (default_displaced_step_hw_singlestep): Remove.
	* gdbarch.sh (displaced_step_hw_singlestep): Remove.
	* gdbarch.c, gdbarch.h: Regenerated.
	* infrun.c (resume): Don't call
	gdbarch_displaced_step_hw_singlestep.  Call
	gdbarch_software_single_step instead.
	* rs6000-tdep.c (ppc_displaced_step_hw_singlestep): Remove.
	(rs6000_gdbarch_init): Don't call
	set_gdbarch_displaced_step_hw_singlestep.
	* s390-linux-tdep.c (s390_displaced_step_hw_singlestep): Remove.
	(s390_gdbarch_init): Don't call
	set_gdbarch_displaced_step_hw_singlestep.
---
 gdb/aarch64-linux-tdep.c |  2 --
 gdb/aarch64-tdep.c       |  9 ---------
 gdb/arch-utils.c         |  7 -------
 gdb/arch-utils.h         |  5 -----
 gdb/gdbarch.c            | 23 -----------------------
 gdb/gdbarch.h            | 14 --------------
 gdb/gdbarch.sh           | 11 -----------
 gdb/infrun.c             | 21 +++++++++++++++++++--
 gdb/rs6000-tdep.c        | 12 +-----------
 gdb/s390-linux-tdep.c    |  9 ---------
 10 files changed, 20 insertions(+), 93 deletions(-)

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index 651a0f0..8344d06 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1208,8 +1208,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_displaced_step_free_closure (gdbarch,
 					   simple_displaced_step_free_closure);
   set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_location);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch,
-					    aarch64_displaced_step_hw_singlestep);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 1df6267..17d0894 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2637,15 +2637,6 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
     }
 }
 
-/* Implement the "displaced_step_hw_singlestep" gdbarch method.  */
-
-int
-aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				      struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
    architectures already created during this debugging session.
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c3d7802..3049252 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -67,13 +67,6 @@ simple_displaced_step_free_closure (struct gdbarch *gdbarch,
   xfree (closure);
 }
 
-int
-default_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				      struct displaced_step_closure *closure)
-{
-  return !gdbarch_software_single_step_p (gdbarch);
-}
-
 CORE_ADDR
 displaced_step_at_entry_point (struct gdbarch *gdbarch)
 {
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 9e1e70e..318e227 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -45,11 +45,6 @@ extern void
   simple_displaced_step_free_closure (struct gdbarch *gdbarch,
                                       struct displaced_step_closure *closure);
 
-/* Default implementation of gdbarch_displaced_hw_singlestep.  */
-extern int
-  default_displaced_step_hw_singlestep (struct gdbarch *,
-					struct displaced_step_closure *);
-
 /* Possible value for gdbarch_displaced_step_location:
    Place displaced instructions at the program's entry point,
    leaving space for inferior function call return breakpoints.  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 04e5a48..123df30 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -278,7 +278,6 @@ struct gdbarch
   gdbarch_skip_permanent_breakpoint_ftype *skip_permanent_breakpoint;
   ULONGEST max_insn_length;
   gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn;
-  gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep;
   gdbarch_displaced_step_fixup_ftype *displaced_step_fixup;
   gdbarch_displaced_step_free_closure_ftype *displaced_step_free_closure;
   gdbarch_displaced_step_location_ftype *displaced_step_location;
@@ -416,7 +415,6 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->adjust_dwarf2_line = default_adjust_dwarf2_line;
   gdbarch->register_reggroup_p = default_register_reggroup_p;
   gdbarch->skip_permanent_breakpoint = default_skip_permanent_breakpoint;
-  gdbarch->displaced_step_hw_singlestep = default_displaced_step_hw_singlestep;
   gdbarch->displaced_step_fixup = NULL;
   gdbarch->displaced_step_free_closure = NULL;
   gdbarch->displaced_step_location = NULL;
@@ -627,7 +625,6 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of skip_permanent_breakpoint, invalid_p == 0 */
   /* Skip verify of max_insn_length, has predicate.  */
   /* Skip verify of displaced_step_copy_insn, has predicate.  */
-  /* Skip verify of displaced_step_hw_singlestep, invalid_p == 0 */
   /* Skip verify of displaced_step_fixup, has predicate.  */
   if ((! gdbarch->displaced_step_free_closure) != (! gdbarch->displaced_step_copy_insn))
     fprintf_unfiltered (log, "\n\tdisplaced_step_free_closure");
@@ -876,9 +873,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: displaced_step_free_closure = <%s>\n",
                       host_address_to_string (gdbarch->displaced_step_free_closure));
   fprintf_unfiltered (file,
-                      "gdbarch_dump: displaced_step_hw_singlestep = <%s>\n",
-                      host_address_to_string (gdbarch->displaced_step_hw_singlestep));
-  fprintf_unfiltered (file,
                       "gdbarch_dump: displaced_step_location = <%s>\n",
                       host_address_to_string (gdbarch->displaced_step_location));
   fprintf_unfiltered (file,
@@ -3772,23 +3766,6 @@ set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch,
 }
 
 int
-gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure)
-{
-  gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->displaced_step_hw_singlestep != NULL);
-  if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_displaced_step_hw_singlestep called\n");
-  return gdbarch->displaced_step_hw_singlestep (gdbarch, closure);
-}
-
-void
-set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-                                          gdbarch_displaced_step_hw_singlestep_ftype displaced_step_hw_singlestep)
-{
-  gdbarch->displaced_step_hw_singlestep = displaced_step_hw_singlestep;
-}
-
-int
 gdbarch_displaced_step_fixup_p (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 18da3ce..f11e837 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -956,20 +956,6 @@ typedef struct displaced_step_closure * (gdbarch_displaced_step_copy_insn_ftype)
 extern struct displaced_step_closure * gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
 extern void set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn);
 
-/* Return true if GDB should use hardware single-stepping to execute
-   the displaced instruction identified by CLOSURE.  If false,
-   GDB will simply restart execution at the displaced instruction
-   location, and it is up to the target to ensure GDB will receive
-   control again (e.g. by placing a software breakpoint instruction
-   into the displaced instruction buffer).
-  
-   The default implementation returns false on all targets that
-   provide a gdbarch_software_single_step routine, and true otherwise. */
-
-typedef int (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern int gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep);
-
 /* Fix up the state resulting from successfully single-stepping a
    displaced instruction, to give the result we would have gotten from
    stepping the instruction in its original location.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 667c3f1..aae3acb 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -777,17 +777,6 @@ V:ULONGEST:max_insn_length:::0:0
 # that case.
 M:struct displaced_step_closure *:displaced_step_copy_insn:CORE_ADDR from, CORE_ADDR to, struct regcache *regs:from, to, regs
 
-# Return true if GDB should use hardware single-stepping to execute
-# the displaced instruction identified by CLOSURE.  If false,
-# GDB will simply restart execution at the displaced instruction
-# location, and it is up to the target to ensure GDB will receive
-# control again (e.g. by placing a software breakpoint instruction
-# into the displaced instruction buffer).
-#
-# The default implementation returns false on all targets that
-# provide a gdbarch_software_single_step routine, and true otherwise.
-m:int:displaced_step_hw_singlestep:struct displaced_step_closure *closure:closure::default_displaced_step_hw_singlestep::0
-
 # Fix up the state resulting from successfully single-stepping a
 # displaced instruction, to give the result we would have gotten from
 # stepping the instruction in its original location.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5dbcf7a..10aa3f1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2611,8 +2611,25 @@ resume (enum gdb_signal sig)
 	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
 
 	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
-	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
-						       displaced->step_closure);
+
+	  if (gdbarch_software_single_step_p (gdbarch))
+	    {
+	      VEC (CORE_ADDR) * next_pcs = NULL;
+
+	      next_pcs = gdbarch_software_single_step (gdbarch,
+						       get_current_frame ());
+
+	      if (next_pcs != 0)
+		{
+		  step = 0;
+		  VEC_free (CORE_ADDR, next_pcs);
+		}
+	      else
+		step = 1;
+	    }
+	  else
+	    step = 1;
+
 	}
     }
 
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 26c8ed9..5f0ede2 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1136,15 +1136,6 @@ ppc_displaced_step_fixup (struct gdbarch *gdbarch,
 				    from + offset);
 }
 
-/* Always use hardware single-stepping to execute the
-   displaced instruction.  */
-static int
-ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				  struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
 /* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX
    instruction and ending with a STWCX/STDCX instruction.  If such a sequence
    is found, attempt to step through it.  A breakpoint is placed at the end of 
@@ -6071,8 +6062,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Setup displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
 					ppc_displaced_step_copy_insn);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch,
-					    ppc_displaced_step_hw_singlestep);
+
   set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup);
   set_gdbarch_displaced_step_free_closure (gdbarch,
 					   simple_displaced_step_free_closure);
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index dd26ba6..77c7b33 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -759,14 +759,6 @@ s390_software_single_step (struct frame_info *frame)
   return next_pcs;
 }
 
-static int
-s390_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				   struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
-
 /* Maps for register sets.  */
 
 static const struct regcache_map_entry s390_gregmap[] =
@@ -7990,7 +7982,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, s390_breakpoint_from_pc);
   set_gdbarch_software_single_step (gdbarch, s390_software_single_step);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch, s390_displaced_step_hw_singlestep);
   set_gdbarch_skip_prologue (gdbarch, s390_skip_prologue);
   set_gdbarch_stack_frame_destroyed_p (gdbarch, s390_stack_frame_destroyed_p);
 
-- 
1.9.1

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

* Re: [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-03-23 14:10 ` [PATCH 1/2] " Yao Qi
@ 2016-03-23 17:25   ` Pedro Alves
  2016-03-30 14:14     ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-03-23 17:25 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 03/23/2016 02:10 PM, Yao Qi wrote:

 
> However, breakpoint insertion in arm is a little different, which
> uses arm_insert_single_step_breakpoint, and it updates a global
> variable arm_override_mode, so that arm_pc_is_thumb can get the
> right arm thumb mode even the program is wrong (see
> gdb.arch/thumb-singlestep.exp).  I failed to remove global variable
> arm_override_mode, so have to add a new gdbarch method
> insert_single_step_breakpoint, which is in default
> insert_single_step_breakpoint for all targets except arm.
> 

I think the problem is the ambiguity in arm_breakpoint_from_pc,
and to the fact that we don't "normalize" breakpoint addresses
before passing them down to target_insert_breakpoint. 

Some callers start with an address coming from the user and want
to consult symbol tables / mapping symbols.  Other caller really
want to trust the thumb bit as set in the address.

Note that arm_breakpoint_from_pc uses arm_pc_is_thumb, which is
what consults symbol tables / mapping symbols.

I think the fix would to make arm_breakpoint_from_pc always trust
that the address bit is already encoded correctly, and trust
IS_THUMB_ADDR, similarly to how the gdbserver version does, in
arm_breakpoint_kind_from_pc.

Then, we'd still need to consult the mapping symbols
consultation, or IOW, do something based on arm_pc_is_thumb _before_
target_insert_breakpoint is reached.  That is, call something like
arm_pc_is_thumb and use the result to encode the thumb bit correctly in
the address passed to target_insert_breakpoint.  IOW, "normalize" the
target address, using some gdbarch method, _before_ that address is passed
to the target routines in the first place.

Along the way, several other functions would stop using arm_pc_is_thumb,
but use IS_THUMB_ADDR directly.  E.g., arm_remote_breakpoint_from_pc.

WDYT?


> -# A return value of 1 means that the software_single_step breakpoints
> -# were inserted; 0 means they were not.
> -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.

s/are inserted/should be inserted/

> +F:VEC (CORE_ADDR) *:software_single_step:struct frame_info *frame:frame
> +
> +m:void:insert_single_step_breakpoint:struct address_space *aspace, CORE_ADDR pc:aspace, pc::insert_single_step_breakpoint::0
>   
>   # 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 696105d..5dbcf7a 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2248,11 +2248,28 @@ 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 ()))
> +      && gdbarch_software_single_step_p (gdbarch))
>       {
> -      hw_step = 0;
> +      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);
> +
> +	  hw_step = 0;
> +
> +	  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
> +	    gdbarch_insert_single_step_breakpoint (gdbarch, aspace, pc);
> +
> +	  VEC_free (CORE_ADDR, next_pcs);
> +	}

This pattern of starting from a VEC of addresses, and a frame and
calling gdbarch_insert_single_step_breakpoint on each address
appears multiple times.  Can we put it in a convenience function?

Though the other calls in record-full.c didn't go through
gdbarch_insert_single_step_breakpoint -- why's that?

Otherwise, this is fine with me.   

Thanks,
Pedro Alves

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

* Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep
  2016-03-23 14:10 ` [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep Yao Qi
@ 2016-03-23 17:26   ` Pedro Alves
  2016-05-09 11:05     ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-03-23 17:26 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 03/23/2016 02:10 PM, Yao Qi wrote:
> displaced_step_hw_singlestep was added for some targets, which can do
> hardware single step, but need software single step for some special
> instructions.  After we change gdbarch method software_single_step,
> displaced_step_hw_singlestep is no longer necessary, because we can
> get the same information from software_single_step.  

I wasn't sure whether this is safe, but I convinced myself it is.
I'd have been nice if there had been a note in the log about the below:

Currently, when stepping past an instruction in the scratch pad, these
archs won't ever reach the software_single_step method, always forcing a
hardware single-step, even if the software_single_step method would
insert some breakpoint.  The question is: is it safe now for them to
analyse the instruction copied to the scratch pad, and potentially insert
a software single-step?

I think it is safe, because we won't ever use displaced stepping
for the cases where the software_single_step method would return
something non-NULL.  software_single_step returns non-NULL _only_
for atomic regions, while OTOH, displaced_step_copy_insn always returns
NULL for atomic regions.  E.g., notice how ppc_displaced_step_copy_insn
vs ppc_deal_with_atomic_sequence.

> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2611,8 +2611,25 @@ resume (enum gdb_signal sig)
>   	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
>   
>   	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
> -	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
> -						       displaced->step_closure);
> +
> +	  if (gdbarch_software_single_step_p (gdbarch))
> +	    {
> +	      VEC (CORE_ADDR) * next_pcs = NULL;

No need to initialize.

> +
> +	      next_pcs = gdbarch_software_single_step (gdbarch,
> +						       get_current_frame ());
> +
> +	      if (next_pcs != 0)

next_pcs != NULL.

Thanks,
Pedro Alves

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

* Re: [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-03-23 17:25   ` Pedro Alves
@ 2016-03-30 14:14     ` Yao Qi
  2016-04-27 15:19       ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-03-30 14:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Pedro Alves <palves@redhat.com> writes:

> I think the problem is the ambiguity in arm_breakpoint_from_pc,
> and to the fact that we don't "normalize" breakpoint addresses
> before passing them down to target_insert_breakpoint. 

We don't pass address to target_insert_breakpoint, instead, we pass
bp_target_info.  If we do what you suggested, we need to set
"normalized" address into bl->target_info.reqstd_address.  This is the
only way to pass normalized address down to target_insert_breakpoint.
This means we propagate the ISA specific bits of address to breakpoint
system, I don't see anything harmful so far, but I feel risky to do so.

>
> Some callers start with an address coming from the user and want
> to consult symbol tables / mapping symbols.  Other caller really
> want to trust the thumb bit as set in the address.

Right.  If we want to fully trust on the bit of address, in some cases,
we need to consult symbols to normalize the address.  The question is
"in what cases do we do so?".  In my experimental patch, address is
normalized for breakpoints except single step breakpoint.

>
> Note that arm_breakpoint_from_pc uses arm_pc_is_thumb, which is
> what consults symbol tables / mapping symbols.
>
> I think the fix would to make arm_breakpoint_from_pc always trust
> that the address bit is already encoded correctly, and trust
> IS_THUMB_ADDR, similarly to how the gdbserver version does, in
> arm_breakpoint_kind_from_pc.

If so, we need to normalize addresses before calling
gdbarch_breakpoint_from_pc.  Moreover, we need to normalize address
before calling insert_single_step_breakpoint out side of
gdbarch_software_single_step.

>
> Then, we'd still need to consult the mapping symbols
> consultation, or IOW, do something based on arm_pc_is_thumb _before_
> target_insert_breakpoint is reached.  That is, call something like
> arm_pc_is_thumb and use the result to encode the thumb bit correctly in
> the address passed to target_insert_breakpoint.  IOW, "normalize" the
> target address, using some gdbarch method, _before_ that address is passed
> to the target routines in the first place.
>
> Along the way, several other functions would stop using arm_pc_is_thumb,
> but use IS_THUMB_ADDR directly.  E.g., arm_remote_breakpoint_from_pc.
>
> WDYT?
>

I wrote a patch as you suggested, and attach it below.  I am not very
happy on this one, but I am open to thoughts from you and others.

>
>> -# A return value of 1 means that the software_single_step breakpoints
>> -# were inserted; 0 means they were not.
>> -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.
>
> s/are inserted/should be inserted/
>

I'll fix it.

>> +F:VEC (CORE_ADDR) *:software_single_step:struct frame_info *frame:frame
>> +
>> +m:void:insert_single_step_breakpoint:struct address_space *aspace, CORE_ADDR pc:aspace, pc::insert_single_step_breakpoint::0
>>   
>>   # 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 696105d..5dbcf7a 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -2248,11 +2248,28 @@ 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 ()))
>> +      && gdbarch_software_single_step_p (gdbarch))
>>       {
>> -      hw_step = 0;
>> +      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);
>> +
>> +	  hw_step = 0;
>> +
>> +	  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
>> +	    gdbarch_insert_single_step_breakpoint (gdbarch, aspace, pc);
>> +
>> +	  VEC_free (CORE_ADDR, next_pcs);
>> +	}
>
> This pattern of starting from a VEC of addresses, and a frame and
> calling gdbarch_insert_single_step_breakpoint on each address
> appears multiple times.  Can we put it in a convenience function?

Sure, I'll do.

>
> Though the other calls in record-full.c didn't go through
> gdbarch_insert_single_step_breakpoint -- why's that?

Oh, gdbarch_insert_single_step_breakpoint should be called.  I'll fix it.

-- 
Yao (齐尧)
From c2472962d9a7eed8dbfd9a1536cf84128bbb3ce6 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 24 Mar 2016 13:58:21 +0000
Subject: [PATCH] Remove arm_override_mode

This patch removes global variable arm_override_mode.  The reason we
need this global variable is that some callers of arm_pc_is_thumb
want to know the arm/thumb mode by consulting symbols, but some
callers want to know the arm/thumb mode by checking LSB of address
only.

In order to remove this global variable, we switch arm backend to
rely on LSB of address for arm/thumb mode checking, so the address
should be properly "encoded" before going to
gdbarch_breakpoint_from_pc.  A new gdbarch method addr_bits_encode
is added and in arm, we consult arm_pc_is_thumb and adjust address
properly.

gdb:

2016-03-30  Yao Qi  <yao.qi@linaro.org>

	* arch-utils.c (displaced_step_at_entry_point): Call
	gdbarch_addr_bits_encode.
	(default_remote_breakpoint_from_pc): Likewise.
	* arm-linux-tdep.c (arm_linux_software_single_step): Call
	insert_single_step_breakpoint.
	* arm-tdep.c (arm_override_mode): Remove.
	(arm_pc_is_thumb): Don't check arm_override_mode.
	(arm_addr_bits_encode): New function.
	(arm_insert_single_step_breakpoint): Remove.
	(arm_breakpoint_from_pc): Use IS_THUMB_ADDR rather than
	(arm_remote_breakpoint_from_pc): Likewise.
	(arm_gdbarch_init): Call set_gdbarch_addr_bits_encode.
	* arm-tdep.h (arm_insert_single_step_breakpoint): Remove
	declaration.
	* breakpoint.c (insert_bp_location): Call
	gdbarch_addr_bits_encode.
	(program_breakpoint_here_p): Likewise.
	* gdbarch.c, gdbarch.h: Regenerated.
	* gdbarch.sh (addr_bits_encode): New.
	* infcall.c (call_function_by_hand_dummy): Call
	gdbarch_addr_bits_encode.
	* infrun.c (resume): Likewise.
	(keep_going_stepped_thread): Likewise.
	* linux-tdep.c (linux_displaced_step_location): Likewise.
---
 gdb/arch-utils.c     |  3 +++
 gdb/arm-linux-tdep.c |  2 +-
 gdb/arm-tdep.c       | 51 +++++++++++++++++----------------------------------
 gdb/arm-tdep.h       |  2 --
 gdb/breakpoint.c     | 18 ++++++++++++++++--
 gdb/gdbarch.c        | 23 +++++++++++++++++++++++
 gdb/gdbarch.h        |  6 ++++++
 gdb/gdbarch.sh       |  3 +++
 gdb/infcall.c        |  2 +-
 gdb/infrun.c         |  2 ++
 gdb/linux-tdep.c     |  1 +
 11 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c3d7802..bd185da 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -85,6 +85,7 @@ displaced_step_at_entry_point (struct gdbarch *gdbarch)
   /* Inferior calls also use the entry point as a breakpoint location.
      We don't want displaced stepping to interfere with those
      breakpoints, so leave space.  */
+  addr = gdbarch_addr_bits_encode (gdbarch, addr);
   gdbarch_breakpoint_from_pc (gdbarch, &addr, &bp_len);
   addr += bp_len * 2;
 
@@ -809,6 +810,7 @@ void
 default_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
 				   int *kindptr)
 {
+  *pcptr = gdbarch_addr_bits_encode  (gdbarch, *pcptr);
   gdbarch_breakpoint_from_pc (gdbarch, pcptr, kindptr);
 }
 
@@ -853,6 +855,7 @@ default_skip_permanent_breakpoint (struct regcache *regcache)
   const gdb_byte *bp_insn;
   int bp_len;
 
+  current_pc = gdbarch_addr_bits_encode (gdbarch, current_pc);
   bp_insn = gdbarch_breakpoint_from_pc (gdbarch, &current_pc, &bp_len);
   current_pc += bp_len;
   regcache_write_pc (regcache, current_pc);
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 9b68315..89e1ec7 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -952,7 +952,7 @@ 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);
+    insert_single_step_breakpoint (gdbarch, aspace, pc);
 
   do_cleanups (old_chain);
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index ad69834..74a94ed 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -143,13 +143,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,10 +415,6 @@ 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 the user wants to override the symbol table, let him.  */
   if (strcmp (arm_force_mode_string, "arm") == 0)
     return 0;
@@ -480,6 +469,17 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val)
     return (val & 0x03fffffc);
 }
 
+/* Implement the addr_bits_encode gdbarch method.  */
+
+static CORE_ADDR
+arm_addr_bits_encode (struct gdbarch *gdbarch, CORE_ADDR val)
+{
+  if (arm_pc_is_thumb (gdbarch, val))
+    return MAKE_THUMB_ADDR (val);
+  else
+    return UNMAKE_THUMB_ADDR (val);
+}
+
 /* Return 1 if PC is the start of a compiler helper function which
    can be safely ignored during prologue skipping.  IS_THUMB is true
    if the function is known to be a Thumb function due to the way it
@@ -4085,26 +4085,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)
-{
-  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
    the buffer to be NEW_LEN bytes ending at ENDADDR.  Return
    NULL if an error occurs.  BUF is freed.  */
@@ -6178,7 +6158,7 @@ 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);
+    insert_single_step_breakpoint (gdbarch, aspace, pc);
 
   do_cleanups (old_chain);
 
@@ -7699,7 +7679,7 @@ arm_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
 
-  if (arm_pc_is_thumb (gdbarch, *pcptr))
+  if (IS_THUMB_ADDR (*pcptr))
     {
       *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
 
@@ -7734,9 +7714,11 @@ static void
 arm_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
 			       int *kindptr)
 {
+  CORE_ADDR pc = *pcptr;
+
   arm_breakpoint_from_pc (gdbarch, pcptr, kindptr);
 
-  if (arm_pc_is_thumb (gdbarch, *pcptr) && *kindptr == 4)
+  if (IS_THUMB_ADDR (pc) && *kindptr == 4)
     /* The documented magic value for a 32-bit Thumb-2 breakpoint, so
        that this is not confused with a 32-bit ARM breakpoint.  */
     *kindptr = 3;
@@ -9214,6 +9196,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* Address manipulation.  */
   set_gdbarch_addr_bits_remove (gdbarch, arm_addr_bits_remove);
+  set_gdbarch_addr_bits_encode (gdbarch, arm_addr_bits_encode);
 
   /* Advance PC across function entry code.  */
   set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue);
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);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f99a7ab..6da8339 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2602,6 +2602,8 @@ build_target_command_list (struct bp_location *bl)
     bl->target_info.persist = 1;
 }
 
+static int breakpoint_address_is_meaningful (struct breakpoint *bpt);
+
 /* Insert a low-level "breakpoint" of some type.  BL is the breakpoint
    location.  Any error messages are printed to TMP_ERROR_STREAM; and
    DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems.
@@ -2632,7 +2634,19 @@ insert_bp_location (struct bp_location *bl,
      we have a breakpoint inserted at that address and thus
      read the breakpoint instead of returning the data saved in
      the breakpoint location's shadow contents.  */
-  bl->target_info.reqstd_address = bl->address;
+  if (bl->owner->type == bp_single_step
+      || !breakpoint_address_is_meaningful (bl->owner))
+    bl->target_info.reqstd_address = bl->address;
+  else
+    {
+      /* Encode ISA specific bits into reqstd_address for breakpoints
+	 except single step breakpoint, because the target address of
+	 single step breakpoint has already encoded with ISA specific
+	 bits.  */
+      bl->target_info.reqstd_address = gdbarch_addr_bits_encode (bl->gdbarch,
+								 bl->address);
+    }
+
   bl->target_info.placed_address_space = bl->pspace->aspace;
   bl->target_info.length = bl->length;
 
@@ -9084,7 +9098,7 @@ program_breakpoint_here_p (struct gdbarch *gdbarch, CORE_ADDR address)
   struct cleanup *cleanup;
   int retval = 0;
 
-  addr = address;
+  addr = gdbarch_addr_bits_encode (gdbarch, address);
   bpoint = gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
 
   /* Software breakpoints unsupported?  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..52aedb0 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -244,6 +244,7 @@ struct gdbarch
   int frame_red_zone_size;
   gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
   gdbarch_addr_bits_remove_ftype *addr_bits_remove;
+  gdbarch_addr_bits_encode_ftype *addr_bits_encode;
   gdbarch_software_single_step_ftype *software_single_step;
   gdbarch_single_step_through_delay_ftype *single_step_through_delay;
   gdbarch_print_insn_ftype *print_insn;
@@ -404,6 +405,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->stabs_argument_has_addr = default_stabs_argument_has_addr;
   gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
   gdbarch->addr_bits_remove = core_addr_identity;
+  gdbarch->addr_bits_encode = core_addr_identity;
   gdbarch->skip_trampoline_code = generic_skip_trampoline_code;
   gdbarch->skip_solib_resolver = generic_skip_solib_resolver;
   gdbarch->in_solib_return_trampoline = generic_in_solib_return_trampoline;
@@ -590,6 +592,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of stabs_argument_has_addr, invalid_p == 0 */
   /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0 */
   /* Skip verify of addr_bits_remove, invalid_p == 0 */
+  /* Skip verify of addr_bits_encode, invalid_p == 0 */
   /* Skip verify of software_single_step, has predicate.  */
   /* Skip verify of single_step_through_delay, has predicate.  */
   if (gdbarch->print_insn == 0)
@@ -708,6 +711,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: addr_bit = %s\n",
                       plongest (gdbarch->addr_bit));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: addr_bits_encode = <%s>\n",
+                      host_address_to_string (gdbarch->addr_bits_encode));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: addr_bits_remove = <%s>\n",
                       host_address_to_string (gdbarch->addr_bits_remove));
   fprintf_unfiltered (file,
@@ -3059,6 +3065,23 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
   gdbarch->addr_bits_remove = addr_bits_remove;
 }
 
+CORE_ADDR
+gdbarch_addr_bits_encode (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->addr_bits_encode != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_addr_bits_encode called\n");
+  return gdbarch->addr_bits_encode (gdbarch, addr);
+}
+
+void
+set_gdbarch_addr_bits_encode (struct gdbarch *gdbarch,
+                              gdbarch_addr_bits_encode_ftype addr_bits_encode)
+{
+  gdbarch->addr_bits_encode = addr_bits_encode;
+}
+
 int
 gdbarch_software_single_step_p (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 252fc4b..cba7bb0 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -642,6 +642,12 @@ typedef CORE_ADDR (gdbarch_addr_bits_remove_ftype) (struct gdbarch *gdbarch, COR
 extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr);
 extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
 
+/* Encode the ISA specific bits into address. */
+
+typedef CORE_ADDR (gdbarch_addr_bits_encode_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern CORE_ADDR gdbarch_addr_bits_encode (struct gdbarch *gdbarch, CORE_ADDR addr);
+extern void set_gdbarch_addr_bits_encode (struct gdbarch *gdbarch, gdbarch_addr_bits_encode_ftype *addr_bits_encode);
+
 /* 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.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 37f59b7..8c937b3 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -601,6 +601,9 @@ m:CORE_ADDR:convert_from_func_ptr_addr:CORE_ADDR addr, struct target_ops *targ:a
 # possible it should be in TARGET_READ_PC instead).
 m:CORE_ADDR:addr_bits_remove:CORE_ADDR addr:addr::core_addr_identity::0
 
+# Encode the ISA specific bits into address.
+m:CORE_ADDR:addr_bits_encode: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.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 77cd931..4809457 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -897,7 +897,7 @@ call_function_by_hand_dummy (struct value *function,
 	   If software breakpoints are unsupported for this target we
 	   leave the user visible memory content uninitialized.  */
 
-	bp_addr_as_address = bp_addr;
+	bp_addr_as_address = gdbarch_addr_bits_encode (gdbarch, bp_addr);
 	bp_bytes = gdbarch_breakpoint_from_pc (gdbarch, &bp_addr_as_address,
 					       &bp_size);
 	if (bp_bytes != NULL)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 696105d..feee63d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2525,6 +2525,7 @@ resume (enum gdb_signal sig)
 		 don't want this thread to step further from PC
 		 (overstep).  */
 	      gdb_assert (!step_over_info_valid_p ());
+	      pc = gdbarch_addr_bits_encode (gdbarch, pc);
 	      insert_single_step_breakpoint (gdbarch, aspace, pc);
 	      insert_breakpoints ();
 
@@ -7217,6 +7218,7 @@ keep_going_stepped_thread (struct thread_info *tp)
       clear_step_over_info ();
       tp->control.trap_expected = 0;
 
+      stop_pc = gdbarch_addr_bits_encode (gdbarch, stop_pc);
       insert_single_step_breakpoint (get_frame_arch (frame),
 				     get_frame_address_space (frame),
 				     stop_pc);
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index f197aa7..94faedd 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2437,6 +2437,7 @@ linux_displaced_step_location (struct gdbarch *gdbarch)
   /* Inferior calls also use the entry point as a breakpoint location.
      We don't want displaced stepping to interfere with those
      breakpoints, so leave space.  */
+  addr = gdbarch_addr_bits_encode (gdbarch, addr);
   gdbarch_breakpoint_from_pc (gdbarch, &addr, &bp_len);
   addr += bp_len * 2;
 
-- 
1.9.1

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

* Re: [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-03-30 14:14     ` Yao Qi
@ 2016-04-27 15:19       ` Pedro Alves
  2016-04-29 14:48         ` Yao Qi
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-04-27 15:19 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 03/30/2016 03:14 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> I think the problem is the ambiguity in arm_breakpoint_from_pc,
>> and to the fact that we don't "normalize" breakpoint addresses
>> before passing them down to target_insert_breakpoint. 
> 
> We don't pass address to target_insert_breakpoint, instead, we pass
> bp_target_info. 

Yeah, well, we pass the address _in_ a bp_target_info, but we
pass the address.

> If we do what you suggested, we need to set
> "normalized" address into bl->target_info.reqstd_address.  This is the
> only way to pass normalized address down to target_insert_breakpoint.

Right.

> This means we propagate the ISA specific bits of address to breakpoint
> system, I don't see anything harmful so far, but I feel risky to do so.
> 
>>
>> Some callers start with an address coming from the user and want
>> to consult symbol tables / mapping symbols.  Other caller really
>> want to trust the thumb bit as set in the address.
> 
> Right.  If we want to fully trust on the bit of address, in some cases,
> we need to consult symbols to normalize the address.

Yeah.

> The question is
> "in what cases do we do so?".  In my experimental patch, address is
> normalized for breakpoints except single step breakpoint.
> 
>>
>> Note that arm_breakpoint_from_pc uses arm_pc_is_thumb, which is
>> what consults symbol tables / mapping symbols.
>>
>> I think the fix would to make arm_breakpoint_from_pc always trust
>> that the address bit is already encoded correctly, and trust
>> IS_THUMB_ADDR, similarly to how the gdbserver version does, in
>> arm_breakpoint_kind_from_pc.
> 
> If so, we need to normalize addresses before calling
> gdbarch_breakpoint_from_pc.  Moreover, we need to normalize address
> before calling insert_single_step_breakpoint out side of
> gdbarch_software_single_step.
> 
>>
>> Then, we'd still need to consult the mapping symbols
>> consultation, or IOW, do something based on arm_pc_is_thumb _before_
>> target_insert_breakpoint is reached.  That is, call something like
>> arm_pc_is_thumb and use the result to encode the thumb bit correctly in
>> the address passed to target_insert_breakpoint.  IOW, "normalize" the
>> target address, using some gdbarch method, _before_ that address is passed
>> to the target routines in the first place.
>>
>> Along the way, several other functions would stop using arm_pc_is_thumb,
>> but use IS_THUMB_ADDR directly.  E.g., arm_remote_breakpoint_from_pc.
>>
>> WDYT?

Another similar/related idea would be to go 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:

  /* Return the breakpoint kind for this target based on PC.  The PCPTR is
     adjusted to the real memory location in case a flag (e.g., the Thumb bit on
     ARM) was present in the PC.  */
  int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);

  /* Return the software breakpoint from KIND.  KIND can have target
     specific meaning like the Z0 kind parameter.
     SIZE is set to the software breakpoint's length in memory.  */
  const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);


struct bp_target_info already has the "placed_size" field,
maybe we could reuse it, just like we started from the "len"
field on gdbserver, in 271652949894 (Support breakpoint kinds for 
software breakpoints in GDBServer).

In effect, this would move the gdbarch_remote_breakpoint_from_pc
calls to common code, just like it happened in gdbserver.

So when setting a single-step breakpoint, we'd get the "kind"
from the current mode, and when setting breakpoints from
user-input, we'd get it from the symbols tables / mapping symbols.

This would probably be the cleanest, and would expose the most
sharing opportunities with gdbserver.

I think it'd avoid the encoding <-> decoding juggling to get
at the "kind" that we see in your patch.

> @@ -809,6 +810,7 @@ void
>  default_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr,
>  				   int *kindptr)
>  {
> +  *pcptr = gdbarch_addr_bits_encode  (gdbarch, *pcptr);
>    gdbarch_breakpoint_from_pc (gdbarch, pcptr, kindptr);
>  }

Would we need this one?  This is called from remote.c:remote_insert_breakpoint,
which already has the addr bit encoded in the PC.

Thanks,
Pedro Alves

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

* Re: [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-04-27 15:19       ` Pedro Alves
@ 2016-04-29 14:48         ` Yao Qi
  2016-05-02 10:21           ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2016-04-29 14:48 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Pedro Alves <palves@redhat.com> writes:

> Another similar/related idea would be to go 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:
>
>   /* Return the breakpoint kind for this target based on PC.  The PCPTR is
>      adjusted to the real memory location in case a flag (e.g., the Thumb bit on
>      ARM) was present in the PC.  */
>   int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
>
>   /* Return the software breakpoint from KIND.  KIND can have target
>      specific meaning like the Z0 kind parameter.
>      SIZE is set to the software breakpoint's length in memory.  */
>   const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
>
>
> struct bp_target_info already has the "placed_size" field,
> maybe we could reuse it, just like we started from the "len"
> field on gdbserver, in 271652949894 (Support breakpoint kinds for 
> software breakpoints in GDBServer).

Yeah, I can give a try.

>
> In effect, this would move the gdbarch_remote_breakpoint_from_pc
> calls to common code, just like it happened in gdbserver.
>

gdbarch_remote_breakpoint_from_pc will be no longer needed, because
we've got "kind" field.  We can use it when sending Z packet.

> So when setting a single-step breakpoint, we'd get the "kind"
> from the current mode, and when setting breakpoints from
> user-input, we'd get it from the symbols tables / mapping symbols.

This means we need this to get the "kind" from the current mode,

  /* Return the breakpoint kind for this target based on the current
     processor state (e.g. the current instruction mode on ARM) and the
     PC.  The PCPTR is adjusted to the real memory location in case a flag
     (e.g., the Thumb bit on ARM) is present in the PC.  */
  int (*breakpoint_kind_from_current_state) (CORE_ADDR *pcptr);

and also need to pass breakpoint location to to_insert_breakpoint to get
the type of breakpoint we are inserting, right?

-- 
Yao (齐尧)

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

* Re: [PATCH 1/2] gdbarch software_single_step returns VEC (CORE_ADDR) *
  2016-04-29 14:48         ` Yao Qi
@ 2016-05-02 10:21           ` Pedro Alves
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2016-05-02 10:21 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 04/29/2016 03:48 PM, Yao Qi wrote:
> 
>> > So when setting a single-step breakpoint, we'd get the "kind"
>> > from the current mode, and when setting breakpoints from
>> > user-input, we'd get it from the symbols tables / mapping symbols.
> This means we need this to get the "kind" from the current mode,
> 
>   /* Return the breakpoint kind for this target based on the current
>      processor state (e.g. the current instruction mode on ARM) and the
>      PC.  The PCPTR is adjusted to the real memory location in case a flag
>      (e.g., the Thumb bit on ARM) is present in the PC.  */
>   int (*breakpoint_kind_from_current_state) (CORE_ADDR *pcptr);

Hmm, not sure, maybe.  From the perspective of syncing design with
gdbserver, it makes sense.  On gdbserver, I think that's only used for advancing
past a permanent breakpoint.  On gdb side, we use
gdbarch_skip_permanent_breakpoint, which I think no arch overrides currently,
so it always ends up in default_skip_permanent_breakpoint -> gdbarch_breakpoint_from_pc.
(Looks like we could eliminate gdbarch_skip_permanent_breakpoint.)
We could also say we should keep gdb's permanent breakpoint skipping's
logic of determining which breakpoint instruction to skip in sync with
bp_loc_is_permanent -> program_breakpoint_here_p -> gdbarch_breakpoint_from_pc,
though in this case looking at the current mode doesn't make sense.

> 
> and also need to pass breakpoint location to to_insert_breakpoint to get
> the type of breakpoint we are inserting, right?

I was thinking we'd have all we need in bp_target_info->placed_size,
but of course I may be missing something.

Thanks,
Pedro Alves

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

* Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep
  2016-03-23 17:26   ` Pedro Alves
@ 2016-05-09 11:05     ` Yao Qi
  0 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2016-05-09 11:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

Pedro Alves <palves@redhat.com> writes:

> I wasn't sure whether this is safe, but I convinced myself it is.
> I'd have been nice if there had been a note in the log about the below:
>
> Currently, when stepping past an instruction in the scratch pad, these
> archs won't ever reach the software_single_step method, always forcing a
> hardware single-step, even if the software_single_step method would
> insert some breakpoint.  The question is: is it safe now for them to
> analyse the instruction copied to the scratch pad, and potentially insert
> a software single-step?
>
> I think it is safe, because we won't ever use displaced stepping
> for the cases where the software_single_step method would return
> something non-NULL.  software_single_step returns non-NULL _only_

except displaced stepping on arm linux target.  However, GDB won't use
software single step for displaced stepping, because it either
single-step the instructions the scratch pad or resume the program from
the instructions in the scratch pad but these instructions end with a
breakpoint instruction (see arm_displaced_init_closure).

> for atomic regions, while OTOH, displaced_step_copy_insn always returns
> NULL for atomic regions.  E.g., notice how ppc_displaced_step_copy_insn
> vs ppc_deal_with_atomic_sequence.

I'd like to add some comments, see the patch below,

-- 
Yao (齐尧)
From c758e0080db93b26c88ab2d6dddb5451470c1a7f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 18 Mar 2016 16:25:38 +0000
Subject: [PATCH] Remove gdbarch method displaced_step_hw_singlestep

displaced_step_hw_singlestep was added for some targets, which can do
hardware single step, but need software single step for some special
instructions.  After we change gdbarch method software_single_step,
displaced_step_hw_singlestep is no longer necessary, because we can
get the same information from software_single_step.  This patch is
to remove displaced_step_hw_singlestep.

gdb:

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

	* aarch64-linux-tdep.c (aarch64_linux_init_abi): Don't call
	set_gdbarch_displaced_step_hw_singlestep.
	* aarch64-tdep.c (aarch64_displaced_step_hw_singlestep): Remove.
	* arch-utils.c (default_displaced_step_hw_singlestep): Remove.
	* arch-utils.h (default_displaced_step_hw_singlestep): Remove.
	* gdbarch.sh (displaced_step_hw_singlestep): Remove.
	* gdbarch.c, gdbarch.h: Regenerated.
	* infrun.c (resume): Don't call
	gdbarch_displaced_step_hw_singlestep.  Call
	gdbarch_software_single_step instead.
	* rs6000-tdep.c (ppc_displaced_step_hw_singlestep): Remove.
	(rs6000_gdbarch_init): Don't call
	set_gdbarch_displaced_step_hw_singlestep.
	* s390-linux-tdep.c (s390_displaced_step_hw_singlestep): Remove.
	(s390_gdbarch_init): Don't call
	set_gdbarch_displaced_step_hw_singlestep.

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index 651a0f0..8344d06 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1208,8 +1208,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_displaced_step_free_closure (gdbarch,
 					   simple_displaced_step_free_closure);
   set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_location);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch,
-					    aarch64_displaced_step_hw_singlestep);
 }
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 52c9ea6..460eade 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2637,15 +2637,6 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
     }
 }
 
-/* Implement the "displaced_step_hw_singlestep" gdbarch method.  */
-
-int
-aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				      struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
    architectures already created during this debugging session.
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c3d7802..3049252 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -67,13 +67,6 @@ simple_displaced_step_free_closure (struct gdbarch *gdbarch,
   xfree (closure);
 }
 
-int
-default_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				      struct displaced_step_closure *closure)
-{
-  return !gdbarch_software_single_step_p (gdbarch);
-}
-
 CORE_ADDR
 displaced_step_at_entry_point (struct gdbarch *gdbarch)
 {
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 9e1e70e..318e227 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -45,11 +45,6 @@ extern void
   simple_displaced_step_free_closure (struct gdbarch *gdbarch,
                                       struct displaced_step_closure *closure);
 
-/* Default implementation of gdbarch_displaced_hw_singlestep.  */
-extern int
-  default_displaced_step_hw_singlestep (struct gdbarch *,
-					struct displaced_step_closure *);
-
 /* Possible value for gdbarch_displaced_step_location:
    Place displaced instructions at the program's entry point,
    leaving space for inferior function call return breakpoints.  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index e59a93d..d469dc9 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -277,7 +277,6 @@ struct gdbarch
   gdbarch_skip_permanent_breakpoint_ftype *skip_permanent_breakpoint;
   ULONGEST max_insn_length;
   gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn;
-  gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep;
   gdbarch_displaced_step_fixup_ftype *displaced_step_fixup;
   gdbarch_displaced_step_free_closure_ftype *displaced_step_free_closure;
   gdbarch_displaced_step_location_ftype *displaced_step_location;
@@ -414,7 +413,6 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->adjust_dwarf2_line = default_adjust_dwarf2_line;
   gdbarch->register_reggroup_p = default_register_reggroup_p;
   gdbarch->skip_permanent_breakpoint = default_skip_permanent_breakpoint;
-  gdbarch->displaced_step_hw_singlestep = default_displaced_step_hw_singlestep;
   gdbarch->displaced_step_fixup = NULL;
   gdbarch->displaced_step_free_closure = NULL;
   gdbarch->displaced_step_location = NULL;
@@ -624,7 +622,6 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of skip_permanent_breakpoint, invalid_p == 0 */
   /* Skip verify of max_insn_length, has predicate.  */
   /* Skip verify of displaced_step_copy_insn, has predicate.  */
-  /* Skip verify of displaced_step_hw_singlestep, invalid_p == 0 */
   /* Skip verify of displaced_step_fixup, has predicate.  */
   if ((! gdbarch->displaced_step_free_closure) != (! gdbarch->displaced_step_copy_insn))
     fprintf_unfiltered (log, "\n\tdisplaced_step_free_closure");
@@ -873,9 +870,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: displaced_step_free_closure = <%s>\n",
                       host_address_to_string (gdbarch->displaced_step_free_closure));
   fprintf_unfiltered (file,
-                      "gdbarch_dump: displaced_step_hw_singlestep = <%s>\n",
-                      host_address_to_string (gdbarch->displaced_step_hw_singlestep));
-  fprintf_unfiltered (file,
                       "gdbarch_dump: displaced_step_location = <%s>\n",
                       host_address_to_string (gdbarch->displaced_step_location));
   fprintf_unfiltered (file,
@@ -3749,23 +3743,6 @@ set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch,
 }
 
 int
-gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure)
-{
-  gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->displaced_step_hw_singlestep != NULL);
-  if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_displaced_step_hw_singlestep called\n");
-  return gdbarch->displaced_step_hw_singlestep (gdbarch, closure);
-}
-
-void
-set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-                                          gdbarch_displaced_step_hw_singlestep_ftype displaced_step_hw_singlestep)
-{
-  gdbarch->displaced_step_hw_singlestep = displaced_step_hw_singlestep;
-}
-
-int
 gdbarch_displaced_step_fixup_p (struct gdbarch *gdbarch)
 {
   gdb_assert (gdbarch != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index e026c0e..da0edcc 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -957,20 +957,6 @@ typedef struct displaced_step_closure * (gdbarch_displaced_step_copy_insn_ftype)
 extern struct displaced_step_closure * gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
 extern void set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn);
 
-/* Return true if GDB should use hardware single-stepping to execute
-   the displaced instruction identified by CLOSURE.  If false,
-   GDB will simply restart execution at the displaced instruction
-   location, and it is up to the target to ensure GDB will receive
-   control again (e.g. by placing a software breakpoint instruction
-   into the displaced instruction buffer).
-  
-   The default implementation returns false on all targets that
-   provide a gdbarch_software_single_step routine, and true otherwise. */
-
-typedef int (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern int gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep);
-
 /* Fix up the state resulting from successfully single-stepping a
    displaced instruction, to give the result we would have gotten from
    stepping the instruction in its original location.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index b144c04..8b8710f 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -780,17 +780,6 @@ V:ULONGEST:max_insn_length:::0:0
 # that case.
 M:struct displaced_step_closure *:displaced_step_copy_insn:CORE_ADDR from, CORE_ADDR to, struct regcache *regs:from, to, regs
 
-# Return true if GDB should use hardware single-stepping to execute
-# the displaced instruction identified by CLOSURE.  If false,
-# GDB will simply restart execution at the displaced instruction
-# location, and it is up to the target to ensure GDB will receive
-# control again (e.g. by placing a software breakpoint instruction
-# into the displaced instruction buffer).
-#
-# The default implementation returns false on all targets that
-# provide a gdbarch_software_single_step routine, and true otherwise.
-m:int:displaced_step_hw_singlestep:struct displaced_step_closure *closure:closure::default_displaced_step_hw_singlestep::0
-
 # Fix up the state resulting from successfully single-stepping a
 # displaced instruction, to give the result we would have gotten from
 # stepping the instruction in its original location.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3ffec86..464e62a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2626,8 +2626,33 @@ resume (enum gdb_signal sig)
 	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
 
 	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
-	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
-						       displaced->step_closure);
+
+	  /* Although GDB won't use software single step for displaced
+	     stepping, we call software_single_step here to determine
+	     whether to
+	     - single step the instructions in the scratch pad, (like
+	     x86, ppc and aarch64),
+	     - or resume the program from the instructions in the scratch
+	     pad.  These instructions must end with a breakpoint
+	     instruction.  */
+	  if (gdbarch_software_single_step_p (gdbarch))
+	    {
+	      VEC (CORE_ADDR) * next_pcs;
+
+	      next_pcs = gdbarch_software_single_step (gdbarch,
+						       get_current_frame ());
+
+	      if (next_pcs != NULL)
+		{
+		  step = 0;
+		  VEC_free (CORE_ADDR, next_pcs);
+		}
+	      else
+		step = 1;
+	    }
+	  else
+	    step = 1;
+
 	}
     }
 
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 26c8ed9..5f0ede2 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -1136,15 +1136,6 @@ ppc_displaced_step_fixup (struct gdbarch *gdbarch,
 				    from + offset);
 }
 
-/* Always use hardware single-stepping to execute the
-   displaced instruction.  */
-static int
-ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				  struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
 /* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX
    instruction and ending with a STWCX/STDCX instruction.  If such a sequence
    is found, attempt to step through it.  A breakpoint is placed at the end of 
@@ -6071,8 +6062,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Setup displaced stepping.  */
   set_gdbarch_displaced_step_copy_insn (gdbarch,
 					ppc_displaced_step_copy_insn);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch,
-					    ppc_displaced_step_hw_singlestep);
+
   set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup);
   set_gdbarch_displaced_step_free_closure (gdbarch,
 					   simple_displaced_step_free_closure);
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index dd26ba6..77c7b33 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -759,14 +759,6 @@ s390_software_single_step (struct frame_info *frame)
   return next_pcs;
 }
 
-static int
-s390_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
-				   struct displaced_step_closure *closure)
-{
-  return 1;
-}
-
-
 /* Maps for register sets.  */
 
 static const struct regcache_map_entry s390_gregmap[] =
@@ -7990,7 +7982,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
   set_gdbarch_breakpoint_from_pc (gdbarch, s390_breakpoint_from_pc);
   set_gdbarch_software_single_step (gdbarch, s390_software_single_step);
-  set_gdbarch_displaced_step_hw_singlestep (gdbarch, s390_displaced_step_hw_singlestep);
   set_gdbarch_skip_prologue (gdbarch, s390_skip_prologue);
   set_gdbarch_stack_frame_destroyed_p (gdbarch, s390_stack_frame_destroyed_p);
 

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

end of thread, other threads:[~2016-05-09 11:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-23 14:10 [PATCH 0/2] gdbarch software_single_step returns VEC (CORE_ADDR) * Yao Qi
2016-03-23 14:10 ` [PATCH 1/2] " Yao Qi
2016-03-23 17:25   ` Pedro Alves
2016-03-30 14:14     ` Yao Qi
2016-04-27 15:19       ` Pedro Alves
2016-04-29 14:48         ` Yao Qi
2016-05-02 10:21           ` Pedro Alves
2016-03-23 14:10 ` [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep Yao Qi
2016-03-23 17:26   ` Pedro Alves
2016-05-09 11:05     ` 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).