From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128745 invoked by alias); 4 Mar 2020 21:05:02 -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 128572 invoked by uid 89); 4 Mar 2020 21:04:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-29.5 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=x26, HX-Received:16c8, offending, honoring X-HELO: mail-ot1-f68.google.com Received: from mail-ot1-f68.google.com (HELO mail-ot1-f68.google.com) (209.85.210.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Mar 2020 21:03:52 +0000 Received: by mail-ot1-f68.google.com with SMTP id x19so3487820otp.7 for ; Wed, 04 Mar 2020 13:03:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/jmYn187l5o34zHK28DBV6FFWV0v8UOaZd9CngbWV9w=; b=siefnlXAuhkuCq6K6ppC4C7ScT3DCoyXDZo4sNQHTYhCYEnISIgmlAGFOlEq/DQKsg NgJqYoJUzunyVa+O/5LFlHlhM3ma6C+2Sd664tFKKm/IbprYohJjPYW/jkbEjeX4/4Da oWbMBWiGIkKh4OJicomlSMneDjrLa36mECiVzZ0TKG+l06v6y20GLA16ZD/Iv70f0JxO InR6rjw0HpYHwNPL8JZ4G1+he/ExDmmoB6I2BWmWr9VCzdGOcpmrXG5GPISHH4A8nnoq l/NFo4izQZ+nUwG3OsaGQKafMceB0Lgr6pTW/jsh1gjrYVRCcXJ55PgpSQV9AirNY7TH wm4A== MIME-Version: 1.0 References: <20200302175915.176614-1-cbiesinger@google.com> <9909656a-b1d9-a897-c272-a6b788d93620@linaro.org> In-Reply-To: <9909656a-b1d9-a897-c272-a6b788d93620@linaro.org> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Wed, 04 Mar 2020 21:05:00 -0000 Message-ID: Subject: Re: [PATCH] Explicitly put registers in the general group for ARM To: Luis Machado Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2020-03/txt/msg00102.txt 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. > 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 @@ > > > > > > > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > - > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > > > > > - > > + > > > >