diff mbox

Fix remote 'g' command error handling

Message ID 20180426205304.610-1-andrzej.kaczmarek@codecoup.pl
State New, archived
Headers

Commit Message

Andrzej Kaczmarek April 26, 2018, 8:53 p.m. UTC 
  'g' command returns hex-string as response so simply checking for 'E'
to determine if it failed is not enough and can trigger spurious error
messages. For example, invalid behaviour can be easily triggered on
Cortex-M as follows:

  (gdb) set $r0 = 0xe0
  Sending packet: $P0=e0000000#72...Packet received: OK
  Packet P (set-register) is supported
  Sending packet: $g#67...Packet received: E0000000849A0020...
  Remote failure reply: E0000000849A0020...

This patch fixes the problem by calling putpkt()/getpkt() directly and
checking result with packet_check_result(). This works fine since Enn
response has odd number of bytes while proper response has even number
of bytes.

Also, remote_send() is now not used anywhere so it can be removed.

gdb/Changelog:

	* remote.c (send_g_packet): Use putpkt/getpkt to send command
	(remote_send) Unused, removed.
---
 gdb/remote.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

Comments

Pedro Alves April 26, 2018, 10:55 p.m. UTC | #1 
On 04/26/2018 09:53 PM, Andrzej Kaczmarek wrote:
> 'g' command returns hex-string as response so simply checking for 'E'
> to determine if it failed is not enough and can trigger spurious error
> messages. For example, invalid behaviour can be easily triggered on
> Cortex-M as follows:
> 
>   (gdb) set $r0 = 0xe0
>   Sending packet: $P0=e0000000#72...Packet received: OK
>   Packet P (set-register) is supported
>   Sending packet: $g#67...Packet received: E0000000849A0020...
>   Remote failure reply: E0000000849A0020...
> 
> This patch fixes the problem by calling putpkt()/getpkt() directly and
> checking result with packet_check_result(). This works fine since Enn
> response has odd number of bytes while proper response has even number
> of bytes.
> 
> Also, remote_send() is now not used anywhere so it can be removed.

Thanks, I've merged it with minor tweaks to the ChangeLog entry.

This is an old one -- see PR 9665 at
<https://sourceware.org/bugzilla/show_bug.cgi?id=9665>.

As mentioned in the PR, the workaround is simple -- make the stub
send back lowercase hex digits instead of uppercase.  It seems
possible that others packets have the same problem, even, so
I'd suggest doing that in any case.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 193037b6e7..5920b82dd7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -122,8 +122,6 @@  static void remote_mourn (struct target_ops *ops);
 
 static void extended_remote_restart (void);
 
-static void remote_send (char **buf, long *sizeof_buf_p);
-
 static int readchar (int timeout);
 
 static void remote_serial_write (const char *str, int len);
@@ -7524,7 +7522,11 @@  send_g_packet (void)
   int buf_len;
 
   xsnprintf (rs->buf, get_remote_packet_size (), "g");
-  remote_send (&rs->buf, &rs->buf_size);
+  putpkt (rs->buf);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (packet_check_result (rs->buf) == PACKET_ERROR)
+    error (_("Could not read registers; remote failure reply '%s'"),
+           rs->buf);
 
   /* We can get out of synch in various cases.  If the first character
      in the buffer is not a hex character, assume that has happened
@@ -8600,22 +8602,6 @@  remote_serial_write (const char *str, int len)
     set_quit_flag ();
 }
 
-/* Send the command in *BUF to the remote machine, and read the reply
-   into *BUF.  Report an error if we get an error reply.  Resize
-   *BUF using xrealloc if necessary to hold the result, and update
-   *SIZEOF_BUF.  */
-
-static void
-remote_send (char **buf,
-	     long *sizeof_buf)
-{
-  putpkt (*buf);
-  getpkt (buf, sizeof_buf, 0);
-
-  if ((*buf)[0] == 'E')
-    error (_("Remote failure reply: %s"), *buf);
-}
-
 /* Return a string representing an escaped version of BUF, of len N.
    E.g. \n is converted to \\n, \t to \\t, etc.  */