This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH] Explicitly put registers in the general group for ARM

From: Luis Machado <luis dot machado at linaro dot org>
To: Christian Biesinger <cbiesinger at google dot com>, gdb-patches at sourceware dot org
Date: Mon, 2 Mar 2020 16:07:03 -0300
Subject: Re: [PATCH] Explicitly put registers in the general group for ARM
References: <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 donefor all the targets that currently do not explicitly set the group nameand fall into the "general" category based on the previous heuristic. Ofcourse, this is if we want to honor the existing documentation.

There are also other target description producers out there that willkeep generating wrong group information, and there is little we can doabout it. So it makes me wonder if it is worth honoring thedocumentation or if we should just update the documentation with theexisting behavior, since we'll have to keep it for a while.

Users can always define some arbitrary group name that will not getdisplayed by "info registers" anyway, right?

I'm thinking the offending patch will still need to be reverted or fixedup to bring back the old behavior.

Shahab was working on an alternative patch, but it hasn't been submittedyet.


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>

Follow-Ups:
- Re: [PATCH] Explicitly put registers in the general group for ARM
  - From: Christian Biesinger via gdb-patches

References:
- [PATCH] Explicitly put registers in the general group for ARM
  - From: Christian Biesinger via gdb-patches

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]