From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24774 invoked by alias); 23 Feb 2015 17:18:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24669 invoked by uid 89); 23 Feb 2015 17:18:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 23 Feb 2015 17:18:28 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1NHIR9g027947 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 23 Feb 2015 12:18:27 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t1NHIPTi003758 for ; Mon, 23 Feb 2015 12:18:26 -0500 Message-ID: <54EB60E1.8050708@redhat.com> Date: Mon, 23 Feb 2015 17:18:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [pushed] remote.c: simplify parsing stop reasons in T stop replies References: <1424710040-17332-1-git-send-email-palves@redhat.com> In-Reply-To: <1424710040-17332-1-git-send-email-palves@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-02/txt/msg00672.txt.bz2 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 + + * remote.c (skip_to_semicolon): New function. + (remote_parse_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 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"),