From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16636 invoked by alias); 21 Oct 2014 23:23:56 -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 16627 invoked by uid 89); 21 Oct 2014 23:23:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 21 Oct 2014 23:23:54 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1XgimM-00027m-6X from Don_Breazeal@mentor.com ; Tue, 21 Oct 2014 16:23:50 -0700 Received: from [127.0.0.1] ([172.30.15.1]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 21 Oct 2014 16:23:49 -0700 Message-ID: <5446EB04.7010208@codesourcery.com> Date: Tue, 21 Oct 2014 23:23:00 -0000 From: "Breazeal, Don" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.1.2 MIME-Version: 1.0 To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH 04/16 v2] Determine supported extended-remote features References: <1407434395-19089-1-git-send-email-donb@codesourcery.com> <1408580964-27916-5-git-send-email-donb@codesourcery.com> <543E9E09.80009@redhat.com> In-Reply-To: <543E9E09.80009@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00556.txt.bz2 On 10/15/2014 9:17 AM, Pedro Alves wrote: > On 08/21/2014 01:29 AM, Don Breazeal wrote: >> This patch implements a mechanism for GDB to ask gdbserver what >> extended-mode features are enabled. >> >> The problems that are solved include: >> >> 1) A mechanism other than the qSupported RSP packet is needed for >> gdbserver to inform GDB about the list of extended mode features that >> are supported. > > Sorry, this doesn't really make sense to me. > >> >> In the existing implementation most of the information about >> supported gdbserver features is sent to GDB in response to a >> qSupported RSP message. This exchange occurs prior to GDB >> sending the "!" packet to enable extended mode. So as-is this >> message doesn't work for features that are dependent on extended- >> mode. > > The differences between extended and non-extended modes are very > few, and I'd rather have fewer -- eventually merge them -- not > more. > > I don't understand what motivated this. What's wrong with > just reporting the features as supported in qSupported, and > then have GDB decide whether to make use the features or not? > >> Also, the qSupported exchange is intended to inform GDB about which >> packets are supported, and most of the extended mode features do not >> have associated packets. There are some features included in the >> qSupported response that do not have associated RSP messages, but the >> code comments make it clear that this practice is not acceptable for new >> packets. > > Which comments? > >> >> 2) A mechanism is needed to enable extended mode features when GDB >> tells gdbserver to use extended mode. >> >> The existing implementation checks for ptrace extended events >> (just PTRACE_O_TRACELONE) and enables them immediately after checking. >> This is done when the first event occurs as the program is loaded, which >> can occur before GDB has connected to gdbserver and told it whether or >> not to use extended mode. > > > Thanks, > Pedro Alves > Hi Pedro, Thanks for looking at this. I should have made the rationale for the new qExtendedFeatures packet more clear in my original submission. I'll try to answer all of your questions in the explanation below. I had started out trying to report the features as supported/unsupported using qSupported, following the implementation of PACKET_multiprocess_feature as a model. I abandoned that approach when I saw these comments in remote.c_initialize_remote: /* Ideally all configs would have a command associated. Some still don't though. */ [---snip---] case PACKET_multiprocess_feature: [---snip---] /* Additions to this list need to be well justified: pre-existing packets are OK; new packets are not. */ I interpreted this to mean that the qSupported query was intended to be used only to enable/disable packets that mapped to commands, and that defining new packets that represented something else was discouraged. In the end, the qExtendedFeatures approach seemed better to me because - it didn't violate the directive in the comment above - it was less disruptive to existing code, and didn't affect native - it could potentially be used to eliminate some special-case code that used packets that had no associated command, e.g. the multiprocess feature could be implemented using qExtendedFeatures instead of PACKET_multiprocess_feature. Implementing the qSupported approach required: 1) sending "!" before sending qSupported so that gdbserver knew it was in extended mode when responding to qSupported 2) splitting nat/linux-ptrace.c:linux_enable_event_reporting into two functions, one to check if extended events were supported at startup time, and one to enable any supported extended events where linux_enable_event_reporting is called today. This allowed gdbserver to know what extended events were supported when responding to qSupported. Note that this affects the native implementation. 3) defining a new packet for each of the extended-mode events In refreshing my memory about this I wrote up some detailed notes on it. Let me know if you want to see the gruesome details. Do you think the qSupported approach is preferable despite the comment? Did I misinterpret it? Thanks --Don