From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26436 invoked by alias); 4 Mar 2020 22:01:26 -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 26338 invoked by uid 89); 4 Mar 2020 22:01:25 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*RU:209.85.221.66, HX-Spam-Relays-External:209.85.221.66, redo, H*c:alternative X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Mar 2020 22:01:19 +0000 Received: by mail-wr1-f66.google.com with SMTP id j16so4460245wrt.3 for ; Wed, 04 Mar 2020 14:01:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7TIj9CMc1JAQzzWbYgSnbktnRt89bbaLhllPXjI8Sm0=; b=JuQmUMIGjLWqyRSWZ9rG4MOtL4+El+Bn6JSqSqt4IZTtAz/C2dpox6Sv0M/QRcJNQH Kfy5j11AtlhXwm1xmFFEM5b3ekigtkBYQhbM67EwrURavm5FireKF7asvuMIibZYs4ZM xHiy5mTnoPDiVT9oyO2iUZSAZhaMO3UcJJE4MWk8K50X8OVopvdB2htf5y7RAtHdnzIY gXfIygRrpq4mnaarKONeZMemL5vbfIF3uSeSJNo5FcadQ65Fg0F3K9BQmEW6Z9JB9NNH E0ISKNjpbCuu9CFN0wOpr34dwi453y9+XW4anEl3++WwP45xFkTc6AvAjQTxi9NI/FFl lyZA== MIME-Version: 1.0 References: <20200302175915.176614-1-cbiesinger@google.com> <9909656a-b1d9-a897-c272-a6b788d93620@linaro.org> In-Reply-To: From: Luis Machado Date: Wed, 04 Mar 2020 22:01:00 -0000 Message-ID: Subject: Re: [PATCH] Explicitly put registers in the general group for ARM To: Christian Biesinger Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2020-03/txt/msg00103.txt On Wed, Mar 4, 2020, 18:03 Christian Biesinger wrote: > On Mon, Mar 2, 2020 at 1:07 PM Luis Machado > wrote: > > > > Hi, > > > > On 3/2/20 2:59 PM, Christian Biesinger via gdb-patches wrote: > > > This seems like the right thing per the documentation: > > > > https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format > > > "If no group is specified, GDB will not display the register in > > > info registers." > > > but should also help with this issue: > > > https://sourceware.org/ml/gdb-patches/2020-02/msg01038.html] > > > > I don't have a problem with the patch, but i think this should be done > > for all the targets that currently do not explicitly set the group name > > and fall into the "general" category based on the previous heuristic. Of > > course, this is if we want to honor the existing documentation. > > Yeah, I agree. But I don't have the bandwidth to do this all targets, > it is a good amount of manual work. > Just to be clear, i'm not expecting you to fix all the cases. I could do it as well, but i want to make sure we agree on a reasonable way forward, given the existing behavior, so we don't have to redo the work later. Or do work that won't be useful. I'll raise a thread about this and we can share some thoughts then. > > There are also other target description producers out there that will > > keep generating wrong group information, and there is little we can do > > about it. So it makes me wonder if it is worth honoring the > > documentation or if we should just update the documentation with the > > existing behavior, since we'll have to keep it for a while. > > Yeah, but I still think it is good to be explicit about which > registers are in the general group? If y'all disagree, I can drop this > patch. > > > Users can always define some arbitrary group name that will not get > > displayed by "info registers" anyway, right? > > That is true. > > > I'm thinking the offending patch will still need to be reverted or fixed > > up to bring back the old behavior. > > > > Shahab was working on an alternative patch, but it hasn't been submitted > > yet. > > Yeah, I think this is independent of that. > > Christian > > > > Note that "set tdesc filename features/arm/arm-core.xml; maint print > c-tdesc" > > > prints an empty target description both before and after this change, > > > so I just manually edited the .c file to specify the group. > > > > > > gdb/ChangeLog: > > > > > > 2020-03-02 Christian Biesinger > > > > > > * features/aarch64-core.c (create_feature_aarch64_core): Put all > > > registers in the "general" group. > > > * features/aarch64-core.xml: Likewise. > > > * features/arm/arm-core.c (create_feature_arm_arm_core): > Likewise. > > > * features/arm/arm-core.xml: Likewise. > > > --- > > > gdb/features/aarch64-core.c | 68 > +++++++++++++++++------------------ > > > gdb/features/aarch64-core.xml | 68 > +++++++++++++++++------------------ > > > gdb/features/arm/arm-core.c | 34 +++++++++--------- > > > gdb/features/arm/arm-core.xml | 34 +++++++++--------- > > > 4 files changed, 102 insertions(+), 102 deletions(-) > > > > > > diff --git a/gdb/features/aarch64-core.c b/gdb/features/aarch64-core.c > > > index a080a641c4..f929b2cbf9 100644 > > > --- a/gdb/features/aarch64-core.c > > > +++ b/gdb/features/aarch64-core.c > > > @@ -30,39 +30,39 @@ create_feature_aarch64_core (struct target_desc > *result, long regnum) > > > tdesc_add_flag (type_with_fields, 30, "Z"); > > > tdesc_add_flag (type_with_fields, 31, "N"); > > > > > > - tdesc_create_reg (feature, "x0", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x1", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x2", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x3", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x4", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x5", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x6", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x7", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x8", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x9", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x10", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x11", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x12", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x13", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x14", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x15", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x16", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x17", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x18", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x19", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x20", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x21", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x22", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x23", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x24", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x25", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x26", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x27", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x28", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x29", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "x30", regnum++, 1, NULL, 64, "int"); > > > - tdesc_create_reg (feature, "sp", regnum++, 1, NULL, 64, "data_ptr"); > > > - tdesc_create_reg (feature, "pc", regnum++, 1, NULL, 64, "code_ptr"); > > > - tdesc_create_reg (feature, "cpsr", regnum++, 1, NULL, 32, > "cpsr_flags"); > > > + tdesc_create_reg (feature, "x0", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x1", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x2", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x3", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x4", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x5", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x6", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x7", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x8", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x9", regnum++, 1, "general", 64, "int"); > > > + tdesc_create_reg (feature, "x10", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x11", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x12", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x13", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x14", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x15", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x16", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x17", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x18", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x19", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x20", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x21", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x22", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x23", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x24", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x25", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x26", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x27", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x28", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x29", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "x30", regnum++, 1, "general", 64, > "int"); > > > + tdesc_create_reg (feature, "sp", regnum++, 1, "general", 64, > "data_ptr"); > > > + tdesc_create_reg (feature, "pc", regnum++, 1, "general", 64, > "code_ptr"); > > > + tdesc_create_reg (feature, "cpsr", regnum++, 1, "general", 32, > "cpsr_flags"); > > > return regnum; > > > } > > > diff --git a/gdb/features/aarch64-core.xml > b/gdb/features/aarch64-core.xml > > > index ee6a3a6dfa..2f66088c6d 100644 > > > --- a/gdb/features/aarch64-core.xml > > > +++ b/gdb/features/aarch64-core.xml > > > @@ -8,40 +8,40 @@ > > > > > > > > > > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > > > > - > > > + > > > > > > > > > > > > @@ -86,6 +86,6 @@ > > > > > > > > > > > > - > > > + > > > > > > > > > diff --git a/gdb/features/arm/arm-core.c b/gdb/features/arm/arm-core.c > > > index e401411fc5..4519752975 100644 > > > --- a/gdb/features/arm/arm-core.c > > > +++ b/gdb/features/arm/arm-core.c > > > @@ -9,23 +9,23 @@ create_feature_arm_arm_core (struct target_desc > *result, long regnum) > > > struct tdesc_feature *feature; > > > > > > feature = tdesc_create_feature (result, "org.gnu.gdb.arm.core"); > > > - tdesc_create_reg (feature, "r0", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r1", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r2", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r3", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r4", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r5", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r6", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r7", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r8", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r9", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r10", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r11", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "r12", regnum++, 1, NULL, 32, "uint32"); > > > - tdesc_create_reg (feature, "sp", regnum++, 1, NULL, 32, "data_ptr"); > > > - tdesc_create_reg (feature, "lr", regnum++, 1, NULL, 32, "int"); > > > - tdesc_create_reg (feature, "pc", regnum++, 1, NULL, 32, "code_ptr"); > > > + tdesc_create_reg (feature, "r0", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r1", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r2", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r3", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r4", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r5", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r6", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r7", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r8", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r9", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r10", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r11", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "r12", regnum++, 1, "general", 32, > "uint32"); > > > + tdesc_create_reg (feature, "sp", regnum++, 1, "general", 32, > "data_ptr"); > > > + tdesc_create_reg (feature, "lr", regnum++, 1, "general", 32, "int"); > > > + tdesc_create_reg (feature, "pc", regnum++, 1, "general", 32, > "code_ptr"); > > > regnum = 25; > > > - tdesc_create_reg (feature, "cpsr", regnum++, 1, NULL, 32, "int"); > > > + tdesc_create_reg (feature, "cpsr", regnum++, 1, "general", 32, > "int"); > > > return regnum; > > > } > > > diff --git a/gdb/features/arm/arm-core.xml > b/gdb/features/arm/arm-core.xml > > > index 144fc4638a..d506004fe1 100644 > > > --- a/gdb/features/arm/arm-core.xml > > > +++ b/gdb/features/arm/arm-core.xml > > > @@ -7,25 +7,25 @@ > > > > > > > > > > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > - > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > + > > > > > > > > > - > > > + > > > > > > >