From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 77538 invoked by alias); 4 Mar 2020 13:17:09 -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 77530 invoked by uid 89); 4 Mar 2020 13:17:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=Luis, HX-Received:ad4, realistic X-HELO: mail-qv1-f65.google.com Received: from mail-qv1-f65.google.com (HELO mail-qv1-f65.google.com) (209.85.219.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Mar 2020 13:17:06 +0000 Received: by mail-qv1-f65.google.com with SMTP id o18so748810qvf.1 for ; Wed, 04 Mar 2020 05:17:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=1/FmUeB4bvfi4qSLZiYiwHZtcYeh9bDMuyIOUgerzO8=; b=sqVsf6RzZj12T49UmQkB78TDZfNkMhkU0D0gM5/GPdCXT7lQcC6eXhPB/yRZN1ROXr O1DZhOsmOlqNHFB4u0PGv/XpLTxBK3yBeYQsX4whitzLRnAEJxmmvEvDaMhLejMVEX9M X/tVkbKRU+yNQvIobH+R8bsFmGFolptq2aGqCN1JnJiFA6afPPPSSYW3bmM0YeYgGR6D 8dEh1bW4WHEAfvPrSKLnmutU7GrsWFPMD0SbLCOtBnAUNPM9myBKavkcj7/A62FzrdEy VFx/JOcLkYBvur6PN0AiDY03UdCQjlDj1tzqqwCVcXUzfxqkKjx80P0w+7jWSXV73Cus 26rA== Return-Path: Received: from [192.168.0.185] ([179.183.11.205]) by smtp.gmail.com with ESMTPSA id 127sm7362466qkj.97.2020.03.04.05.17.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Mar 2020 05:17:04 -0800 (PST) Subject: Re: [Regression] [PATCH] Do not print empty-group regs when printing general ones To: Shahab Vahedi , Christian Biesinger Cc: Shahab Vahedi , "gdb-patches@sourceware.org" , Claudiu Zissulescu , Francois Bedard , Andrew Burgess References: <20200120155315.30333-1-shahab.vahedi@gmail.com> <75f76108-a233-6fce-66a2-86452371e1be@linaro.org> <9c256b27-4a00-8830-46e2-934922e39cc2@linaro.org> <20200228133618.GA3269@gmail.com> <51718427-72e2-ce12-7181-26ec9b38d947@linaro.org> <20200228140801.GB3269@gmail.com> <20200228141545.GC3269@gmail.com> From: Luis Machado Message-ID: <33535485-cd54-6817-6e09-7f9de835766e@linaro.org> Date: Wed, 04 Mar 2020 13:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200228141545.GC3269@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-03/txt/msg00083.txt On 2/28/20 11:15 AM, Shahab Vahedi wrote: > On Fri, Feb 28, 2020 at 08:10:35AM -0600, Christian Biesinger wrote: >> On Fri, Feb 28, 2020 at 8:08 AM Shahab Vahedi wrote: >>> >>> On Fri, Feb 28, 2020 at 10:50:54AM -0300, Luis Machado wrote: >>>> On 2/28/20 10:36 AM, Shahab Vahedi wrote: >>>>> On Fri, Feb 28, 2020 at 10:31:26AM -0300, Luis Machado wrote: >>>>>> >>>>>> >>>>>> On 2/28/20 10:22 AM, Christian Biesinger wrote: >>>>>>> On Fri, Feb 28, 2020 at 7:08 AM Luis Machado wrote: >>>>>>>> >>>>>>>> On 1/31/20 7:34 AM, Shahab Vahedi wrote: >>>>>>>>> This patch was reviewed once (as OK): >>>>>>>>> https://sourceware.org/ml/gdb-patches/2020-01/msg00613.html >>>>>>>>> >>>>>>>>> Could someone review/merge it? >>>>>>>>> >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> Shahab >>>>>>>>> >>>>>>>> >>>>>>>> FTR, this has broken general register printing for ARM/AArch64. Now >>>>>>>> "info reg" shows nothing. >>>>>>>> >>>>>>>> Given there are already remote stubs, probes and gdbservers running out >>>>>>>> there, this is an undesirable change to have. >>>>>>>> >>>>>>>> I had an IRC chat with Christian and he pointed me at some documentation >>>>>>>> stating empty-group registers should not be printed, but i think this is >>>>>>>> a case where the implementation has diverged from the documentation. >>>>>>>> >>>>>>>> https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format >>>>>>>> >>>>>>>> We could probably patch up any non-standard target description XML's >>>>>>>> from now on, but the existing behavior may have to be preserved. >>>>>>> >>>>>>> Most targets under features/ do not specify group="general" in their >>>>>>> XML files for anything (only S/390 does); it seems like that should >>>>>>> maybe be fixed either way? >>>>>> >>>>>> I agree. >>>>> >>>>> The documentation [1] says: >>>>> If no group is specified, GDB will not display the register in info registers >>>>> >>>>> [1] >>>>> https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format >>>>> >>>> >>>> That's valid, but unfortunately it doesn't change the fact the existing code >>>> is breaking backwards compatibility with the installed base. >>>> >>>> As we discussed on IRC, this is code put together in early 2007 and hasn't >>>> been touched since, apart from a small change in 2017 to cope with arbitrary >>>> group strings. Plus we have plenty of existing target descriptions that do >>>> not honor explicitly setting a register group. >>>> >>>> With that said, i think the documentation would have a lower priority in >>>> this regard. We should fix the existing target descriptions to be more >>>> strict with the group names, but the old behavior would have to be honored >>>> IMO. >>> >>> I agree. The point in the patch was to make extra registers go away. However, >>> it apparently eliminated the output of "info registers" for other targets and >>> that is not OK. No matter sticking to the documentation or not. Feel free to >>> revert the patch. >>> >>> Ideally I'd like a solution that: >>> >>> 1) "info registers": must not print the non-default (non-general) registers >>> as it was the case with c9c895b9666 >>> >>> 2) "info registers": should only print "general" group registers. This requires >>> adding 'group="general"' to every XML features out there. So I don't know >>> how realistic it is. >> >> Maybe we can do something like: if there is no register with >> group=general, print all registers w/o group. Otherwise, only print >> registers with group=general. >> >> Christian > > I like that. We must update the documentation to something like: > > * no gorup=... mentioned is the same as group="general" --> default > * group="" explicitly means that the register does not belong to any group. > > And make the code act accordingly. > > > Cheers, > Shahab > Should we revert this for now then, and address this problem with an upcoming patch?