From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68723 invoked by alias); 20 Jan 2020 20:58:48 -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 68662 invoked by uid 89); 20 Jan 2020 20:58:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*RU:209.85.221.65, HX-Spam-Relays-External:209.85.221.65 X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 20:58:42 +0000 Received: by mail-wr1-f65.google.com with SMTP id q6so912025wro.9 for ; Mon, 20 Jan 2020 12:58:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=T9/fLhf+UNdl/4B2jh3h7PGshlsJdXtm/Xxkqx4eyVY=; b=V+S+Ztfo2V+njYiyrBRmnzo9BtZHK6xhqQHQv0RNHHlQJcbiixhpGPGODShnJ8PLdQ 09fvsZrQyIYSskD/ZlY/AtAng0AEa5JfSkArbRHIUD/4lc7VoV6OD4UfgtU0e6LZpvCy h6s4Mjn6mwdW0WBBaxGYCdwbsVHmVcsVmVKFUsfti4JCaCVLt20rjPdPFxpIym3SPMYn AETj70Zjf3evo4gYWKMg/5cPFCrMi5ojmdrpQ+nH5TJWdYpY3etjRn6hDaq7TGRnO8mD RMOE9Wx71p2RE6rssjlLvAijI7Z/j1uBRVAIjzpq6+pFJZkbMotOZJPoRy+heRO8S72J glvg== Return-Path: Received: from localhost (host86-191-239-73.range86-191.btcentralplus.com. [86.191.239.73]) by smtp.gmail.com with ESMTPSA id f1sm49467467wru.6.2020.01.20.12.58.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 20 Jan 2020 12:58:39 -0800 (PST) Date: Mon, 20 Jan 2020 23:04:00 -0000 From: Andrew Burgess To: Shahab Vahedi Cc: gdb-patches@sourceware.org, Shahab Vahedi , Claudiu Zissulescu , Francois Bedard Subject: Re: [PATCH] Do not print empty-group regs when printing general ones Message-ID: <20200120205838.GG3865@embecosm.com> References: <20200120155315.30333-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200120155315.30333-1-shahab.vahedi@gmail.com> X-Fortune: He who laughs last didn't get the joke. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00614.txt.bz2 * Shahab Vahedi [2020-01-20 16:53:15 +0100]: > From: Shahab Vahedi > > When the command "info registers" (same as "info registers general"), > is issued, _all_ the registers from a tdesc XML are printed. This > includes the registers with empty register groups (set as "") which > are supposed to be only printed by "info registers all" (or "info > all-registers"). > > This bug got introduced after all the overhauls that the > tdesc_register_in_reggroup_p() went through. You can see that the > logic of tdesc_register_in_reggroup_p() did NOT remain the same after > all those changes: > > git difftool c9c895b9666..HEAD -- gdb/target-descriptions.c > > With the current implementation, when the reg->group is an empty > string, this function returns -1, while in the working revision > (c9c895b9666), it returned 0. This patch makes sure that the 0 is > returned again. > > The old implementation of tdesc_register_in_reggroup_p() returned > -1 when "reggroup" was set to "all_reggroups" at line 4 below: > > 1 tdesc_register_reggroup_p (...) > 2 { > 3 ... > 4 ret = tdesc_register_in_reggroup_p (gdbarch, regno, reggroup); > 5 if (ret != -1) > 6 return ret; > 7 > 8 return default_register_reggroup_p (gdbarch, regno, reggroup); > 9 } > > As a result, the execution continued at line 8 and the > default_register_reggroup_p(..., reggroup=all_reggroups) would > return 1. However, with the current implementation of > tdesc_register_in_reggroup_p() that allows checking against any > arbitrary group name, it returns 0 when comparing the "reg->group" > against the string "all" which is the group name for "all_reggroups". > I have added a special check to cover this case and > "info all-registers" works as expected. > > gdb/ChangeLog: > 2020-01-20 Shahab Vahedi > > * target-descriptions.c (tdesc_register_in_reggroup_p): > Return 0 when reg->group is empty and reggroup is not. Looks good to me. Thanks, Andrew > --- > gdb/target-descriptions.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index 04711ba2fa5..937210cf25e 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -977,13 +977,15 @@ tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno, > { > struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno); > > - if (reg != NULL && !reg->group.empty () > - && (reg->group == reggroup_name (reggroup))) > + if (reg != NULL) > + { > + if (reggroup == all_reggroup) > return 1; > - > - if (reg != NULL > - && (reggroup == save_reggroup || reggroup == restore_reggroup)) > - return reg->save_restore; > + else if (reggroup == save_reggroup || reggroup == restore_reggroup) > + return reg->save_restore; > + else > + return (int) (reg->group == reggroup_name (reggroup)); > + } > > return -1; > } > -- > 2.25.0 >