From 750ce8d1caf237464dd663de54ad055c0e58409d Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Tue, 15 Sep 2015 14:09:18 +0100 Subject: [PATCH] Support single step by arch or target 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 native aarch64 linux target, 1 is returned. On other targets, -1 is returned. On the remote target, if the target supports s and S actions in the vCont? reply, then target can do single step. However, old GDBserver will send s and S in the reply to vCont?, which will confuse new GDB. For example, old GDBserver on arm-linux will send s and S in the reply to vCont?, but it doesn't support hardware single step. On the other hand, new GDBserver, on arm-linux for example, will not send s and S in the reply to vCont?, but old GDB thinks it doesn't support vCont packet at all. In order to address this problem, I add a new qSupported feature vContSupported, which indicates GDB wants to know the supported actions in the reply to vCont?, and qSupported response contains vContSupported if the stub is able tell supported vCont actions in the reply of vCont?. If the patched GDB talks with patched GDBserver on x86, the RSP traffic is like this: -> $qSupported:...+;vContSupported+ <- ...+;vContSupported+ ... -> $vCont? <- vCont;c;C;t;s;S;r then, GDB knows the stub can do single step, and may stop using software single step even the architecture doesn't support hardware single step. If the patched GDB talks with patched GDBserver on arm, the last vCont? reply will become: <- vCont;c;C;t GDB thinks the target doesn't support single step, so it will use software single step. If the patched GDB talks with unpatched GDBserver, the RSP traffic is like this: -> $qSupported:...+;vContSupported+ <- ...+ ... -> $vCont? <- vCont;c;C;t;s;S;r although GDBserver returns s and S, GDB still thinks GDBserver may not support single step because it doesn't support vContSupported. If the unpatched GDB talks with patched GDBserver on x86, the RSP traffic is like: -> $qSupported:...+; <- ...+;vContSupported+ ... -> $vCont? <- vCont;c;C;t;s;S;r Since GDB doesn't sent vContSupported in the qSupported feature, GDBserver sends s and S regardless of the support of hardware single step. gdb: 2015-09-15 Yao Qi * 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) : New fields. (PACKET_vContSupported): New enum. (remote_protocol_features): New element for vContSupported. (remote_query_supported): Append "vContSupported+". (remote_vcont_probe): Remove support_s and support_S, use rs->supports_vCont.s and rs->supports_vCont.S instead. Disable vCont packet if c and C actions are not supported. (remote_can_do_single_step): New function. (init_remote_ops): Install it to to_can_do_single_step. (_initialize_remote): Call add_packet_config_cmd. * target.h (struct target_ops) : New field. (target_can_do_single_step): New macro. * target-delegates.c: Re-generated. gdb/gdbserver: 2015-09-15 Yao Qi * server.c (vCont_supported): New global variable. (handle_query): Set vCont_supported to 1 if "vContSupported+" matches. Append ";vContSupported+" to own_buf. (handle_v_requests): Append ";s;S" to own_buf if target supports hardware single step or vCont_supported is false. (capture_main): Set vCont_supported to zero. gdb/doc: 2015-09-15 Yao Qi * gdb.texinfo (General Query Packets): Add vContSupported to tables of 'gdbfeatures' and 'stub features' supported in the qSupported packet, as well as to the list containing stub feature details. --- gdb/ChangeLog | 21 +++++++++++++++ gdb/aarch64-linux-nat.c | 9 +++++++ gdb/arm-linux-tdep.c | 5 ++++ gdb/doc/ChangeLog | 7 +++++ gdb/doc/gdb.texinfo | 8 ++++++ gdb/gdbserver/ChangeLog | 9 +++++++ gdb/gdbserver/server.c | 23 +++++++++++++++- gdb/remote.c | 58 ++++++++++++++++++++++++++++++++++------- gdb/target-delegates.c | 31 ++++++++++++++++++++++ gdb/target.h | 9 +++++++ 10 files changed, 170 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index aefa3019b8..9932d60edb 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,24 @@ +2015-09-15 Yao Qi + + * 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) : New fields. + (PACKET_vContSupported): New enum. + (remote_protocol_features): New element for vContSupported. + (remote_query_supported): Append "vContSupported+". + (remote_vcont_probe): Remove support_s and support_S, use + rs->supports_vCont.s and rs->supports_vCont.S instead. Disable + vCont packet if c and C actions are not supported. + (remote_can_do_single_step): New function. + (init_remote_ops): Install it to to_can_do_single_step. + (_initialize_remote): Call add_packet_config_cmd. + * target.h (struct target_ops) : New field. + (target_can_do_single_step): New macro. + * target-delegates.c: Re-generated. + 2015-09-15 Yao Qi * aarch64-linux-nat.c (aarch64_linux_siginfo_fixup): New function. diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index ddb2788ba8..d7ac19eb5a 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -807,6 +807,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 @@ -858,6 +866,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 b3ad8686e4..bc2cec4197 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/doc/ChangeLog b/gdb/doc/ChangeLog index 1f99a2f35f..b5de9691bd 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,10 @@ +2015-09-15 Yao Qi + + * gdb.texinfo (General Query Packets): Add vContSupported to + tables of 'gdbfeatures' and 'stub features' supported in the + qSupported packet, as well as to the list containing stub + feature details. + 2015-09-11 Don Breazeal * gdb.texinfo (Remote Configuration): Add exec event diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 395f0d479f..4ecdb8fc7b 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -36132,6 +36132,10 @@ This feature indicates whether @value{GDBN} supports exec event extensions to the remote protocol. @value{GDBN} does not use such extensions unless the stub also reports that it supports them by including @samp{exec-events+} in its @samp{qSupported} reply. + +@item vContSupported +This feature indicates whether @value{GDBN} wants to know the +supported actions in the reply to @samp{vCont?} packet. @end table Stubs should ignore any unknown values for @@ -36608,6 +36612,10 @@ and vforkdone events. @item exec-events The remote stub reports the @samp{exec} stop reason for exec events. +@item vContSupported +The remote stub reports the supported actions in the reply to +@samp{vCont?} packet. + @end table @item qSymbol:: diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 7831b858f9..fac52efe9a 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,12 @@ +2015-09-15 Yao Qi + + * server.c (vCont_supported): New global variable. + (handle_query): Set vCont_supported to 1 if "vContSupported+" + matches. Append ";vContSupported+" to own_buf. + (handle_v_requests): Append ";s;S" to own_buf if target supports + hardware single step or vCont_supported is false. + (capture_main): Set vCont_supported to zero. + 2015-09-15 Yao Qi * linux-low.c (linux_supports_conditional_breakpoints): Rename diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 1481c47dc4..3b9ec9813d 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -64,6 +64,11 @@ int non_stop; int swbreak_feature; int hwbreak_feature; +/* True if the "vContSupported" feature is active. In that case, GDB + wants us to report whether single step is supported in the reply to + "vCont?" packet. */ +static int vCont_supported; + /* Whether we should attempt to disable the operating system's address space randomization feature before starting an inferior. */ int disable_randomization = 1; @@ -2118,6 +2123,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_exec_events ()) report_exec_events = 1; } + else if (strcmp (p, "vContSupported+") == 0) + vCont_supported = 1; else target_process_qsupported (p); @@ -2227,6 +2234,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (the_target->pid_to_exec_file != NULL) strcat (own_buf, ";qXfer:exec-file:read+"); + strcat (own_buf, ";vContSupported+"); + /* Reinitialize components as needed for the new connection. */ hostio_handle_new_gdb_connection (); target_handle_new_gdb_connection (); @@ -2825,7 +2834,18 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len) if (startswith (own_buf, "vCont?")) { - strcpy (own_buf, "vCont;c;C;s;S;t"); + strcpy (own_buf, "vCont;c;C;t"); + + if (target_supports_hardware_single_step () || !vCont_supported) + { + /* If target supports hardware single step, add actions s + and S to the list of supported actions. On the other + hand, if GDB doesn't request the supported vCont actions + in qSupported packet, add s and S to the list too. */ + own_buf = own_buf + strlen (own_buf); + strcpy (own_buf, ";s;S"); + } + if (target_supports_range_stepping ()) { own_buf = own_buf + strlen (own_buf); @@ -3566,6 +3586,7 @@ captured_main (int argc, char *argv[]) cont_thread = null_ptid; swbreak_feature = 0; hwbreak_feature = 0; + vCont_supported = 0; remote_open (port); diff --git a/gdb/remote.c b/gdb/remote.c index db83e6b918..b9dc4afb04 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -260,6 +260,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. */ @@ -1466,6 +1472,9 @@ enum { /* Support for exec events. */ PACKET_exec_event_feature, + /* Support for query supported vCont actions. */ + PACKET_vContSupported, + PACKET_MAX }; @@ -4375,7 +4384,8 @@ static const struct protocol_feature remote_protocol_features[] = { { "exec-events", PACKET_DISABLE, remote_supported_packet, PACKET_exec_event_feature }, { "Qbtrace-conf:pt:size", PACKET_DISABLE, remote_supported_packet, - PACKET_Qbtrace_conf_pt_size } + PACKET_Qbtrace_conf_pt_size }, + { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported } }; static char *remote_support_xml; @@ -4468,6 +4478,9 @@ remote_query_supported (void) q = remote_query_supported_append (q, "exec-events+"); } + if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE) + q = remote_query_supported_append (q, "vContSupported+"); + q = reconcat (q, "qSupported:", q, (char *) NULL); putpkt (q); @@ -5061,10 +5074,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; @@ -5073,9 +5086,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)) @@ -5088,9 +5101,9 @@ remote_vcont_probe (struct remote_state *rs) p = strchr (p, ';'); } - /* 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 c, and C are not all supported, we can't use vCont. Clearing + BUF will make packet_ok disable the packet. */ + if (!support_c || !support_C) buf[0] = 0; } @@ -12653,6 +12666,29 @@ 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) +{ + /* We can only tell whether target supports single step or not by + supported s and S vCont actions if the stub supports vContSupported + feature. If the stub doesn't support vContSupported feature, + we have conservatively to think target doesn't supports single + step. */ + if (packet_support (PACKET_vContSupported) == PACKET_ENABLE) + { + 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; + } + else + return 0; +} + static void init_remote_ops (void) { @@ -12724,6 +12760,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; @@ -13431,6 +13468,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_conf_pt_size], "Qbtrace-conf:pt:size", "btrace-conf-pt-size", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_vContSupported], + "vContSupported", "verbose-resume-supported", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_exec_event_feature], "exec-event-feature", "exec-event-feature", 0); diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 87197f8c51..c358814324 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -826,6 +826,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) { @@ -4028,6 +4055,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) @@ -4298,6 +4327,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; @@ -4450,6 +4480,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 5f05b5651c..2875ad4768 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -559,6 +559,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 *) @@ -1910,6 +1916,9 @@ extern char *target_thread_name (struct thread_info *); addr, len) +#define target_can_do_single_step() \ + (*current_target.to_can_do_single_step) (¤t_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.