From: Luis Machado <luis.machado@linaro.org>
To: Christian Biesinger <cbiesinger@google.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] Explicitly put registers in the general group for ARM
Date: Mon, 02 Mar 2020 19:07:00 -0000 [thread overview]
Message-ID: <9909656a-b1d9-a897-c272-a6b788d93620@linaro.org> (raw)
In-Reply-To: <20200302175915.176614-1-cbiesinger@google.com>
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 <cbiesinger@google.com>
>
> * 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 @@
>
> <!DOCTYPE feature SYSTEM "gdb-target.dtd">
> <feature name="org.gnu.gdb.aarch64.core">
> - <reg name="x0" bitsize="64"/>
> - <reg name="x1" bitsize="64"/>
> - <reg name="x2" bitsize="64"/>
> - <reg name="x3" bitsize="64"/>
> - <reg name="x4" bitsize="64"/>
> - <reg name="x5" bitsize="64"/>
> - <reg name="x6" bitsize="64"/>
> - <reg name="x7" bitsize="64"/>
> - <reg name="x8" bitsize="64"/>
> - <reg name="x9" bitsize="64"/>
> - <reg name="x10" bitsize="64"/>
> - <reg name="x11" bitsize="64"/>
> - <reg name="x12" bitsize="64"/>
> - <reg name="x13" bitsize="64"/>
> - <reg name="x14" bitsize="64"/>
> - <reg name="x15" bitsize="64"/>
> - <reg name="x16" bitsize="64"/>
> - <reg name="x17" bitsize="64"/>
> - <reg name="x18" bitsize="64"/>
> - <reg name="x19" bitsize="64"/>
> - <reg name="x20" bitsize="64"/>
> - <reg name="x21" bitsize="64"/>
> - <reg name="x22" bitsize="64"/>
> - <reg name="x23" bitsize="64"/>
> - <reg name="x24" bitsize="64"/>
> - <reg name="x25" bitsize="64"/>
> - <reg name="x26" bitsize="64"/>
> - <reg name="x27" bitsize="64"/>
> - <reg name="x28" bitsize="64"/>
> - <reg name="x29" bitsize="64"/>
> - <reg name="x30" bitsize="64"/>
> - <reg name="sp" bitsize="64" type="data_ptr"/>
> + <reg name="x0" bitsize="64" group="general"/>
> + <reg name="x1" bitsize="64" group="general"/>
> + <reg name="x2" bitsize="64" group="general"/>
> + <reg name="x3" bitsize="64" group="general"/>
> + <reg name="x4" bitsize="64" group="general"/>
> + <reg name="x5" bitsize="64" group="general"/>
> + <reg name="x6" bitsize="64" group="general"/>
> + <reg name="x7" bitsize="64" group="general"/>
> + <reg name="x8" bitsize="64" group="general"/>
> + <reg name="x9" bitsize="64" group="general"/>
> + <reg name="x10" bitsize="64" group="general"/>
> + <reg name="x11" bitsize="64" group="general"/>
> + <reg name="x12" bitsize="64" group="general"/>
> + <reg name="x13" bitsize="64" group="general"/>
> + <reg name="x14" bitsize="64" group="general"/>
> + <reg name="x15" bitsize="64" group="general"/>
> + <reg name="x16" bitsize="64" group="general"/>
> + <reg name="x17" bitsize="64" group="general"/>
> + <reg name="x18" bitsize="64" group="general"/>
> + <reg name="x19" bitsize="64" group="general"/>
> + <reg name="x20" bitsize="64" group="general"/>
> + <reg name="x21" bitsize="64" group="general"/>
> + <reg name="x22" bitsize="64" group="general"/>
> + <reg name="x23" bitsize="64" group="general"/>
> + <reg name="x24" bitsize="64" group="general"/>
> + <reg name="x25" bitsize="64" group="general"/>
> + <reg name="x26" bitsize="64" group="general"/>
> + <reg name="x27" bitsize="64" group="general"/>
> + <reg name="x28" bitsize="64" group="general"/>
> + <reg name="x29" bitsize="64" group="general"/>
> + <reg name="x30" bitsize="64" group="general"/>
> + <reg name="sp" bitsize="64" type="data_ptr" group="general"/>
>
> - <reg name="pc" bitsize="64" type="code_ptr"/>
> + <reg name="pc" bitsize="64" type="code_ptr" group="general"/>
>
> <flags id="cpsr_flags" size="4">
> <!-- Stack Pointer. -->
> @@ -86,6 +86,6 @@
> <!-- Negative Condition flag. -->
> <field name="N" start="31" end="31"/>
> </flags>
> - <reg name="cpsr" bitsize="32" type="cpsr_flags"/>
> + <reg name="cpsr" bitsize="32" type="cpsr_flags" group="general"/>
>
> </feature>
> 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 @@
>
> <!DOCTYPE feature SYSTEM "gdb-target.dtd">
> <feature name="org.gnu.gdb.arm.core">
> - <reg name="r0" bitsize="32" type="uint32"/>
> - <reg name="r1" bitsize="32" type="uint32"/>
> - <reg name="r2" bitsize="32" type="uint32"/>
> - <reg name="r3" bitsize="32" type="uint32"/>
> - <reg name="r4" bitsize="32" type="uint32"/>
> - <reg name="r5" bitsize="32" type="uint32"/>
> - <reg name="r6" bitsize="32" type="uint32"/>
> - <reg name="r7" bitsize="32" type="uint32"/>
> - <reg name="r8" bitsize="32" type="uint32"/>
> - <reg name="r9" bitsize="32" type="uint32"/>
> - <reg name="r10" bitsize="32" type="uint32"/>
> - <reg name="r11" bitsize="32" type="uint32"/>
> - <reg name="r12" bitsize="32" type="uint32"/>
> - <reg name="sp" bitsize="32" type="data_ptr"/>
> - <reg name="lr" bitsize="32"/>
> - <reg name="pc" bitsize="32" type="code_ptr"/>
> + <reg name="r0" bitsize="32" type="uint32" group="general"/>
> + <reg name="r1" bitsize="32" type="uint32" group="general"/>
> + <reg name="r2" bitsize="32" type="uint32" group="general"/>
> + <reg name="r3" bitsize="32" type="uint32" group="general"/>
> + <reg name="r4" bitsize="32" type="uint32" group="general"/>
> + <reg name="r5" bitsize="32" type="uint32" group="general"/>
> + <reg name="r6" bitsize="32" type="uint32" group="general"/>
> + <reg name="r7" bitsize="32" type="uint32" group="general"/>
> + <reg name="r8" bitsize="32" type="uint32" group="general"/>
> + <reg name="r9" bitsize="32" type="uint32" group="general"/>
> + <reg name="r10" bitsize="32" type="uint32" group="general"/>
> + <reg name="r11" bitsize="32" type="uint32" group="general"/>
> + <reg name="r12" bitsize="32" type="uint32" group="general"/>
> + <reg name="sp" bitsize="32" type="data_ptr" group="general"/>
> + <reg name="lr" bitsize="32" group="general"/>
> + <reg name="pc" bitsize="32" type="code_ptr"/ group="general">
>
> <!-- The CPSR is register 25, rather than register 16, because
> the FPA registers historically were placed between the PC
> and the CPSR in the "g" packet. -->
> - <reg name="cpsr" bitsize="32" regnum="25"/>
> + <reg name="cpsr" bitsize="32" regnum="25" group="general"/>
> </feature>
>
next prev parent reply other threads:[~2020-03-02 19:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-02 17:59 Christian Biesinger via gdb-patches
2020-03-02 19:07 ` Luis Machado [this message]
2020-03-04 21:05 ` Christian Biesinger via gdb-patches
2020-03-04 22:01 ` Luis Machado
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9909656a-b1d9-a897-c272-a6b788d93620@linaro.org \
--to=luis.machado@linaro.org \
--cc=cbiesinger@google.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox