RFA/RFC: vCont for the remote protocol [client]
Daniel Jacobowitz
drow@mvista.com
Thu Oct 16 20:31:00 GMT 2003
On Tue, Oct 14, 2003 at 08:14:52PM -0400, Andrew Cagney wrote:
> [I think my GNU e-mail is down] [I'm also having trouble reading this
> unified diff, so watch out :-)]
>
> +/* Check for the availability of vCont. This function should also check
> + the response. */
>
> see below ...
>
> > static void
> >-remote_resume (ptid_t ptid, int step, enum target_signal siggnal)
> >+remote_vcont_probe (struct remote_state *rs, char *buf)
> > {
> >- struct remote_state *rs = get_remote_state ();
> >- char *buf = alloca (rs->remote_packet_size);
> >- int pid = PIDGET (ptid);
> >- char *p;
> >-
> >- if (pid == -1)
> >- set_thread (0, 0); /* run any thread */
> >- else
> >- set_thread (pid, 0); /* run this thread */
> >-
> >- last_sent_signal = siggnal;
> >- last_sent_step = step;
> >+ strcpy (buf, "vCont?");
> >+ putpkt (buf);
> >+ getpkt (buf, rs->remote_packet_size, 0);
> >+ packet_ok (buf, &remote_protocol_vcont);
> >+}
>
> packet_ok will go through the path:
>
> /* The packet may or may not be OK. Just assume it is */
> return PACKET_OK;
>
> Shouldn't remote_vcont_probe apply additional sanity checks. For
> instance that the response is well formed, and that all the letters
> [SCsc] appeared? If they are not all present, just mark the packet is
> not supported for now. (But also comment this).
The "should" there was supposed to imply "but we don't". But hey, it
was easy, so fixed.
> +static int
> +remote_vcont_resume (ptid_t ptid, int step, enum target_signal siggnal)
>
> What's the return value?
>From the comment a few lines up.
This function returns
non-zero iff it resumes the inferior.
> +{
> + struct remote_state *rs = get_remote_state ();
> + int pid = PIDGET (ptid);
> + char *buf = alloca (rs->remote_packet_size);
>
> Rather than ALLOCA here, can you ...
>
> + /* If we could generate a wider range of packets, we'd have to worry
> + about overflowing BUF. Should there be a generic
> + "multi-part-packet" packet? */
> +
> + if (PIDGET (inferior_ptid) == MAGIC_NULL_PID)
> + {
> + /* MAGIC_NULL_PTID means that we don't have any active threads, so we
> + don't have any PID numbers the inferior will understand. Make sure
> + to only send forms that do not specify a PID. */
> + if (step && siggnal != TARGET_SIGNAL_0)
> + sprintf (buf, "vCont;S%02x", siggnal);
>
> ... use XASPRINTF here (along with some sort of cleanup).
Is GDB trying to move away from alloca? The internals manual says:
GDB can use the non-portable function `alloca' for the allocation of
small temporary values (such as strings).
So I use it to avoid cleanups. OTOH, it occurs to me that
rs->remote_packet_size is a bit large; OTOOH, remote.c uses this idiom
all over the place already.
I've used xmalloc instead, since the buf is used for getpkt and thus
must be remote_packet_size large.
Here's what I am about to check in.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
2003-10-16 Daniel Jacobowitz <drow@mvista.com>
* remote.c (remote_protocol_vcont): New variable.
(set_remote_protocol_vcont_packet_cmd): New function.
(show_remote_protocol_vcont_packet_cmd): New function.
(init_all_packet_configs): Handle remote_protocol_vcont.
(remote_vcont_probe): New function.
(remote_vcont_resume): New function.
(remote_resume): Use it.
(remote_async_resume): Call remote_resume.
(_initialize_remote): Add verbose-resume packet commands.
Index: gdb/remote.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/remote.c,v
retrieving revision 1.117
diff -u -p -r1.117 remote.c
--- gdb/remote.c 15 Oct 2003 21:10:55 -0000 1.117
+++ gdb/remote.c 16 Oct 2003 20:09:26 -0000
@@ -749,6 +749,23 @@ packet_ok (const char *buf, struct packe
}
}
+/* Should we try the 'vCont' (descriptive resume) request? */
+static struct packet_config remote_protocol_vcont;
+
+static void
+set_remote_protocol_vcont_packet_cmd (char *args, int from_tty,
+ struct cmd_list_element *c)
+{
+ update_packet_config (&remote_protocol_vcont);
+}
+
+static void
+show_remote_protocol_vcont_packet_cmd (char *args, int from_tty,
+ struct cmd_list_element *c)
+{
+ show_packet_config_cmd (&remote_protocol_vcont);
+}
+
/* Should we try the 'qSymbol' (target symbol lookup service) request? */
static struct packet_config remote_protocol_qSymbol;
@@ -2190,6 +2207,7 @@ init_all_packet_configs (void)
update_packet_config (&remote_protocol_E);
update_packet_config (&remote_protocol_P);
update_packet_config (&remote_protocol_qSymbol);
+ update_packet_config (&remote_protocol_vcont);
for (i = 0; i < NR_Z_PACKET_TYPES; i++)
update_packet_config (&remote_protocol_Z[i]);
/* Force remote_write_bytes to check whether target supports binary
@@ -2503,115 +2521,144 @@ bin2hex (const char *bin, char *hex, int
return i;
}
-/* Tell the remote machine to resume. */
-
-static enum target_signal last_sent_signal = TARGET_SIGNAL_0;
-
-static int last_sent_step;
+/* Check for the availability of vCont. This function should also check
+ the response. */
static void
-remote_resume (ptid_t ptid, int step, enum target_signal siggnal)
+remote_vcont_probe (struct remote_state *rs, char *buf)
{
- struct remote_state *rs = get_remote_state ();
- char *buf = alloca (rs->remote_packet_size);
- int pid = PIDGET (ptid);
- char *p;
+ strcpy (buf, "vCont?");
+ putpkt (buf);
+ getpkt (buf, rs->remote_packet_size, 0);
- if (pid == -1)
- set_thread (0, 0); /* run any thread */
- else
- set_thread (pid, 0); /* run this thread */
+ /* Make sure that the features we assume are supported. */
+ if (strncmp (buf, "vCont", 5) == 0)
+ {
+ char *p = &buf[5];
+ int support_s, support_S, support_c, support_C;
- last_sent_signal = siggnal;
- last_sent_step = step;
+ support_s = 0;
+ support_S = 0;
+ support_c = 0;
+ support_C = 0;
+ while (p && *p == ';')
+ {
+ p++;
+ if (*p == 's' && (*(p + 1) == ';' || *(p + 1) == 0))
+ support_s = 1;
+ else if (*p == 'S' && (*(p + 1) == ';' || *(p + 1) == 0))
+ support_S = 1;
+ else if (*p == 'c' && (*(p + 1) == ';' || *(p + 1) == 0))
+ support_c = 1;
+ else if (*p == 'C' && (*(p + 1) == ';' || *(p + 1) == 0))
+ support_C = 1;
- /* A hook for when we need to do something at the last moment before
- resumption. */
- if (target_resume_hook)
- (*target_resume_hook) ();
+ 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)
+ buf[0] = 0;
+ }
- /* The s/S/c/C packets do not return status. So if the target does
- not support the S or C packets, the debug agent returns an empty
- string which is detected in remote_wait(). This protocol defect
- is fixed in the e/E packets. */
+ packet_ok (buf, &remote_protocol_vcont);
+}
- if (step && step_range_end)
- {
- /* If the target does not support the 'E' packet, we try the 'S'
- packet. Ideally we would fall back to the 'e' packet if that
- too is not supported. But that would require another copy of
- the code to issue the 'e' packet (and fall back to 's' if not
- supported) in remote_wait(). */
-
- if (siggnal != TARGET_SIGNAL_0)
- {
- if (remote_protocol_E.support != PACKET_DISABLE)
- {
- p = buf;
- *p++ = 'E';
- *p++ = tohex (((int) siggnal >> 4) & 0xf);
- *p++ = tohex (((int) siggnal) & 0xf);
- *p++ = ',';
- p += hexnumstr (p, (ULONGEST) step_range_start);
- *p++ = ',';
- p += hexnumstr (p, (ULONGEST) step_range_end);
- *p++ = 0;
+/* Resume the remote inferior by using a "vCont" packet. The thread
+ to be resumed is PTID; STEP and SIGGNAL indicate whether the
+ resumed thread should be single-stepped and/or signalled. If PTID's
+ PID is -1, then all threads are resumed; the thread to be stepped and/or
+ signalled is given in the global INFERIOR_PTID. This function returns
+ non-zero iff it resumes the inferior.
- putpkt (buf);
- getpkt (buf, (rs->remote_packet_size), 0);
+ This function issues a strict subset of all possible vCont commands at the
+ moment. */
- if (packet_ok (buf, &remote_protocol_E) == PACKET_OK)
- return;
- }
- }
- else
- {
- if (remote_protocol_e.support != PACKET_DISABLE)
- {
- p = buf;
- *p++ = 'e';
- p += hexnumstr (p, (ULONGEST) step_range_start);
- *p++ = ',';
- p += hexnumstr (p, (ULONGEST) step_range_end);
- *p++ = 0;
+static int
+remote_vcont_resume (ptid_t ptid, int step, enum target_signal siggnal)
+{
+ struct remote_state *rs = get_remote_state ();
+ int pid = PIDGET (ptid);
+ char *buf = NULL;
+ struct cleanup *old_cleanup;
- putpkt (buf);
- getpkt (buf, (rs->remote_packet_size), 0);
+ buf = xmalloc (rs->remote_packet_size);
+ old_cleanup = make_cleanup (xfree, buf);
- if (packet_ok (buf, &remote_protocol_e) == PACKET_OK)
- return;
- }
- }
+ if (remote_protocol_vcont.support == PACKET_SUPPORT_UNKNOWN)
+ remote_vcont_probe (rs, buf);
+
+ if (remote_protocol_vcont.support == PACKET_DISABLE)
+ {
+ do_cleanups (old_cleanup);
+ return 0;
}
- if (siggnal != TARGET_SIGNAL_0)
+ /* If we could generate a wider range of packets, we'd have to worry
+ about overflowing BUF. Should there be a generic
+ "multi-part-packet" packet? */
+
+ if (PIDGET (inferior_ptid) == MAGIC_NULL_PID)
+ {
+ /* MAGIC_NULL_PTID means that we don't have any active threads, so we
+ don't have any PID numbers the inferior will understand. Make sure
+ to only send forms that do not specify a PID. */
+ if (step && siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;S%02x", siggnal);
+ else if (step)
+ sprintf (buf, "vCont;s");
+ else if (siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;C%02x", siggnal);
+ else
+ sprintf (buf, "vCont;c");
+ }
+ else if (pid == -1)
{
- buf[0] = step ? 'S' : 'C';
- buf[1] = tohex (((int) siggnal >> 4) & 0xf);
- buf[2] = tohex (((int) siggnal) & 0xf);
- buf[3] = '\0';
+ /* Resume all threads, with preference for INFERIOR_PTID. */
+ if (step && siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;S%02x:%x;c", siggnal, PIDGET (inferior_ptid));
+ else if (step)
+ sprintf (buf, "vCont;s:%x;c", PIDGET (inferior_ptid));
+ else if (siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;C%02x:%x;c", siggnal, PIDGET (inferior_ptid));
+ else
+ sprintf (buf, "vCont;c");
}
else
- strcpy (buf, step ? "s" : "c");
+ {
+ /* Scheduler locking; resume only PTID. */
+ if (step && siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;S%02x:%x", siggnal, pid);
+ else if (step)
+ sprintf (buf, "vCont;s:%x", pid);
+ else if (siggnal != TARGET_SIGNAL_0)
+ sprintf (buf, "vCont;C%02x:%x", siggnal, pid);
+ else
+ sprintf (buf, "vCont;c:%x", pid);
+ }
putpkt (buf);
+
+ do_cleanups (old_cleanup);
+
+ return 1;
}
-/* Same as remote_resume, but with async support. */
+/* Tell the remote machine to resume. */
+
+static enum target_signal last_sent_signal = TARGET_SIGNAL_0;
+
+static int last_sent_step;
+
static void
-remote_async_resume (ptid_t ptid, int step, enum target_signal siggnal)
+remote_resume (ptid_t ptid, int step, enum target_signal siggnal)
{
struct remote_state *rs = get_remote_state ();
char *buf = alloca (rs->remote_packet_size);
int pid = PIDGET (ptid);
char *p;
- if (pid == -1)
- set_thread (0, 0); /* run any thread */
- else
- set_thread (pid, 0); /* run this thread */
-
last_sent_signal = siggnal;
last_sent_step = step;
@@ -2620,6 +2667,16 @@ remote_async_resume (ptid_t ptid, int st
if (target_resume_hook)
(*target_resume_hook) ();
+ /* The vCont packet doesn't need to specify threads via Hc. */
+ if (remote_vcont_resume (ptid, step, siggnal))
+ return;
+
+ /* All other supported resume packets do use Hc, so call set_thread. */
+ if (pid == -1)
+ set_thread (0, 0); /* run any thread */
+ else
+ set_thread (pid, 0); /* run this thread */
+
/* The s/S/c/C packets do not return status. So if the target does
not support the S or C packets, the debug agent returns an empty
string which is detected in remote_wait(). This protocol defect
@@ -2651,7 +2708,7 @@ remote_async_resume (ptid_t ptid, int st
getpkt (buf, (rs->remote_packet_size), 0);
if (packet_ok (buf, &remote_protocol_E) == PACKET_OK)
- goto register_event_loop;
+ return;
}
}
else
@@ -2669,7 +2726,7 @@ remote_async_resume (ptid_t ptid, int st
getpkt (buf, (rs->remote_packet_size), 0);
if (packet_ok (buf, &remote_protocol_e) == PACKET_OK)
- goto register_event_loop;
+ return;
}
}
}
@@ -2678,15 +2735,21 @@ remote_async_resume (ptid_t ptid, int st
{
buf[0] = step ? 'S' : 'C';
buf[1] = tohex (((int) siggnal >> 4) & 0xf);
- buf[2] = tohex ((int) siggnal & 0xf);
+ buf[2] = tohex (((int) siggnal) & 0xf);
buf[3] = '\0';
}
else
strcpy (buf, step ? "s" : "c");
-
+
putpkt (buf);
+}
+
+/* Same as remote_resume, but with async support. */
+static void
+remote_async_resume (ptid_t ptid, int step, enum target_signal siggnal)
+{
+ remote_resume (ptid, step, siggnal);
-register_event_loop:
/* We are about to start executing the inferior, let's register it
with the event loop. NOTE: this is the one place where all the
execution commands end up. We could alternatively do this in each
@@ -5952,6 +6015,7 @@ show_remote_cmd (char *args, int from_tt
show_remote_protocol_E_packet_cmd (args, from_tty, NULL);
show_remote_protocol_P_packet_cmd (args, from_tty, NULL);
show_remote_protocol_qSymbol_packet_cmd (args, from_tty, NULL);
+ show_remote_protocol_vcont_packet_cmd (args, from_tty, NULL);
show_remote_protocol_binary_download_cmd (args, from_tty, NULL);
}
@@ -6124,6 +6188,13 @@ in a memory packet.\n",
add_info ("remote-process", remote_info_process,
"Query the remote system for process info.");
+
+ add_packet_config_cmd (&remote_protocol_vcont,
+ "vCont", "verbose-resume",
+ set_remote_protocol_vcont_packet_cmd,
+ show_remote_protocol_vcont_packet_cmd,
+ &remote_set_cmdlist, &remote_show_cmdlist,
+ 0);
add_packet_config_cmd (&remote_protocol_qSymbol,
"qSymbol", "symbol-lookup",
More information about the Gdb-patches
mailing list