From ca4f7f8bebc069b8beec58754dc721e9be0798c7 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 29 Apr 2014 14:01:27 +0100 Subject: 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 * remote.c (struct packet_config) : 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. --- gdb/ChangeLog | 14 ++++++++++++++ gdb/remote.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++------ 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 + + * remote.c (struct packet_config) : 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 * 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 -- cgit v1.1