[PATCH 11/11] New target_ops hook to_can_do_single_step

[Yao Qi <qiyaoltc@gmail.com>]

Nowadays, GDB only knows whether architecture supports hardware single
step or software single step (through gdbarch hook software_single_step),
and for a given instruction or instruction sequence, GDB knows how to
do single step (hardware or software).  However, GDB doesn't know whether
the target supports hardware single step.  It is possible that the
architecture doesn't support hardware single step, such as arm, but
the target supports, such as simulator.  This was discussed in this
thread https://www.sourceware.org/ml/gdb/2009-12/msg00033.html before.

I encounter this problem for aarch64 multi-arch support.  When aarch64
debugs arm program, gdbarch is arm, so software single step is still
used.  However, the underneath linux kernel does support hardware
single step, so IWBN to use it.

This patch is to add a new target_ops hook to_can_do_single_step, and
only use it in arm_linux_software_single_step to decide whether or not
to use hardware single step.  On the remote target, if the target
supports s and S actions in the vCont? reply, then target can do single
step.  On the native aarch64 linux target, 1 is returned.  On other
targets, -1 is returned.

gdb:

2015-06-30  Yao Qi  <yao.qi@linaro.org>

	* aarch64-linux-nat.c (aarch64_linux_can_do_single_step): New function.
	(_initialize_aarch64_linux_nat): Install it to to_can_do_single_step.
	* arm-linux-tdep.c (arm_linux_software_single_step): Return 0
	if target_can_do_single_step returns 1.
	* remote.c (struct vCont_action_support) <s>: New field.
	(remote_vcont_probe): Remove support_s and use
	rs->supports_vCont.s instead.
	(remote_can_do_single_step): New function.
	(init_remote_ops): Install it to to_can_do_single_step.
	* target.h (struct target_ops) <to_can_do_single_step>: New field.
	(target_can_do_single_step): New macro.
	* target-delegates.c: Re-generated.
---
 gdb/aarch64-linux-nat.c |  9 +++++++++
 gdb/arm-linux-tdep.c    |  5 +++++
 gdb/remote.c            | 33 +++++++++++++++++++++++++++------
 gdb/target-delegates.c  | 31 +++++++++++++++++++++++++++++++
 gdb/target.h            |  9 +++++++++
 5 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 9814dce..be9a91a 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -1607,6 +1607,14 @@ aarch64_linux_watchpoint_addr_within_range (struct target_ops *target,
   return start <= addr && start + length - 1 >= addr;
 }
 
+/* Implement the "to_can_do_single_step" target_ops method.  */
+
+static int
+aarch64_linux_can_do_single_step (struct target_ops *target)
+{
+  return 1;
+}
+
 /* Define AArch64 maintenance commands.  */
 
 static void
@@ -1658,6 +1666,7 @@ _initialize_aarch64_linux_nat (void)
   t->to_stopped_data_address = aarch64_linux_stopped_data_address;
   t->to_watchpoint_addr_within_range =
     aarch64_linux_watchpoint_addr_within_range;
+  t->to_can_do_single_step = aarch64_linux_can_do_single_step;
 
   /* Override the GNU/Linux inferior startup hook.  */
   super_post_startup_inferior = t->to_post_startup_inferior;
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 6273027..84afdd4 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -917,6 +917,11 @@ arm_linux_software_single_step (struct frame_info *frame)
   if (arm_deal_with_atomic_sequence (frame))
     return 1;
 
+  /* 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;
+
   next_pc = arm_get_next_pc (frame, get_frame_pc (frame));
 
   /* The Linux kernel offers some user-mode helpers in a high page.  We can
diff --git a/gdb/remote.c b/gdb/remote.c
index 68dd99d..16f74c9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -245,6 +245,12 @@ struct vCont_action_support
 
   /* vCont;r */
   int r;
+
+  /* vCont;s */
+  int s;
+
+  /* vCont;S */
+  int S;
 };
 
 /* Controls whether GDB is willing to use range stepping.  */
@@ -4818,10 +4824,10 @@ remote_vcont_probe (struct remote_state *rs)
   if (startswith (buf, "vCont"))
     {
       char *p = &buf[5];
-      int support_s, support_S, support_c, support_C;
+      int support_c, support_C;
 
-      support_s = 0;
-      support_S = 0;
+      rs->supports_vCont.s = 0;
+      rs->supports_vCont.S = 0;
       support_c = 0;
       support_C = 0;
       rs->supports_vCont.t = 0;
@@ -4830,9 +4836,9 @@ remote_vcont_probe (struct remote_state *rs)
 	{
 	  p++;
 	  if (*p == 's' && (*(p + 1) == ';' || *(p + 1) == 0))
-	    support_s = 1;
+	    rs->supports_vCont.s = 1;
 	  else if (*p == 'S' && (*(p + 1) == ';' || *(p + 1) == 0))
-	    support_S = 1;
+	    rs->supports_vCont.S = 1;
 	  else if (*p == 'c' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    support_c = 1;
 	  else if (*p == 'C' && (*(p + 1) == ';' || *(p + 1) == 0))
@@ -4847,7 +4853,8 @@ remote_vcont_probe (struct remote_state *rs)
 
       /* If s, S, c, and C are not all supported, we can't use vCont.  Clearing
          BUF will make packet_ok disable the packet.  */
-      if (!support_s || !support_S || !support_c || !support_C)
+      if (!rs->supports_vCont.s || !rs->supports_vCont.S
+	  || !support_c || !support_C)
 	buf[0] = 0;
     }
 
@@ -12147,6 +12154,19 @@ remote_pid_to_exec_file (struct target_ops *self, int pid)
   return filename;
 }
 
+/* Implement the to_can_do_single_step target_ops method.  */
+
+static int
+remote_can_do_single_step (struct target_ops *ops)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+    remote_vcont_probe (rs);
+
+  return rs->supports_vCont.s && rs->supports_vCont.S;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -12216,6 +12236,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_can_async_p = remote_can_async_p;
   remote_ops.to_is_async_p = remote_is_async_p;
   remote_ops.to_async = remote_async;
+  remote_ops.to_can_do_single_step = remote_can_do_single_step;
   remote_ops.to_terminal_inferior = remote_terminal_inferior;
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 36eacbf..b55a1b1 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -832,6 +832,33 @@ debug_masked_watch_num_registers (struct target_ops *self, CORE_ADDR arg1, CORE_
   return result;
 }
 
+static int
+delegate_can_do_single_step (struct target_ops *self)
+{
+  self = self->beneath;
+  return self->to_can_do_single_step (self);
+}
+
+static int
+tdefault_can_do_single_step (struct target_ops *self)
+{
+  return -1;
+}
+
+static int
+debug_can_do_single_step (struct target_ops *self)
+{
+  int result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_can_do_single_step (...)\n", debug_target.to_shortname);
+  result = debug_target.to_can_do_single_step (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_can_do_single_step (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_int (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 static void
 delegate_terminal_init (struct target_ops *self)
 {
@@ -3935,6 +3962,8 @@ install_delegators (struct target_ops *ops)
     ops->to_can_accel_watchpoint_condition = delegate_can_accel_watchpoint_condition;
   if (ops->to_masked_watch_num_registers == NULL)
     ops->to_masked_watch_num_registers = delegate_masked_watch_num_registers;
+  if (ops->to_can_do_single_step == NULL)
+    ops->to_can_do_single_step = delegate_can_do_single_step;
   if (ops->to_terminal_init == NULL)
     ops->to_terminal_init = delegate_terminal_init;
   if (ops->to_terminal_inferior == NULL)
@@ -4197,6 +4226,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_region_ok_for_hw_watchpoint = default_region_ok_for_hw_watchpoint;
   ops->to_can_accel_watchpoint_condition = tdefault_can_accel_watchpoint_condition;
   ops->to_masked_watch_num_registers = tdefault_masked_watch_num_registers;
+  ops->to_can_do_single_step = tdefault_can_do_single_step;
   ops->to_terminal_init = tdefault_terminal_init;
   ops->to_terminal_inferior = tdefault_terminal_inferior;
   ops->to_terminal_ours_for_output = tdefault_terminal_ours_for_output;
@@ -4345,6 +4375,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_region_ok_for_hw_watchpoint = debug_region_ok_for_hw_watchpoint;
   ops->to_can_accel_watchpoint_condition = debug_can_accel_watchpoint_condition;
   ops->to_masked_watch_num_registers = debug_masked_watch_num_registers;
+  ops->to_can_do_single_step = debug_can_do_single_step;
   ops->to_terminal_init = debug_terminal_init;
   ops->to_terminal_inferior = debug_terminal_inferior;
   ops->to_terminal_ours_for_output = debug_terminal_ours_for_output;
diff --git a/gdb/target.h b/gdb/target.h
index 32234f7..a312664 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -557,6 +557,12 @@ struct target_ops
     int (*to_masked_watch_num_registers) (struct target_ops *,
 					  CORE_ADDR, CORE_ADDR)
       TARGET_DEFAULT_RETURN (-1);
+
+    /* Return 1 for sure target can do single step.  Return -1 for
+       unknown.  Return 0 for target can't do.  */
+    int (*to_can_do_single_step) (struct target_ops *)
+      TARGET_DEFAULT_RETURN (-1);
+
     void (*to_terminal_init) (struct target_ops *)
       TARGET_DEFAULT_IGNORE ();
     void (*to_terminal_inferior) (struct target_ops *)
@@ -1861,6 +1867,9 @@ extern char *target_thread_name (struct thread_info *);
 						      addr, len)
 
 
+#define target_can_do_single_step() \
+  (*current_target.to_can_do_single_step) (&current_target)
+
 /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes.
    TYPE is 0 for write, 1 for read, and 2 for read/write accesses.
    COND is the expression for its condition, or NULL if there's none.
-- 
1.9.1