Patch to speed up remote protocol
Andrew Cagney
ac131313@cygnus.com
Wed Jul 7 04:25:00 GMT 1999
Ian Lance Taylor wrote:
>
> The remote protocol code always retrieves all the register values
> before it stores a register value. That is required for older stubs,
> but it is a waste of time if the stub supports the 'P' request, which
> permits the protocol to set a specific register.
>
> This is particularly unfortunate because gdb likes to write the PC
> every time it continues, which means that when stepping through a
> program, gdb is continually and uselessly fetching all the register
> values, with horrible results over a slow serial line.
>
> This patch fixes this problem by skipping the fetch of all the
> register values when the 'P' request is known to work.
Hmm, good idea.
Attached is a patch that integrates your change into a general cleanup
of ``P'' packet support code.
The patch adds the commands:
set stub-supports-P-packet {supported,unsupported,auto}
show stub-supports-P-packet
so explicit examination/control is also possible. (I'm very interested
in suggestions for a better variable name). The bulk of the code is
also generic so that it can be re-used by code detecting other optional
packets.
One important difference between this and Ian's original patch is that
in the below refuses to allow a target to change its mind over ``P''
support.
Ian, can you give this a wirl?
enjoy,
Andrew
Wed Jul 7 19:29:14 1999 Andrew Cagney <cagney@b1.cygnus.com>
* remote.c (enum packet_support, enum packet_detect, struct
packet_config): Define.
(set_packet_config_cmd, show_packet_config_cmd,
add_packet_config_cmd, init_packet_config): New functions.
Generic support for optional packets.
(stub_supports_P): Change type to ``struct packet_config''.
(set_stub_supports_P_packet_cmd,
show_stub_supports_P_packet_cmd):
New functions.
(_initialize_remote): Add ``set stub-supports-P-packet''
command.
(remote_open_1, remote_async_open_1, remote_cisco_open):
Initialize ``stub_supports_P''.
(remote_store_registers): Re-write ``P'' probe logic.
(store_register_using_P): New function.
From Ian Lance Taylor <ian@airs.com>:
(remote_prepare_to_store): Only read registers when ``P'' packet
is in state unsupported or support-unknown.
Index: remote.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/remote.c,v
retrieving revision 1.201
diff -p -r1.201 remote.c
*** remote.c 1999/07/06 21:45:23 1.201
--- remote.c 1999/07/07 11:11:45
*************** void remote_console_output PARAMS ((char
*** 370,375 ****
--- 370,402 ----
static void check_binary_download PARAMS ((CORE_ADDR addr));
+ #if __STDC__
+ struct packet_config;
+ #endif
+
+ static void show_packet_config_cmd PARAMS ((struct packet_config *config));
+
+ static void set_packet_config_cmd PARAMS ((struct packet_config *config,
+ struct cmd_list_element *c));
+
+ static void add_packet_config_cmd PARAMS ((struct packet_config *config,
+ char *name,
+ void (*set_func) (char *args, int from_tty, struct cmd_list_element *c),
+ void (*show_func) (char *name, int from_tty),
+ char *doc));
+
+ static void init_packet_config PARAMS ((struct packet_config *config));
+
+ static void set_stub_supports_P_packet_cmd PARAMS ((char *args,
+ int from_tty,
+ struct cmd_list_element *c));
+
+ static void show_stub_supports_P_packet_cmd PARAMS ((char *args,
+ int from_tty));
+
+
+
+
/* Define the target subroutine names */
void open_remote_target PARAMS ((char *, int, struct target_ops *, int));
*************** static int remote_address_size;
*** 465,475 ****
trailers, it is only the payload size. */
static int remote_register_buf_size = 0;
! /* Should we try the 'P' request? If this is set to one when the stub
! doesn't support 'P', the only consequence is some unnecessary traffic. */
! static int stub_supports_P = 1;
/* Tokens for use by the asynchronous signal handlers for SIGINT */
PTR sigint_remote_twice_token;
PTR sigint_remote_token;
--- 492,637 ----
trailers, it is only the payload size. */
static int remote_register_buf_size = 0;
+
+ /* Generic configuration support for packets the stub optionally
+ supports. Allows the user to specify the use of the packet as well
+ as allowing GDB to auto-detect support in the remote stub. */
+
+ enum packet_support {
+ PACKET_SUPPORT_UNKNOWN = 0,
+ PACKET_SUPPORTED,
+ PACKET_UNSUPPORTED
+ };
+
+ enum packet_detect {
+ PACKET_AUTO_DETECT = 0,
+ PACKET_MANUAL_DETECT
+ };
+
+ struct packet_config
+ {
+ char *state;
+ char *name;
+ enum packet_detect detect;
+ enum packet_support support;
+ };
+
+ static char packet_config_auto[] = "auto";
+ static char packet_config_supported[] = "supported";
+ static char packet_config_unsupported[] = "unsupported";
+ static char *packet_config_enums[] = {
+ packet_config_auto,
+ packet_config_supported,
+ packet_config_unsupported,
+ 0,
+ };
+
+ static void
+ set_packet_config_cmd (config, c)
+ struct packet_config *config;
+ struct cmd_list_element *c;
+ {
+ if (config->state == packet_config_supported)
+ {
+ config->detect = PACKET_MANUAL_DETECT;
+ config->support = PACKET_SUPPORTED;
+ }
+ else if (config->state == packet_config_unsupported)
+ {
+ config->detect = PACKET_MANUAL_DETECT;
+ config->support = PACKET_UNSUPPORTED;
+ }
+ else if (config->state == packet_config_auto)
+ {
+ config->detect = PACKET_AUTO_DETECT;
+ config->support = PACKET_SUPPORT_UNKNOWN;
+ }
+ else
+ fatal ("Bad enum value");
+ }
! static void
! show_packet_config_cmd (config)
! struct packet_config *config;
! {
! char *support = "internal-error";
! switch (config->support)
! {
! case PACKET_SUPPORTED:
! support = "supported";
! break;
! case PACKET_UNSUPPORTED:
! support = "unsupported";
! break;
! case PACKET_SUPPORT_UNKNOWN:
! support = "unknown";
! break;
! }
! switch (config->detect)
! {
! case PACKET_AUTO_DETECT:
! printf_filtered ("%s is auto-detected, currently %s\n",
! config->name, support);
! break;
! case PACKET_MANUAL_DETECT:
! printf_filtered ("%s is currently %s\n",
! config->name, support);
! }
! }
+ static void
+ add_packet_config_cmd (config, name, set_func, show_func, doc)
+ struct packet_config *config;
+ char *name;
+ void (*set_func) PARAMS ((char *args, int from_tty,
+ struct cmd_list_element *c));
+ void (*show_func) PARAMS ((char *name, int from_tty));
+ char *doc;
+ {
+ struct cmd_list_element *c;
+ config->name = name;
+ c = add_set_enum_cmd (name, class_obscure, packet_config_enums,
+ (char *) &config->state,
+ doc, &setlist);
+ c->function.sfunc = set_func;
+ add_cmd (name, class_obscure, show_func, doc, &showlist);
+ }
+
+ static void
+ init_packet_config (config)
+ struct packet_config *config;
+ {
+ switch (config->detect)
+ {
+ case PACKET_AUTO_DETECT:
+ config->support = PACKET_SUPPORT_UNKNOWN;
+ break;
+ case PACKET_MANUAL_DETECT:
+ /* let the user beware */
+ break;
+ }
+ }
+
+ /* Should we try the 'P' (set register) request? */
+
+ static struct packet_config stub_supports_P;
+
+ static void
+ set_stub_supports_P_packet_cmd (char *args,
+ int from_tty,
+ struct cmd_list_element *c)
+ {
+ set_packet_config_cmd (&stub_supports_P, c);
+ }
+
+ static void
+ show_stub_supports_P_packet_cmd (char *args,
+ int from_tty)
+ {
+ show_packet_config_cmd (&stub_supports_P);
+ }
+
+
/* Tokens for use by the asynchronous signal handlers for SIGINT */
PTR sigint_remote_twice_token;
PTR sigint_remote_token;
*************** serial device is attached to the remote
*** 1711,1721 ****
}
push_target (target); /* Switch to using remote target now */
! /* Start out by trying the 'P' request to set registers. We set
! this each time that we open a new target so that if the user
! switches from one stub to another, we can (if the target is
! closed and reopened) cope. */
! stub_supports_P = 1;
general_thread = -2;
continue_thread = -2;
--- 1873,1879 ----
}
push_target (target); /* Switch to using remote target now */
! init_packet_config (&stub_supports_P);
general_thread = -2;
continue_thread = -2;
*************** serial device is attached to the remote
*** 1805,1815 ****
push_target (target); /* Switch to using remote target now */
! /* Start out by trying the 'P' request to set registers. We set
! this each time that we open a new target so that if the user
! switches from one stub to another, we can (if the target is
! closed and reopened) cope. */
! stub_supports_P = 1;
general_thread = -2;
continue_thread = -2;
--- 1963,1969 ----
push_target (target); /* Switch to using remote target now */
! init_packet_config (&stub_supports_P);
general_thread = -2;
continue_thread = -2;
*************** static void
*** 2728,2736 ****
remote_prepare_to_store ()
{
/* Make sure the entire registers array is valid. */
! read_register_bytes (0, (char *)NULL, REGISTER_BYTES);
}
/* Store register REGNO, or all registers if REGNO == -1, from the contents
of REGISTERS. FIXME: ignores errors. */
--- 2882,2926 ----
remote_prepare_to_store ()
{
/* Make sure the entire registers array is valid. */
! switch (stub_supports_P.support)
! {
! case PACKET_UNSUPPORTED:
! case PACKET_SUPPORT_UNKNOWN:
! read_register_bytes (0, (char *)NULL, REGISTER_BYTES);
! break;
! case PACKET_SUPPORTED:
! break;
! }
}
+ /* Helper: Attempt to store REGNO using the P packet. Return fail IFF
+ packet was not recognized. */
+
+ static int
+ store_register_using_P (regno)
+ int regno;
+ {
+ /* Try storing a single register. */
+ char *buf = alloca (PBUFSIZ);
+ char *regp;
+ char *p;
+ int i;
+
+ sprintf (buf, "P%x=", regno);
+ p = buf + strlen (buf);
+ regp = ®isters[REGISTER_BYTE (regno)];
+ for (i = 0; i < REGISTER_RAW_SIZE (regno); ++i)
+ {
+ *p++ = tohex ((regp[i] >> 4) & 0xf);
+ *p++ = tohex (regp[i] & 0xf);
+ }
+ *p = '\0';
+ remote_send (buf);
+
+ return buf[0] != '\0';
+ }
+
+
/* Store register REGNO, or all registers if REGNO == -1, from the contents
of REGISTERS. FIXME: ignores errors. */
*************** remote_store_registers (regno)
*** 2744,2774 ****
set_thread (inferior_pid, 1);
! if (regno >= 0 && stub_supports_P)
{
! /* Try storing a single register. */
! char *regp;
!
! sprintf (buf, "P%x=", regno);
! p = buf + strlen (buf);
! regp = ®isters[REGISTER_BYTE (regno)];
! for (i = 0; i < REGISTER_RAW_SIZE (regno); ++i)
! {
! *p++ = tohex ((regp[i] >> 4) & 0xf);
! *p++ = tohex (regp[i] & 0xf);
! }
! *p = '\0';
! remote_send (buf);
! if (buf[0] != '\0')
{
! /* The stub understands the 'P' request. We are done. */
! return;
}
-
- /* The stub does not support the 'P' request. Use 'G' instead,
- and don't try using 'P' in the future (it will just waste our
- time). */
- stub_supports_P = 0;
}
buf[0] = 'G';
--- 2934,2966 ----
set_thread (inferior_pid, 1);
! if (regno >= 0)
{
! switch (stub_supports_P.support)
{
! case PACKET_UNSUPPORTED:
! break;
! case PACKET_SUPPORTED:
! if (store_register_using_P (regno))
! return;
! else
! error ("Protocol error: P packet not recognized by stub");
! case PACKET_SUPPORT_UNKNOWN:
! if (store_register_using_P (regno))
! {
! /* The stub recognized the 'P' packet. Remember this. */
! stub_supports_P.support = PACKET_SUPPORTED;
! return;
! }
! else
! {
! /* The stub does not support the 'P' packet. Use 'G'
! instead, and don't try using 'P' in the future (it
! will just waste our time). */
! stub_supports_P.support = PACKET_UNSUPPORTED;
! break;
! }
}
}
buf[0] = 'G';
*************** device is attached to the remote system
*** 4392,4401 ****
push_target (&remote_cisco_ops); /* Switch to using cisco target now */
! /* Start out by trying the 'P' request to set registers. We set this each
! time that we open a new target so that if the user switches from one
! stub to another, we can (if the target is closed and reopened) cope. */
! stub_supports_P = 1;
general_thread = -2;
continue_thread = -2;
--- 4584,4590 ----
push_target (&remote_cisco_ops); /* Switch to using cisco target now */
! init_packet_config (&stub_supports_P);
general_thread = -2;
continue_thread = -2;
*************** in a memory packet.\n",
*** 4841,4844 ****
--- 5030,5037 ----
add_info ("remote-process", remote_info_process,
"Query the remote system for process info.");
+ add_packet_config_cmd (&stub_supports_P, "stub-supports-P-packet",
+ set_stub_supports_P_packet_cmd,
+ show_stub_supports_P_packet_cmd,
+ "Stub supports ``P'' (set register) packet");
}
More information about the Gdb-patches
mailing list