This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] remote.c: simplify parsing stop reasons in T stop replies


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1f10ba14bc8c53120432ccc7b2ba5605b4cee80e

commit 1f10ba14bc8c53120432ccc7b2ba5605b4cee80e
Author: Pedro Alves <palves@redhat.com>
Date:   Mon Feb 23 16:45:39 2015 +0000

    remote.c: simplify parsing stop reasons in T stop replies
    
    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.
    
    gdb/ChangeLog:
    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.

Diff:
---
 gdb/ChangeLog |   8 +++
 gdb/remote.c  | 166 ++++++++++++++++++++++++++++------------------------------
 2 files changed, 89 insertions(+), 85 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index faccd5f..b8bdf58 100644
--- a/gdb/ChangeLog
+++ b/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 a/gdb/remote.c b/gdb/remote.c
index dbfc10b..3479140 100644
--- a/gdb/remote.c
+++ b/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,107 +5531,93 @@ 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.  */
+	  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);
 
-	  /* If this packet is an awatch packet, don't parse the 'a'
-	     as a register number.  */
+	  /* 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, "awatch", strlen("awatch")) != 0
-	      && strncmp (p, "core", strlen ("core") != 0))
+	  if (strncmp (p, "thread", p1 - p) == 0)
+	    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))
 	    {
-	      /* Read the ``P'' register number.  */
-	      pnum = strtol (p, &p_temp, 16);
-	      p1 = p_temp;
+	      event->stopped_by_watchpoint_p = 1;
+	      p = unpack_varlen_hex (++p1, &addr);
+	      event->watch_data_address = (CORE_ADDR) addr;
 	    }
-	  else
-	    p1 = p;
-
-	  if (p1 == p)	/* No register number present here.  */
+	  else if (strncmp (p, "library", p1 - p) == 0)
 	    {
-	      p1 = strchr (p, ':');
-	      if (p1 == NULL)
-		error (_("Malformed packet(a) (missing colon): %s\n\
-Packet: '%s'\n"),
-		       p, buf);
-	      if (strncmp (p, "thread", p1 - p) == 0)
-		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))
-		{
-		  event->stopped_by_watchpoint_p = 1;
-		  p = unpack_varlen_hex (++p1, &addr);
-		  event->watch_data_address = (CORE_ADDR) addr;
-		}
-	      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;
-		}
-	      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;
-		}
-	      else if (strncmp (p, "core", p1 - p) == 0)
-		{
-		  ULONGEST c;
+	      event->ws.kind = TARGET_WAITKIND_LOADED;
+	      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 = skip_to_semicolon (p1 + 1);
+	    }
+	  else if (strncmp (p, "core", p1 - p) == 0)
+	    {
+	      ULONGEST c;
 
-		  p = unpack_varlen_hex (++p1, &c);
-		  event->core = c;
-		}
-	      else
-		{
-		  /* Silently skip unknown optional info.  */
-		  p_temp = strchr (p1 + 1, ';');
-		  if (p_temp)
-		    p = p_temp;
-		}
+	      p = unpack_varlen_hex (++p1, &c);
+	      event->core = c;
 	    }
 	  else
 	    {
-	      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;
+	      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;
 
-	      if (reg == NULL)
-		error (_("Remote sent bad register number %s: %s\n\
+		  if (reg == NULL)
+		    error (_("Remote sent bad register number %s: %s\n\
 Packet: '%s'\n"),
-		       hex_string (pnum), p, buf);
+			   hex_string (pnum), p, buf);
 
-	      cached_reg.num = reg->regnum;
+		  cached_reg.num = reg->regnum;
 
-	      fieldsize = hex2bin (p, cached_reg.data,
-				   register_size (target_gdbarch (),
-						  reg->regnum));
-	      p += 2 * fieldsize;
-	      if (fieldsize < register_size (target_gdbarch (),
-					     reg->regnum))
-		warning (_("Remote reply is too short: %s"), buf);
+		  p = p1 + 1;
+		  fieldsize = hex2bin (p, cached_reg.data,
+				       register_size (target_gdbarch (),
+						      reg->regnum));
+		  p += 2 * fieldsize;
+		  if (fieldsize < register_size (target_gdbarch (),
+						 reg->regnum))
+		    warning (_("Remote reply is too short: %s"), buf);
 
-	      VEC_safe_push (cached_reg_t, event->regcache, &cached_reg);
+		  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 != ';')


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]