RFC: Add named errors and messages to remote protocol

[Jim Blandy <jimb at codesourcery dot com>]

Here's a patch we've just started using internally at CodeSourcery
which implements something that's been discussed on and off for years:
extending the remote protocol to support named errors, and to provide
error messages along with errors.

This is a pretty rough patch.  Probably all the sites in remote.c that
check for errors should call error_from_remote.  But hopefully the
basic idea is clear enough to critique.

gdb/ChangeLog:
2007-06-13  Jim Blandy  <jimb@codesourcery.com>

	* remote.c (remote_escape_output, remote_unescape_input): Move
	earlier in the file.
	(error_from_remote): New function.
	(remote_wait, remote_async_wait): Report error responses using
	error_from_remote, instead of just reporting a warning.

gdb/doc/ChangeLog:
2007-06-13  Jim Blandy  <jimb@codesourcery.com>

	* gdb.texinfo (Overview): Document conventions for describing
	packet syntax here.
	(Standard Replies): New section.
	(Packets, General Query Packets, Tracepoint Packets): Remove
	explicit mention of empty and error replies, except when they
	provide detail not covered in Standard Replies.

diff -r 05eed8d199cc gdb/doc/gdb.texinfo
--- a/gdb/doc/gdb.texinfo	Fri Jun 15 13:04:58 2007 -0700
+++ b/gdb/doc/gdb.texinfo	Mon Jun 18 16:42:10 2007 -0700
@@ -22614,18 +22614,73 @@ So:
 @noindent
 means the same as "0000".
 
-The error response returned for some packets includes a two character
-error number.  That number is not well defined.
-
+In describing packets (and responses), each description has a template
+showing the overall syntax, followed by an explanation of the packet's
+meaning.  We include spaces in some of the templates for clarity;
+these are not part of the packet's syntax.  No @value{GDBN} packet
+uses spaces to separate its components.  For example, a template like
+@samp{foo @var{bar} @var{baz}} describes a packet beginning with the
+three ASCII bytes @samp{foo}, followed by a @var{bar}, followed
+directly by a @var{baz}.  @value{GDBN} does not transmit a space
+character between the @samp{foo} and the @var{bar}, or between the
+@var{bar} and the @var{baz}.
+
+We place optional portions of a packet in @r{[}square brackets@r{]};
+for example, a template like @samp{c @r{[}@var{addr}{]}} describes a
+packet beginning with the single ASCII character @samp{c}, possibly
+followed by an @var{addr}.
+
+@node Standard Replies
+@section Standard Replies
+
+The remote protocol specifies a few standard replies.  All commands
+support these, except as noted in the individual command descriptions.
+
+@table @samp
+
+@item
 @cindex empty response, for unsupported packets
+@cindex unsupported packets, empty response for
 For any @var{command} not supported by the stub, an empty response
 (@samp{$#00}) should be returned.  That way it is possible to extend the
 protocol.  A newer @value{GDBN} can tell if a packet is supported based
-on that response.
-
-A stub is required to support the @samp{g}, @samp{G}, @samp{m}, @samp{M},
-@samp{c}, and @samp{s} @var{command}s.  All other @var{command}s are
-optional.
+on that response (but see also @ref{qSupported}).
+
+@item E @var{xx}
+An error has occurred; @var{xx} is a two-digit hexadecimal error
+number.  In almost all cases, the protocol does not specify the
+meaning of the error numbers; GDB usually ignores the numbers, or
+displays them to the user without further interpretation.
+
+@item E.@var{name}@r{[}.@var{message}@r{]}
+An error has occurred; @var{name} is the name of the error.  The name
+may contain letters, numbers, and @samp{-} characters.  If present,
+@var{message} is an error message, encoded using the escaped eight-bit
+conventions for binary data described above.
+
+Except as noted, named errors may only be returned to commands
+documented to expect them; this ensures that older stubs can interact
+with newer versions of @value{GDBN}, even when interpretations for
+named errors have been added to the protocol.
+
+The protocol uses the following error names:
+
+@table @samp
+
+@item fatal
+A fatal error has occurred; the stub will be unable to interact
+further with @value{GDBN}.  Fatal errors should always include a
+message explaining their cause.
+
+Any command may return this error.
+
+@item memtype
+The memory addressed is of the wrong type for the given command.  For
+example, a @samp{vFlashWrite} command applied to non-flash memory
+elicits an @samp{E.memtype} error response.
+
+@end table
+@end table
 
 @node Packets
 @section Packets
@@ -22635,19 +22690,12 @@ The following table provides a complete 
 @xref{File-I/O Remote Protocol Extension}, for details about the File
 I/O extension of the remote protocol.
 
-Each packet's description has a template showing the packet's overall
-syntax, followed by an explanation of the packet's meaning.  We
-include spaces in some of the templates for clarity; these are not
-part of the packet's syntax.  No @value{GDBN} packet uses spaces to
-separate its components.  For example, a template like @samp{foo
-@var{bar} @var{baz}} describes a packet beginning with the three ASCII
-bytes @samp{foo}, followed by a @var{bar}, followed directly by a
-@var{baz}.  @value{GDBN} does not transmit a space character between the
-@samp{foo} and the @var{bar}, or between the @var{bar} and the
-@var{baz}.
-
 Note that all packet forms beginning with an upper- or lower-case
 letter, other than those described here, are reserved for future use.
+
+A stub is required to support the @samp{g}, @samp{G}, @samp{m}, @samp{M},
+@samp{c}, and @samp{s} @var{command}s.  All other @var{command}s are
+optional.
 
 Here are the packet descriptions.
 
@@ -22683,8 +22731,6 @@ Reply:
 @table @samp
 @item OK
 The arguments were set.
-@item E @var{NN}
-An error occurred.
 @end table
 
 @item b @var{baud}
@@ -22742,8 +22788,6 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
 @end table
 
 @item F @var{RC},@var{EE},@var{CF};@var{XX}
@@ -22766,8 +22810,6 @@ determined by the @value{GDBN} internal 
 determined by the @value{GDBN} internal macros
 @code{DEPRECATED_REGISTER_RAW_SIZE} and @code{REGISTER_NAME} macros.  The
 specification of several standard @samp{g} packets is specified below.
-@item E @var{NN}
-for an error.
 @end table
 
 @item G @var{XX@dots{}}
@@ -22779,8 +22821,6 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
 @end table
 
 @item H @var{c} @var{t}
@@ -22795,8 +22835,6 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
 @end table
 
 @c FIXME: JTC:
@@ -22854,23 +22892,19 @@ Memory contents; each byte is transmitte
 Memory contents; each byte is transmitted as a two-digit hexadecimal
 number.  The reply may contain fewer bytes than requested if the
 server was able to read only part of the region of memory.
-@item E @var{NN}
-@var{NN} is errno
 @end table
 
 @item M @var{addr},@var{length}:@var{XX@dots{}}
 @cindex @samp{M} packet
 Write @var{length} bytes of memory starting at address @var{addr}.
 @var{XX@dots{}} is the data; each byte is transmitted as a two-digit
-hexadecimal number.
+hexadecimal number.  
 
 Reply:
 @table @samp
 @item OK
-for success
-@item E @var{NN}
-for an error (this includes the case where only part of the data was
-written).
+All the data was written successfully.  (If only part of the data was
+written, this command returns an error.)
 @end table
 
 @item p @var{n}
@@ -22883,10 +22917,6 @@ Reply:
 @table @samp
 @item @var{XX@dots{}}
 the register's value
-@item E @var{NN}
-for an error
-@item
-Indicating an unrecognized @var{query}.
 @end table
 
 @item P @var{n@dots{}}=@var{r@dots{}}
@@ -22900,8 +22930,6 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
 @end table
 
 @item q @var{name} @var{params}@dots{}
@@ -23017,8 +23045,8 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
+@item E.memtype
+for vFlashErase addressing non-flash memory
 @end table
 
 @item vFlashWrite:@var{addr}:@var{XX@dots{}}
@@ -23041,8 +23069,6 @@ for success
 for success
 @item E.memtype
 for vFlashWrite addressing non-flash memory
-@item E @var{NN}
-for an error
 @end table
 
 @item vFlashDone
@@ -23065,8 +23091,6 @@ Reply:
 @table @samp
 @item OK
 for success
-@item E @var{NN}
-for an error
 @end table
 
 @item z @var{type},@var{addr},@var{length}
@@ -23110,10 +23134,6 @@ Reply:
 @table @samp
 @item OK
 success
-@item
-not supported
-@item E @var{NN}
-for an error
 @end table
 
 @item z1,@var{addr},@var{length}
@@ -23133,10 +23153,6 @@ Reply:
 @table @samp
 @item OK
 success
-@item
-not supported
-@item E @var{NN}
-for an error
 @end table
 
 @item z2,@var{addr},@var{length}
@@ -23149,10 +23165,6 @@ Reply:
 @table @samp
 @item OK
 success
-@item
-not supported
-@item E @var{NN}
-for an error
 @end table
 
 @item z3,@var{addr},@var{length}
@@ -23165,10 +23177,6 @@ Reply:
 @table @samp
 @item OK
 success
-@item
-not supported
-@item E @var{NN}
-for an error
 @end table
 
 @item z4,@var{addr},@var{length}
@@ -23181,10 +23189,6 @@ Reply:
 @table @samp
 @item OK
 success
-@item
-not supported
-@item E @var{NN}
-for an error
 @end table
 
 @end table
@@ -23340,8 +23344,6 @@ Compute the CRC checksum of a block of m
 Compute the CRC checksum of a block of memory.
 Reply:
 @table @samp
-@item E @var{NN}
-An error (such as memory fault)
 @item C @var{crc32}
 The specified memory region's checksum is @var{crc32}.
 @end table
@@ -23401,12 +23403,6 @@ Reply:
 @item @var{XX}@dots{}
 Hex encoded (big endian) bytes representing the address of the thread
 local storage requested.
-
-@item E @var{nn}
-An error occurred.  @var{nn} are hex digits.
-
-@item
-An empty reply indicates that @samp{qGetTLSAddr} is not supported by the stub.
 @end table
 
 @item qL @var{startflag} @var{threadcount} @var{nextthread}
@@ -23472,13 +23468,6 @@ Reply:
 @table @samp
 @item OK
 The request succeeded.
-
-@item E @var{nn}
-An error occurred.  @var{nn} are hex digits.
-
-@item
-An empty reply indicates that @samp{QPassSignals} is not supported by
-the stub.
 @end table
 
 Use of this packet is controlled by the @code{set remote pass-signals}
@@ -23502,10 +23491,6 @@ A command response with no output.
 A command response with no output.
 @item @var{OUTPUT}
 A command response with the hex encoded output string @var{OUTPUT}.
-@item E @var{NN}
-Indicate a badly formed request.
-@item
-An empty reply indicates that @samp{qRcmd} is not recognized.
 @end table
 
 (Note that the @code{qRcmd} packet's name is separated from the
@@ -23539,9 +23524,6 @@ The stub supports or does not support ea
 The stub supports or does not support each returned @var{stubfeature},
 depending on the form of each @var{stubfeature} (see below for the
 possible forms).
-@item
-An empty reply indicates that @samp{qSupported} is not recognized,
-or that no features needed to be reported to @value{GDBN}.
 @end table
 
 The allowed forms for each feature (either a @var{gdbfeature} in the
@@ -23947,8 +23929,6 @@ Replies:
 @table @samp
 @item OK
 The packet was understood and carried out.
-@item 
-The packet was not recognized.
 @end table
 
 @item QTDP:-@var{n}:@var{addr}:@r{[}S@r{]}@var{action}@dots{}@r{[}-@r{]}
@@ -24011,8 +23991,6 @@ Replies:
 @table @samp
 @item OK
 The packet was understood and carried out.
-@item 
-The packet was not recognized.
 @end table
 
 @item QTFrame:@var{n}
diff -r 05eed8d199cc gdb/remote.c
--- a/gdb/remote.c	Fri Jun 15 13:04:58 2007 -0700
+++ b/gdb/remote.c	Mon Jun 18 17:05:41 2007 -0700
@@ -695,6 +695,95 @@ get_memory_read_packet_size (void)
   if (size > get_remote_packet_size ())
     size = get_remote_packet_size ();
   return size;
+}
+
+
+/* Escaping binary data.  */
+
+/* Convert BUFFER, binary data at least LEN bytes long, into escaped
+   binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
+   encoded in OUT_BUF, and return the number of bytes in OUT_BUF
+   (which may be more than *OUT_LEN due to escape characters).  The
+   total number of bytes in the output buffer will be at most
+   OUT_MAXLEN.  */
+
+static int
+remote_escape_output (const gdb_byte *buffer, int len,
+		      gdb_byte *out_buf, int *out_len,
+		      int out_maxlen)
+{
+  int input_index, output_index;
+
+  output_index = 0;
+  for (input_index = 0; input_index < len; input_index++)
+    {
+      gdb_byte b = buffer[input_index];
+
+      if (b == '$' || b == '#' || b == '}')
+	{
+	  /* These must be escaped.  */
+	  if (output_index + 2 > out_maxlen)
+	    break;
+	  out_buf[output_index++] = '}';
+	  out_buf[output_index++] = b ^ 0x20;
+	}
+      else
+	{
+	  if (output_index + 1 > out_maxlen)
+	    break;
+	  out_buf[output_index++] = b;
+	}
+    }
+
+  *out_len = input_index;
+  return output_index;
+}
+
+/* Convert BUFFER, escaped data LEN bytes long, into binary data
+   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
+   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
+   BUFFER and OUT_BUF may be the same buffer.
+
+   This function reverses remote_escape_output.  It allows more
+   escaped characters than that function does, in particular because
+   '*' must be escaped to avoid the run-length encoding processing
+   in reading packets.  */
+
+static int
+remote_unescape_input (const gdb_byte *buffer, int len,
+		       gdb_byte *out_buf, int out_maxlen)
+{
+  int input_index, output_index;
+  int escaped;
+
+  output_index = 0;
+  escaped = 0;
+  for (input_index = 0; input_index < len; input_index++)
+    {
+      gdb_byte b = buffer[input_index];
+
+      if (output_index + 1 > out_maxlen)
+	{
+	  warning (_("Received too much data from remote target;"
+		     " ignoring overflow."));
+	  return output_index;
+	}
+
+      if (escaped)
+	{
+	  out_buf[output_index++] = b ^ 0x20;
+	  escaped = 0;
+	}
+      else if (b == '}')
+	escaped = 1;
+      else
+	out_buf[output_index++] = b;
+    }
+
+  if (escaped)
+    error (_("Unmatched escape character in target response."));
+
+  return output_index;
 }
 
 
@@ -893,6 +982,34 @@ packet_ok (const char *buf, struct packe
 
   return result;
 }
+
+/* Raise an error based on the the "E" response in PACKET.  */
+static NORETURN void
+error_from_remote (char *packet)
+{
+  gdb_assert (packet[0] == 'E');
+  if (packet[1] == '.')
+    {
+      /* If the error packet includes a message, print that.  */
+      char *message = strchr (packet + 2, '.');
+
+      if (message)
+        {
+          int len = strlen (++message);
+          len = remote_unescape_input (message, len, message, len);
+          message[len] = '\0';
+          error ("Error from remote target: %s", message);
+        }
+
+      else
+        /* Otherwise, just print the error name.  */
+        error ("Error from remote target: %s", packet + 2);
+    }
+  else
+    /* At worst, print the hex error number.  */
+    error ("Error '%s' from remote target", packet + 1);
+}
+
 
 enum {
   PACKET_vCont = 0,
@@ -3141,8 +3258,9 @@ remote_wait (ptid_t ptid, struct target_
       switch (buf[0])
 	{
 	case 'E':		/* Error of some sort.  */
-	  warning (_("Remote failure reply: %s"), buf);
-	  continue;
+	  error_from_remote (buf);
+	  /* Never returns.  */
+
 	case 'F':		/* File-I/O request.  */
 	  remote_fileio_request (buf);
 	  continue;
@@ -3338,8 +3456,9 @@ remote_async_wait (ptid_t ptid, struct t
       switch (buf[0])
 	{
 	case 'E':		/* Error of some sort.  */
-	  warning (_("Remote failure reply: %s"), buf);
-	  continue;
+          error_from_remote (buf);
+	  /* Never returns.  */
+
 	case 'F':		/* File-I/O request.  */
 	  remote_fileio_request (buf);
 	  continue;
@@ -3936,91 +4055,6 @@ remote_address_masked (CORE_ADDR addr)
       addr &= mask;
     }
   return addr;
-}
-
-/* Convert BUFFER, binary data at least LEN bytes long, into escaped
-   binary data in OUT_BUF.  Set *OUT_LEN to the length of the data
-   encoded in OUT_BUF, and return the number of bytes in OUT_BUF
-   (which may be more than *OUT_LEN due to escape characters).  The
-   total number of bytes in the output buffer will be at most
-   OUT_MAXLEN.  */
-
-static int
-remote_escape_output (const gdb_byte *buffer, int len,
-		      gdb_byte *out_buf, int *out_len,
-		      int out_maxlen)
-{
-  int input_index, output_index;
-
-  output_index = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (b == '$' || b == '#' || b == '}')
-	{
-	  /* These must be escaped.  */
-	  if (output_index + 2 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = '}';
-	  out_buf[output_index++] = b ^ 0x20;
-	}
-      else
-	{
-	  if (output_index + 1 > out_maxlen)
-	    break;
-	  out_buf[output_index++] = b;
-	}
-    }
-
-  *out_len = input_index;
-  return output_index;
-}
-
-/* Convert BUFFER, escaped data LEN bytes long, into binary data
-   in OUT_BUF.  Return the number of bytes written to OUT_BUF.
-   Raise an error if the total number of bytes exceeds OUT_MAXLEN.
-
-   This function reverses remote_escape_output.  It allows more
-   escaped characters than that function does, in particular because
-   '*' must be escaped to avoid the run-length encoding processing
-   in reading packets.  */
-
-static int
-remote_unescape_input (const gdb_byte *buffer, int len,
-		       gdb_byte *out_buf, int out_maxlen)
-{
-  int input_index, output_index;
-  int escaped;
-
-  output_index = 0;
-  escaped = 0;
-  for (input_index = 0; input_index < len; input_index++)
-    {
-      gdb_byte b = buffer[input_index];
-
-      if (output_index + 1 > out_maxlen)
-	{
-	  warning (_("Received too much data from remote target;"
-		     " ignoring overflow."));
-	  return output_index;
-	}
-
-      if (escaped)
-	{
-	  out_buf[output_index++] = b ^ 0x20;
-	  escaped = 0;
-	}
-      else if (b == '}')
-	escaped = 1;
-      else
-	out_buf[output_index++] = b;
-    }
-
-  if (escaped)
-    error (_("Unmatched escape character in target response."));
-
-  return output_index;
 }
 
 /* Determine whether the remote target supports binary downloading.