This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [pushed] remote.c: simplify parsing stop reasons in T stop replies
- From: Pedro Alves <palves at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Mon, 23 Feb 2015 17:18:25 +0000
- Subject: Re: [pushed] remote.c: simplify parsing stop reasons in T stop replies
- Authentication-results: sourceware.org; auth=none
- References: <1424710040-17332-1-git-send-email-palves at redhat dot com>
On 02/23/2015 04:47 PM, Pedro Alves wrote:
> We need to be careful with parsing optional stop reasons that start
> with an hex character ("awatch", "core"), as GDBs that aren't aware of
> them parse them as real numbers. That's silly of course, given that
> there should be a colon after those magic "numbers". So if strtol on
> "abbz:" doesn't return "first invalid char" pointing to the colon, we
> know that "abbz" isn't really a register number. It must be optional
> stop info we don't know about. This adjusts GDB to work that way,
> removing the need for the special casing done upfront:
>
> /* If this packet is an awatch packet, don't parse the 'a'
> as a register number. */
> if (strncmp (p, "awatch", strlen("awatch")) != 0
> && strncmp (p, "core", strlen ("core") != 0))
>
> For as long as we care about compatibility with GDB 7.9, we'll need to
> continue to be careful about this, so I added a comment.
>
> Tested on x86_64 Fedora 20, native gdbserver.
For the record, here's the -w version:
diff --git c/gdb/ChangeLog w/gdb/ChangeLog
index faccd5f..b8bdf58 100644
--- c/gdb/ChangeLog
+++ w/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2015-02-23 Pedro Alves <palves@redhat.com>
+
+ * remote.c (skip_to_semicolon): New function.
+ (remote_parse_stop_reply) <T stop reply>: Use it. Don't
+ special case the stop reasons that look like hex numbers
+ upfront. Instead handle real register numbers after matching
+ all the known stop reasons.
+
2015-02-21 Doug Evans <dje@google.com>
PR c++/17976, symtab/17821
diff --git c/gdb/remote.c w/gdb/remote.c
index dbfc10b..3479140 100644
--- c/gdb/remote.c
+++ w/gdb/remote.c
@@ -5489,6 +5489,16 @@ peek_stop_reply (ptid_t ptid)
stop_reply_match_ptid_and_ws, &ptid);
}
+/* Skip PACKET until the next semi-colon (or end of string). */
+
+static char *
+skip_to_semicolon (char *p)
+{
+ while (*p != '\0' && *p != ';')
+ p++;
+ return p;
+}
+
/* Parse the stop reply in BUF. Either the function succeeds, and the
result is stored in EVENT, or throws an error. */
@@ -5521,34 +5531,25 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event)
while (*p)
{
char *p1;
- char *p_temp;
int fieldsize;
- LONGEST pnum = 0;
-
- /* If the packet contains a register number, save it in
- pnum and set p1 to point to the character following it.
- Otherwise p1 points to p. */
- /* If this packet is an awatch packet, don't parse the 'a'
- as a register number. */
-
- if (strncmp (p, "awatch", strlen("awatch")) != 0
- && strncmp (p, "core", strlen ("core") != 0))
- {
- /* Read the ``P'' register number. */
- pnum = strtol (p, &p_temp, 16);
- p1 = p_temp;
- }
- else
- p1 = p;
-
- if (p1 == p) /* No register number present here. */
- {
p1 = strchr (p, ':');
if (p1 == NULL)
error (_("Malformed packet(a) (missing colon): %s\n\
Packet: '%s'\n"),
p, buf);
+ if (p == p1)
+ error (_("Malformed packet(a) (missing register number): %s\n\
+Packet: '%s'\n"),
+ p, buf);
+
+ /* Some "registers" are actually extended stop information.
+ Note if you're adding a new entry here: GDB 7.9 and
+ earlier assume that all register "numbers" that start
+ with an hex digit are real register numbers. Make sure
+ the server only sends such a packet if it knows the
+ client understands it. */
+
if (strncmp (p, "thread", p1 - p) == 0)
event->ptid = read_ptid (++p1, &p);
else if ((strncmp (p, "watch", p1 - p) == 0)
@@ -5561,22 +5562,15 @@ Packet: '%s'\n"),
}
else if (strncmp (p, "library", p1 - p) == 0)
{
- p1++;
- p_temp = p1;
- while (*p_temp && *p_temp != ';')
- p_temp++;
-
event->ws.kind = TARGET_WAITKIND_LOADED;
- p = p_temp;
+ p = skip_to_semicolon (p1 + 1);
}
else if (strncmp (p, "replaylog", p1 - p) == 0)
{
event->ws.kind = TARGET_WAITKIND_NO_HISTORY;
/* p1 will indicate "begin" or "end", but it makes
no difference for now, so ignore it. */
- p_temp = strchr (p1 + 1, ';');
- if (p_temp)
- p = p_temp;
+ p = skip_to_semicolon (p1 + 1);
}
else if (strncmp (p, "core", p1 - p) == 0)
{
@@ -5587,25 +5581,19 @@ Packet: '%s'\n"),
}
else
{
- /* Silently skip unknown optional info. */
- p_temp = strchr (p1 + 1, ';');
- if (p_temp)
- p = p_temp;
- }
- }
- else
+ ULONGEST pnum;
+ char *p_temp;
+
+ /* Maybe a real ``P'' register number. */
+ p_temp = unpack_varlen_hex (p, &pnum);
+ /* If the first invalid character is the colon, we got a
+ register number. Otherwise, it's an unknown stop
+ reason. */
+ if (p_temp == p1)
{
struct packet_reg *reg = packet_reg_from_pnum (rsa, pnum);
cached_reg_t cached_reg;
- p = p1;
-
- if (*p != ':')
- error (_("Malformed packet(b) (missing colon): %s\n\
-Packet: '%s'\n"),
- p, buf);
- ++p;
-
if (reg == NULL)
error (_("Remote sent bad register number %s: %s\n\
Packet: '%s'\n"),
@@ -5613,6 +5601,7 @@ Packet: '%s'\n"),
cached_reg.num = reg->regnum;
+ p = p1 + 1;
fieldsize = hex2bin (p, cached_reg.data,
register_size (target_gdbarch (),
reg->regnum));
@@ -5623,6 +5612,13 @@ Packet: '%s'\n"),
VEC_safe_push (cached_reg_t, event->regcache, &cached_reg);
}
+ else
+ {
+ /* Not a number. Silently skip unknown optional
+ info. */
+ p = skip_to_semicolon (p1 + 1);
+ }
+ }
if (*p != ';')
error (_("Remote register badly formatted: %s\nhere: %s"),