From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122521 invoked by alias); 2 Mar 2020 19:07:14 -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 122510 invoked by uid 89); 2 Mar 2020 19:07:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.9 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=shahab X-HELO: mail-qk1-f194.google.com Received: from mail-qk1-f194.google.com (HELO mail-qk1-f194.google.com) (209.85.222.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Mar 2020 19:07:09 +0000 Received: by mail-qk1-f194.google.com with SMTP id p62so827122qkb.0 for ; Mon, 02 Mar 2020 11:07:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=V+mvI4VHVxp12tQP4wtMVKYNKJ2fo62zaWQUn2w6LYc=; b=dyruXK7rkuiZmOzWYfqQ2FKYHYqTCNA11PNeQC/aUZXO5VHmbBc7wqf9dU0tyfexAN 3HgEZox9p6nBYGOKa0qeeSu5cAK9+IP+aBdFZwM4H+57Nzg4PXNwBkSaEAlZrvGRTV2G uc07zUvwGOhR/LE4MnObV0f7jG+ARkhNMEOCOvWkPG5HDBh4RZgH4vLCxOYAfqwZHF5U RQkXzMvsjx624hA8ye6efRUTLN7TaP70m1Oy+rBSjmlSAFoK7piXStSUvkexLtd65tuA 0EhVn0KcUJYibvtOecTXoR/MiuSWaxKUjJiQSNn2zOWLFYWiM9DrWXWoTPFOnY7potKW HlsQ== Return-Path: Received: from [192.168.0.185] ([191.34.156.219]) by smtp.gmail.com with ESMTPSA id d7sm7691665qkg.62.2020.03.02.11.07.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Mar 2020 11:07:06 -0800 (PST) Subject: Re: [PATCH] Explicitly put registers in the general group for ARM To: Christian Biesinger , gdb-patches@sourceware.org References: <20200302175915.176614-1-cbiesinger@google.com> From: Luis Machado Message-ID: <9909656a-b1d9-a897-c272-a6b788d93620@linaro.org> Date: Mon, 02 Mar 2020 19:07: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: <20200302175915.176614-1-cbiesinger@google.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-03/txt/msg00038.txt 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. 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. Users can always define some arbitrary group name that will not get displayed by "info registers" anyway, right? 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. > > 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 @@ > > > > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - > - > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > > > - > + > >