This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch] fix misparsing in remote_parse_stop_reply
- From: Sandra Loosemore <sandra at codesourcery dot com>
- To: gdb-patches <gdb-patches at sourceware dot org>
- Date: Sun, 16 Aug 2015 22:09:23 -0600
- Subject: [patch] fix misparsing in remote_parse_stop_reply
- Authentication-results: sourceware.org; auth=none
While trying to do some testing on one of our ARM boards via a debug
probe and stub that speaks RSP, I was mystified by getting this error:
(gdb) target remote ...
(gdb) break main
(gdb) load
(gdb) c
Cannot detach breakpoints of inferior_ptid
Eventually I realized that this wasn't just a badly-worded message; GDB
was somehow mis-interpreting the stop reply 'T05f:f0021000;' as
TARGET_WAITKIND_FORKED, and wandering off into code that it shouldn't
have been executing in the first place. (On ARM, register 0xf is the
PC, BTW, and this particular stub works fine with a GDB from last year.)
Tracking it down further to remote_parse_stop_reply, the trouble is that
the test
else if (strncmp (p, "fork", p1 - p) == 0)
is matching here; p1 points to the colon so it is matching exactly one
character, the initial 'f'. While "fork" is new-ish, all the stop
reason keywords use similar matching logic.
I don't see anything in the RSP documentation to indicate that the
special stop reason keywords in a 'T' reply can be abbreviated, so I
suspect this is just a think-o in the code, and it should be testing for
an exact match of the keyword instead. Is there something else going on
here that I'm missing?
The attached patch is sufficient to unblock testing. Does it seem
plausible? This particular debug probe is quite slow so I haven't had
time to run the full GDB testsuite yet, or test on any other target.
It would also be good if somebody could translate that puzzling error
message into something more user-friendly and less
implementor-speaky.... but I still don't know what it means, so I can't
suggest anything. :-(
-Sandra
2015-08-16 Sandra Loosemore <sandra@codesourcery.com>
gdb/
* remote.c (strprefix): New.
(remote_parse_stop_reply): Use strprefix instead of strncmp
to ensure exact match of keyword.
diff --git a/gdb/remote.c b/gdb/remote.c
index ca1f0df..4e483fd 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5835,6 +5835,18 @@ skip_to_semicolon (char *p)
return p;
}
+/* Helper for remote_parse_stop_reply. Return nonzero if the substring
+ starting with P and ending with PEND matches PREFIX. */
+
+static int
+strprefix (const char *p, const char *pend, const char *prefix)
+{
+ for ( ; p < pend; p++, prefix++)
+ if (*p != *prefix)
+ return 0;
+ return *prefix == '\0';
+}
+
/* Parse the stop reply in BUF. Either the function succeeds, and the
result is stored in EVENT, or throws an error. */
@@ -5886,17 +5898,17 @@ Packet: '%s'\n"),
the server only sends such a packet if it knows the
client understands it. */
- if (strncmp (p, "thread", p1 - p) == 0)
+ if (strprefix (p, p1, "thread"))
event->ptid = read_ptid (++p1, &p);
- else if ((strncmp (p, "watch", p1 - p) == 0)
- || (strncmp (p, "rwatch", p1 - p) == 0)
- || (strncmp (p, "awatch", p1 - p) == 0))
+ else if (strprefix (p, p1, "watch")
+ || strprefix (p, p1, "rwatch")
+ || strprefix (p, p1, "awatch"))
{
event->stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
p = unpack_varlen_hex (++p1, &addr);
event->watch_data_address = (CORE_ADDR) addr;
}
- else if (strncmp (p, "swbreak", p1 - p) == 0)
+ else if (strprefix (p, p1, "swbreak"))
{
event->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
@@ -5910,7 +5922,7 @@ Packet: '%s'\n"),
use of it in a backward compatible way. */
p = skip_to_semicolon (p1 + 1);
}
- else if (strncmp (p, "hwbreak", p1 - p) == 0)
+ else if (strprefix (p, p1, "hwbreak"))
{
event->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
@@ -5922,36 +5934,36 @@ Packet: '%s'\n"),
/* See above. */
p = skip_to_semicolon (p1 + 1);
}
- else if (strncmp (p, "library", p1 - p) == 0)
+ else if (strprefix (p, p1, "library"))
{
event->ws.kind = TARGET_WAITKIND_LOADED;
p = skip_to_semicolon (p1 + 1);
}
- else if (strncmp (p, "replaylog", p1 - p) == 0)
+ else if (strprefix (p, p1, "replaylog"))
{
event->ws.kind = TARGET_WAITKIND_NO_HISTORY;
/* p1 will indicate "begin" or "end", but it makes
no difference for now, so ignore it. */
p = skip_to_semicolon (p1 + 1);
}
- else if (strncmp (p, "core", p1 - p) == 0)
+ else if (strprefix (p, p1, "core"))
{
ULONGEST c;
p = unpack_varlen_hex (++p1, &c);
event->core = c;
}
- else if (strncmp (p, "fork", p1 - p) == 0)
+ else if (strprefix (p, p1, "fork"))
{
event->ws.value.related_pid = read_ptid (++p1, &p);
event->ws.kind = TARGET_WAITKIND_FORKED;
}
- else if (strncmp (p, "vfork", p1 - p) == 0)
+ else if (strprefix (p, p1, "vfork"))
{
event->ws.value.related_pid = read_ptid (++p1, &p);
event->ws.kind = TARGET_WAITKIND_VFORKED;
}
- else if (strncmp (p, "vforkdone", p1 - p) == 0)
+ else if (strprefix (p, p1, "vforkdone"))
{
event->ws.kind = TARGET_WAITKIND_VFORK_DONE;
p = skip_to_semicolon (p1 + 1);