Fix remote connection to targets that don't support the QNonStop packet.

... and others.  The recent patch that fixed several "set remote
foo-packet on/off" commands introduced a regression, observable when
connecting GDB to QEMU.  For instance:

        (gdb) set debug remote 1
        (gdb) tar rem :4444
        Remote debugging using :4444
        Sending packet: $qSupported:multiprocess+;qRelocInsn+#2a...Ack
        Packet received: PacketSize=1000;qXfer:features:read+
        Packet qSupported (supported-packets) is supported
        Sending packet: $Hgp0.0#ad...Ack
        Packet received: OK
        Sending packet: $qXfer:features:read:target.xml:0,ffb#79...Ack
        Packet received: [...]
        Sending packet: $qXfer:features:read:arm-core.xml:0,ffb#08...Ack
        Packet received: [...]
 !!! -> Sending packet: $QNonStop:0#8c...Ack
        Packet received:
        Remote refused setting all-stop mode with:

The "QNonStop" feature is associated with the PACKET_QNonStop packet,
with a default of PACKET_DISABLE, so GDB should not be sending the
packet at all.

The patch that introduced the regression decoupled packet_config's
'detect' and 'support' fields, making the former (an auto_boolean)
purely the associated "set remote foo-packet" command's variable.  In
the example above, the packet config's 'supported' field does end up
correctly set to PACKET_DISABLE.  However, nothing is presently
initializing packet configs that don't actually have a command
associated.  Those configs's 'detect' field then ends up set to
AUTO_BOOLEAN_TRUE, simply because that happens to be 0.  This forces
GDB to assume the packet is supported, irrespective of what the target
claims it supports, just like if the user had done "set remote
foo-packet on" (this being the associated command, if there was one).

Ideally, all packet configs would have a command associated.  While
that isn't true, make sure all packet configs are initialized, even if
no command is associated, and add an assertion that prevents adding
more packets/features without an associated command.

Tested on x86_64 Fedora 17, against pristine gdbserver, and against a
gdbserver with the QNonStop packet/feature disabled with a local hack.

gdb/
2014-04-29  Pedro Alves  <palves@redhat.com>

	* remote.c (struct packet_config) <detect>: Extend comment.
	(add_packet_config_cmd): Don't set the config's detect or support
	fields here.
	(init_all_packet_configs): Also initialize the config's 'detect'
	field.
	(reset_all_packet_configs_support): New function.
	(remote_open_1): Call reset_all_packet_configs_support instead of
	init_all_packet_configs.
	(_initialize_remote): Initialize all packet configs.  Assert that
	all packets have an associated command, except a few known
	outliers.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56f3c17..f11f4c9 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2014-04-29  Pedro Alves  <palves@redhat.com>
+
+       * remote.c (struct packet_config) <detect>: Extend comment.
+       (add_packet_config_cmd): Don't set the config's detect or support
+       fields here.
+       (init_all_packet_configs): Also initialize the config's 'detect'
+       field.
+       (reset_all_packet_configs_support): New function.
+       (remote_open_1): Call reset_all_packet_configs_support instead of
+       init_all_packet_configs.
+       (_initialize_remote): Initialize all packet configs.  Assert that
+       all packets have an associated command, except a few known
+       outliers.
+
 2014-04-28  Joel Brobecker  <brobecker@adacore.com>
 
        * dwarf2read.c (read_subrange_type): Handle dynamic

diff --git a/gdb/remote.c b/gdb/remote.c
index dcd3cdd..4177b39 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1067,7 +1067,8 @@ struct packet_config
     /* If auto, GDB auto-detects support for this packet or feature,
        either through qSupported, or by trying the packet and looking
        at the response.  If true, GDB assumes the target supports this
-       packet.  If false, the packet is disabled.  */
+       packet.  If false, the packet is disabled.  Configs that don't
+       have an associated command always have this set to auto.  */
     enum auto_boolean detect;
 
     /* Does the target support this packet?  */
@@ -1129,8 +1130,6 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
 
   config->name = name;
   config->title = title;
-  config->detect = AUTO_BOOLEAN_AUTO;
-  config->support = PACKET_SUPPORT_UNKNOWN;
   set_doc = xstrprintf ("Set use of remote protocol `%s' (%s) packet",
                        name, title);
   show_doc = xstrprintf ("Show current use of remote "
@@ -3632,10 +3631,11 @@ extended_remote_open (char *name, int from_tty)
   remote_open_1 (name, from_tty, &extended_remote_ops, 1 /*extended_p */);
 }
 
-/* Generic code for opening a connection to a remote target.  */
+/* Reset all packets back to "unknown support".  Called when opening a
+   new connection to a remote target.  */
 
 static void
-init_all_packet_configs (void)
+reset_all_packet_configs_support (void)
 {
   int i;
 
@@ -3643,6 +3643,20 @@ init_all_packet_configs (void)
     remote_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
 }
 
+/* Initialize all packet configs.  */
+
+static void
+init_all_packet_configs (void)
+{
+  int i;
+
+  for (i = 0; i < PACKET_MAX; i++)
+    {
+      remote_protocol_packets[i].detect = AUTO_BOOLEAN_AUTO;
+      remote_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
+    }
+}
+
 /* Symbol look-up.  */
 
 static void
@@ -4192,7 +4206,7 @@ remote_open_1 (char *name, int from_tty,
 
   /* Reset the target state; these things will be queried either by
      remote_query_supported or as they are needed.  */
-  init_all_packet_configs ();
+  reset_all_packet_configs_support ();
   rs->cached_wait_status = 0;
   rs->explicit_packet_size = 0;
   rs->noack_mode = 0;
@@ -11869,6 +11883,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
                             NULL, /* FIXME: i18n: */
                             &setlist, &showlist);
 
+  init_all_packet_configs ();
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_X],
                         "X", "binary-download", 1);
 
@@ -12052,6 +12068,38 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_btrace],
        "qXfer:btrace", "read-btrace", 0);
 
+  /* Assert that we've registered commands for all packet configs.  */
+  {
+    int i;
+
+    for (i = 0; i < PACKET_MAX; i++)
+      {
+       /* Ideally all configs would have a command associated.  Some
+          still don't though.  */
+       int excepted;
+
+       switch (i)
+         {
+         case PACKET_QNonStop:
+         case PACKET_multiprocess_feature:
+         case PACKET_EnableDisableTracepoints_feature:
+         case PACKET_tracenz_feature:
+         case PACKET_DisconnectedTracing_feature:
+         case PACKET_augmented_libraries_svr4_read_feature:
+           /* Additions to this list need to be well justified.  */
+           excepted = 1;
+           break;
+         default:
+           excepted = 0;
+           break;
+         }
+
+       /* This catches both forgetting to add a config command, and
+          forgetting to remove a packet from the exception list.  */
+       gdb_assert (excepted == (remote_protocol_packets[i].name == NULL));
+      }
+  }
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their