From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11777 invoked by alias); 28 Apr 2014 19:16:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11760 invoked by uid 89); 28 Apr 2014 19:16:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 28 Apr 2014 19:16:10 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id C31251160E5; Mon, 28 Apr 2014 15:16:08 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id YMNUUzbSzdLR; Mon, 28 Apr 2014 15:16:08 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 9C9301160DD; Mon, 28 Apr 2014 15:16:08 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 4FDBEE0487; Mon, 28 Apr 2014 15:16:08 -0400 (EDT) Date: Mon, 28 Apr 2014 19:16:00 -0000 From: Joel Brobecker To: Pedro Alves Cc: gdb-patches@sourceware.org, Yao Qi Subject: Re: [PATCH v2] Fix several "set remote foo-packet on/off" commands. Message-ID: <20140428191608.GA9089@adacore.com> References: <1396307414-2053-1-git-send-email-palves@redhat.com> <533A7E83.4070200@codesourcery.com> <533AABE1.8040101@redhat.com> <533AB01E.4060003@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="2fHTh5uZTiUOsy+g" Content-Disposition: inline In-Reply-To: <533AB01E.4060003@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-04/txt/msg00593.txt.bz2 --2fHTh5uZTiUOsy+g Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2976 Hi Pedro, > gdb/ > 2014-04-01 Pedro Alves > > * remote.c (struct remote_state): Remove multi_process_aware, [...] Unfortunately, your patch seems to be introducing 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: What happens, here, it seems, is that GDB thinks it's OK to use the QNonStop even though it wasn't specified in the qSupported reply. Looking at the code that sets the feature's status when not seen in the qSupported reply, we see: /* Handle the defaults for unmentioned features. */ for (i = 0; i < ARRAY_SIZE (remote_protocol_features); i++) if (!seen[i]) { const struct protocol_feature *feature; feature = &remote_protocol_features[i]; feature->func (feature, feature->default_support, NULL); } In the case of PACKET_QNonStop, we have the following definition... { "QNonStop", PACKET_DISABLE, remote_supported_packet, PACKET_QNonStop }, ... so feature->func is remote_supported_packet, which then sets the feature as: static void remote_supported_packet (const struct protocol_feature *feature, enum packet_support support, const char *argument) { [warning + return if argument not NULL] remote_protocol_packets[feature->packet].support = support; } However, looking at the packet_support function, it basically relies on packet_config_support, which itself only looks at the support field of our feature if "detect" was set to AUTO_BOOLEAN_AUTO: switch (config->detect) { case AUTO_BOOLEAN_TRUE: return PACKET_ENABLE; case AUTO_BOOLEAN_FALSE: return PACKET_DISABLE; case AUTO_BOOLEAN_AUTO: return config->support; default: gdb_assert_not_reached (_("bad switch")); } However, in the case of this option, it's set to AUTO_BOOLEAN_TRUE. So the config->support value that we set earlier has no effect. I worked around the issue by making this package a configurable packet (diff attached) but I have a feeling that there is either something I am not getting, or perhaps a hole in the current design. Or perhaps something else? I am not quite clear on what should be done, yet. Thanks! -- Joel --2fHTh5uZTiUOsy+g Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="remote.c.diff" Content-length: 554 diff --git a/gdb/remote.c b/gdb/remote.c index dcd3cdd..14dd8c9 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -11878,6 +11878,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (&remote_protocol_packets[PACKET_QPassSignals], "QPassSignals", "pass-signals", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_QNonStop], + "QNonStop", "non-stop", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_QProgramSignals], "QProgramSignals", "program-signals", 0); --2fHTh5uZTiUOsy+g--