aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPedro Alves <palves@redhat.com>2014-04-29 14:01:27 +0100
committerPedro Alves <palves@redhat.com>2014-04-29 14:01:27 +0100
commitca4f7f8bebc069b8beec58754dc721e9be0798c7 (patch)
treeeb0f68755cd593dc62a139bd42b38be2d4521651
parent11903c4c65d3bf0391362ac389b96ffdd9f58212 (diff)
downloadgdb-ca4f7f8bebc069b8beec58754dc721e9be0798c7.zip
gdb-ca4f7f8bebc069b8beec58754dc721e9be0798c7.tar.gz
gdb-ca4f7f8bebc069b8beec58754dc721e9be0798c7.tar.bz2
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.
-rw-r--r--gdb/ChangeLog14
-rw-r--r--gdb/remote.c60
2 files changed, 68 insertions, 6 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56f3c17..f11f4c9 100644
--- 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
--- 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