From patchwork Mon Aug 17 04:09:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sandra Loosemore X-Patchwork-Id: 8236 Received: (qmail 127468 invoked by alias); 17 Aug 2015 04:11:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 127458 invoked by uid 89); 17 Aug 2015 04:11:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Aug 2015 04:11:33 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1ZRBuP-0006Y0-4D from Sandra_Loosemore@mentor.com for gdb-patches@sourceware.org; Sun, 16 Aug 2015 21:20:29 -0700 Received: from [IPv6:::1] (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.3.224.2; Sun, 16 Aug 2015 21:11:28 -0700 Message-ID: <55D15E73.7040306@codesourcery.com> Date: Sun, 16 Aug 2015 22:09:23 -0600 From: Sandra Loosemore User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: gdb-patches Subject: [patch] fix misparsing in remote_parse_stop_reply 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 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);