diff mbox

[patchv4] Workaround gdbserver<7.7 for setfs

Message ID 20160406134911.GA30545@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil April 6, 2016, 1:49 p.m. UTC 
  On Tue, 05 Apr 2016 18:29:51 +0200, Pedro Alves wrote:
> MustReplyEmpty is actually not a useful probe packet, because
> that's actually an M (write memory) packet, which is then
> misinterpreted and fails the write.  So not an unknown packet.

Oops, OK.


> Try qMustReplyEmpty or something like that instead.

With these requirements on this workaround I have followed the gdbserver-7.6
sources and the bug was in function handle_notif_ack which is called only from
handle_v_requests, therefore for any packets /^v/.

OK to check in this one?


Thanks,
Jan
gdb/gdbserver/ChangeLog
2016-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* remote.c (unknown_v_replies_ok): New variable.
	(packet_config_support): Read it.
	(remote_start_remote): Set it.

Comments

Pedro Alves April 6, 2016, 2:31 p.m. UTC | #1 
On 04/06/2016 02:49 PM, Jan Kratochvil wrote:

>> > Try qMustReplyEmpty or something like that instead.
> With these requirements on this workaround I have followed the gdbserver-7.6
> sources and the bug was in function handle_notif_ack which is called only from
> handle_v_requests, therefore for any packets /^v/.

Ah.

> OK to check in this one?

OK with a change.

> diff --git a/gdb/remote.c b/gdb/remote.c
> index 5c407b6..a88f4cd 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1496,6 +1496,15 @@ enum {
>  
>  static struct packet_config remote_protocol_packets[PACKET_MAX];
>  
> +/* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
> +   unknown 'v' packet with string "OK".  "OK" gets interpreted by GDB
> +   as a reply to known packet.  For packet "vFile:setfs:" it is an
> +   invalid reply and GDB would return error in
> +   remote_hostio_set_filesystem, making remote files access impossible.
> +   If this variable is non-zero it means the remote gdbserver is buggy
> +   and any not yet detected packets are assumed as unsupported.  */
> +static int unknown_v_replies_ok;

This comment looks great now, thanks.  It helps the reader understand
exactly what is being worked around, and it'll help us decide what to
do in the future if this ever gets in the way.

Please make this new variable a field of 'struct remote_state' instead
of a global.

OK with that change.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 5c407b6..a88f4cd 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1496,6 +1496,15 @@  enum {
 
 static struct packet_config remote_protocol_packets[PACKET_MAX];
 
+/* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
+   unknown 'v' packet with string "OK".  "OK" gets interpreted by GDB
+   as a reply to known packet.  For packet "vFile:setfs:" it is an
+   invalid reply and GDB would return error in
+   remote_hostio_set_filesystem, making remote files access impossible.
+   If this variable is non-zero it means the remote gdbserver is buggy
+   and any not yet detected packets are assumed as unsupported.  */
+static int unknown_v_replies_ok;
+
 /* Returns the packet's corresponding "set remote foo-packet" command
    state.  See struct packet_config for more details.  */
 
@@ -1519,6 +1528,9 @@  packet_config_support (struct packet_config *config)
     case AUTO_BOOLEAN_FALSE:
       return PACKET_DISABLE;
     case AUTO_BOOLEAN_AUTO:
+      if (unknown_v_replies_ok && config->name != NULL
+	  && config->name[0] == 'v')
+	return PACKET_DISABLE;
       return config->support;
     default:
       gdb_assert_not_reached (_("bad switch"));
@@ -4023,6 +4035,21 @@  remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
   if (packet_support (PACKET_QAllow) != PACKET_DISABLE)
     remote_set_permissions (target);
 
+  /* See unknown_v_replies_ok description.  */
+  {
+    const char v_mustreplyempty[] = "vMustReplyEmpty";
+
+    putpkt (v_mustreplyempty);
+    getpkt (&rs->buf, &rs->buf_size, 0);
+    if (strcmp (rs->buf, "OK") == 0)
+      unknown_v_replies_ok = 1;
+    else if (strcmp (rs->buf, "") == 0)
+      unknown_v_replies_ok = 0;
+    else
+      error (_("Remote replied unexpectedly to '%s': %s"), v_mustreplyempty,
+	     rs->buf);
+  }
+
   /* Next, we possibly activate noack mode.
 
      If the QStartNoAckMode packet configuration is set to AUTO,