Support for arbitrary reggroups

Added help text in 'help info registers'
Fixed 'register' typos
Fixed style of test program
Fixed copyright '2017'
Fixed code styles in expect

Allow for freeing reggroups

Add documentation for this feature

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne <[email protected]>
Date: Tue, 19 Dec 2017 23:22:54 +0900

Until now this feature has existed but was not documented. Adding docs and tests.

+ at item info registers @var{reggroup} @dots{}
+Print the name and value of the registers in each of the specified
+ at var{reggroup}. The @var{reggoup} can be any of those returned by

+ at code{maint print reggroups}.

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne <[email protected]>

gdb/NEWS +0 -0

index 44f481d1f5..7d27262aee 100644

Quoted 10 lines

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne <[email protected]>

gdb/NEWS +0 -0

index 44f481d1f5..7d27262aee 100644
@@ -3,6 +3,13 @@
 
 *** Changes since GDB 8.0
 

gdb/NEWS line 11

Quoted 6 lines

gdb/NEWS +6 -0

+* GDB now supports dynamically creating arbitrary register groups specified
+  in XML target descriptions.  This allows for finer grain grouping of
+  registers on systems with a large amount of registers.
+
+* On Unix systems, GDBserver now does globbing expansion and variable
+  substitution in inferior command line arguments.

The second paragraph doesn't belong to this changeset, right?

gdb/NEWS

Quoted 9 lines

gdb/NEWS +4 -4

 @item group
-The register group to which this register belongs.  It must
-be either @code{general}, @code{float}, or @code{vector}.  If no
- at var{group} is specified, @value{GDBN} will not display the register
-in @code{info registers}.
+The register group to which this register belongs.  It can be one of the
+standard register groups @code{general}, @code{float}, @code{vector} or an
+arbitrary string.  The string should be limited to alphanumeric characters
+and internal hyphens.  If no @var{group} is specified, @value{GDBN} will

What do you mean by "internal hyphens"?

Thanks.

Quoted 6 lines

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne <[email protected]>
Date: Tue, 19 Dec 2017 23:22:54 +0900

Until now this feature has existed but was not documented. Adding docs and tests.

Thanks!

Quoted 3 lines

+ at item info registers @var{reggroup} @dots{}
+Print the name and value of the registers in each of the specified
+ at var{reggroup}. The @var{reggoup} can be any of those returned by

Please use "@var{reggroup}s", with the trailing "s", otherwise this is not correct English.

Quoted 1 line

+ at code{maint print reggroups}.

Please add here a cross-reference to the node where "maint print reggroups" is described.

gdb/infcmd.c interrupt all running threads in non-stop mode, use the -a option.")); · line 3465

Quoted 6 lines

   c = add_info ("registers", info_registers_command, _("\

List of integer registers and their contents, for selected stack frame.\n\

gdb/infcmd.c +3 -1

-Register name as argument means describe only that register."));
+Register name as argument means describe only that register.\n\
+Register group name as argument means describe the registers in the\n\
+named register group."));

Since this command accepts more than one reggroup, I think the doc string should mention that.

The documentation parts are okay with these nits fixed.

Quoted 10 lines · Eli Zaretskii

Quoted 10 lines

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne <[email protected]>

gdb/NEWS +0 -0

index 44f481d1f5..7d27262aee 100644
@@ -3,6 +3,13 @@
 
 *** Changes since GDB 8.0
 

Quoted 8 lines

gdb/NEWS line 11

Quoted 6 lines

gdb/NEWS +6 -0

+* GDB now supports dynamically creating arbitrary register groups specified
+  in XML target descriptions.  This allows for finer grain grouping of
+  registers on systems with a large amount of registers.
+
+* On Unix systems, GDBserver now does globbing expansion and variable
+  substitution in inferior command line arguments.

The second paragraph doesn't belong to this changeset, right?

Right, I accidently brought this in during conflict resolution. Will fix.

Quoted 11 lines

gdb/NEWS

Quoted 9 lines

gdb/NEWS +4 -4

 @item group
-The register group to which this register belongs.  It must
-be either @code{general}, @code{float}, or @code{vector}.  If no
- at var{group} is specified, @value{GDBN} will not display the register
-in @code{info registers}.
+The register group to which this register belongs.  It can be one of the
+standard register groups @code{general}, @code{float}, @code{vector} or an
+arbitrary string.  The string should be limited to alphanumeric characters
+and internal hyphens.  If no @var{group} is specified, @value{GDBN} will

What do you mean by "internal hyphens"?

This means, hyphens withing the register group name, not starting or ending with hyphens. (i.e. special-spr, but not rg1- or -rg2)

Date: Wed, 20 Dec 2017 07:13:00 +0900
From: Stafford Horne <[email protected]>
Cc: gdb-patches at sourceware.org, openrisc at lists.librecores.org

Quoted 11 lines

gdb/doc/gdb.texinfo architecture's normal floating point format) of the correct size for · line 41767

Quoted 9 lines

@item group

gdb/doc/gdb.texinfo +4 -4

-The register group to which this register belongs.  It must
-be either @code{general}, @code{float}, or @code{vector}.  If no
- at var{group} is specified, @value{GDBN} will not display the register
-in @code{info registers}.
+The register group to which this register belongs.  It can be one of the
+standard register groups @code{general}, @code{float}, @code{vector} or an
+arbitrary string.  The string should be limited to alphanumeric characters
+and internal hyphens.  If no @var{group} is specified, @value{GDBN} will

What do you mean by "internal hyphens"?

This means, hyphens withing the register group name, not starting or ending with hyphens. (i.e. special-spr, but not rg1- or -rg2)

Quoted 8 lines

From: Stafford Horne <[email protected]>
Cc: Openrisc <[email protected]>, Stafford Horne
<[email protected]>
Date: Tue, 19 Dec 2017 23:22:54 +0900

Until now this feature has existed but was not documented. Adding docs
and tests.

Thanks!

Quoted 3 lines

+ at item info registers @var{reggroup} @dots{}
+Print the name and value of the registers in each of the specified
+ at var{reggroup}. The @var{reggoup} can be any of those returned by

Please use "@var{reggroup}s", with the trailing "s", otherwise this is not correct English.

Quoted 1 line

+ at code{maint print reggroups}.

Please add here a cross-reference to the node where "maint print reggroups" is described.

Until now this feature has existed but was not documented. Adding docs and tests.

gdb/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* infcmd.c (_initialize_infcmd): Add help for info reg $reggroup feature.

gdb/doc/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* gdb.texinfo (Registers): Document info reg $reggroup feature.

gdb/testsuite/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* gdb.base/reggroups.c: New file.
* gdb.base/reggroups.exp: New file.

---
 gdb/doc/gdb.texinfo                  |  5 +++
 gdb/infcmd.c                         |  8 +++--
 gdb/testsuite/gdb.base/reggroups.c   |  5 +++
 gdb/testsuite/gdb.base/reggroups.exp | 63 
++++++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/reggroups.c
 create mode 100644 gdb/testsuite/gdb.base/reggroups.exp

gdb/doc/gdb.texinfo +0 -0

index 60ed80c363..e169260e7e 100644
@@ -11023,6 +11023,11 @@ and vector registers (in the selected stack 

frame).
Print the names and values of all registers, including floating-point and vector registers (in the selected stack frame).

+ at item info registers @var{reggroup} @dots{}

gdb/doc/gdb.texinfo +4 -0

+Print the name and value of the registers in each of the specified
+ at var{reggroup}.  The @var{reggoup} can be any of those returned by
+ at code{maint print reggroups}.
+
 @item info registers @var{regname} @dots{}
 Print the @dfn{relativized} value of each specified register 

@var{regname}.
As discussed in detail below, register values are normally relative to

gdb/infcmd.c +6 -2

index 8bde28eab6..6e449d4a0e 100644
@@ -3460,13 +3460,17 @@ interrupt all running threads in non-stop
 mode, use the -a option."));
 
   c = add_info ("registers", info_registers_command, _("\
 List of integer registers and their contents, for selected stack 
 frame.\n\
-Register name as argument means describe only that register."));
+Register name as argument means describe only that register.\n\
+Register group name as argument means describe the registers in the\n\
+named register group."));
   add_info_alias ("r", "registers", 1);
   set_cmd_completer (c, reg_or_group_completer);
 
   c = add_info ("all-registers", info_all_registers_command, _("\
 List of all registers and their contents, for selected stack frame.\n\
-Register name as argument means describe only that register."));
+Register name as argument means describe only that register.\n\
+Register group name as argument means describe the registers in the\n\
+named register group."));
   set_cmd_completer (c, reg_or_group_completer);

add_info ("program", info_program_command,

gdb/testsuite/gdb.base/reggroups.c new file +5 -0

new file mode 100644
index 0000000000..8e8f518aae
@@ -0,0 +1,5 @@
+int
+main()
+{
+  return 0;
+}

gdb/testsuite/gdb.base/reggroups.exp new file +16 -0

new file mode 100644
index 0000000000..c10f02567d
@@ -0,0 +1,63 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see 

<http://www.gnu.org/licenses/>.

gdb/testsuite/gdb.base/reggroups.exp new file +6 -0

+
+# Test listing reggroups and the registers in each group.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile 

debug]} {

gdb/testsuite/gdb.base/reggroups.exp new file +15 -0

+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+proc fetch_reggroups {test} {
+    global gdb_prompt
+    global expect_out
+
+    set reggroups {}
+    gdb_test_multiple "maint print reggroups" "get reggroups" {
+	-re "maint print reggroups\r\n" {

gdb/testsuite/gdb.base/reggroups.exp new file +6 -0

+	    if { [llength $reggroups] != 0 } {
+		pass $test
+	    } else {
+		fail "$test - didn't fetch any reggroups"
+	    }
+	}

Currently the reggroups gdbarch_data cannot be manipulated until after the gdbarch is completely initialized. This is usually done when the object init depends on architecture specific fields. In the case of reggroups it only depends on the obstack being available.

Coverting this to pre_init allows using reggroups during gdbarch initialization. This is needed to allow registering arbitrary reggroups
during gdbarch initializations.

gdb/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* reggroups.c (reggroups_init): Change to depend only on obstack rather than gdbarch.
(reggroup_add): Remove logic for forcing premature init. (_initialize_reggroup): Set reggroups_data with gdbarch_data_register_pre_init() rather than gdbarch_data_register_post_init().

---
 gdb/reggroups.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

gdb/reggroups.c +3 -3

index 2ecd0b494f..5d5e33f2a3 100644
@@ -26,6 +26,7 @@
 #include "regcache.h"
 #include "command.h"
 #include "gdbcmd.h"		/* For maintenanceprintlist.  */
+#include "gdb_obstack.h"
 
 /* Individual register groups.  */
 

@@ -76,10 +77,9 @@ struct reggroups
 static struct gdbarch_data *reggroups_data;
 
 static void *
-reggroups_init (struct gdbarch *gdbarch)
+reggroups_init (struct obstack *obstack)
 {
-  struct reggroups *groups = GDBARCH_OBSTACK_ZALLOC (gdbarch,
-						     struct reggroups);
+  struct reggroups *groups = OBSTACK_ZALLOC (obstack, struct 
 reggroups);
 
   groups->last = &groups->first;

   return groups;

gdb/reggroups.c +0 -7

@@ -105,13 +105,6 @@ reggroup_add (struct gdbarch *gdbarch, struct
 reggroup *group)
   struct reggroups *groups
     = (struct reggroups *) gdbarch_data (gdbarch, reggroups_data);
 
-  if (groups == NULL)
-    {
-      /* ULGH, called during architecture initialization.  Patch
-         things up.  */
-      groups = (struct reggroups *) reggroups_init (gdbarch);
-      deprecated_set_gdbarch_data (gdbarch, reggroups_data, groups);
-    }
   add_group (groups, group,
 	     GDBARCH_OBSTACK_ZALLOC (gdbarch, struct reggroup_el));

}

gdb/reggroups.c +1 -1

@@ -298,7 +291,7 @@ struct reggroup *const restore_reggroup = 
 &restore_group;
 void
 _initialize_reggroup (void)
 {
-  reggroups_data = gdbarch_data_register_post_init (reggroups_init);
+  reggroups_data = gdbarch_data_register_pre_init (reggroups_init);
 
   /* The pre-defined list of groups.  */

add_group (&default_groups, general_reggroup, XNEW (struct
reggroup_el));

Traditionally reggroups have been created via reggroup_new() during initialization code and never freed. Now, if we want to initialize reggroups dynamically (i.e. in target description) we should be able to free them. Create this function reggroup_gdbarch_new() which will allocate the reggroup memory onto the passed gdbarch obstack.

gdb/ChangeLog:

2017-06-10 Stafford Horne <[email protected]>

* reggroups.c (reggroup_gdbarch_new): New function.
* reggroups.h (reggroup_gdbarch_new): New function.

---
 gdb/reggroups.c | 12 ++++++++++++
 gdb/reggroups.h |  4 ++++
 2 files changed, 16 insertions(+)

gdb/reggroups.c +12 -0

index 5d5e33f2a3..acad91a0ab 100644
@@ -46,6 +46,18 @@ reggroup_new (const char *name, enum reggroup_type 
 type)
   return group;
 }
 
+struct reggroup *
+reggroup_gdbarch_new (struct gdbarch *gdbarch, const char *name,
+		      enum reggroup_type type)
+{
+  struct reggroup *group = GDBARCH_OBSTACK_ZALLOC (gdbarch,
+						   struct reggroup);
+
+  group->name = gdbarch_obstack_strdup (gdbarch, name);
+  group->type = type;
+  return group;
+}
+
 /* Register group attributes.  */
 

const char *

gdb/reggroups.h +4 -0

index 18fc1bf294..c1653cd39d 100644
@@ -41,6 +41,10 @@ extern struct reggroup *const restore_reggroup;
 /* Create a new local register group.  */
 extern struct reggroup *reggroup_new (const char *name,
 				      enum reggroup_type type);
+/* Create a new register group allocated onto the gdbarch obstack.  */
+extern struct reggroup *reggroup_gdbarch_new (struct gdbarch *gdbarch,
+					      const char *name,
+					      enum reggroup_type type);
 
 /* Add a register group (with attribute values) to the pre-defined 

list. */
extern void reggroup_add (struct gdbarch *gdbarch, struct reggroup
*group);

tdesc_register_in_reggroup_p in now able to handle arbitrary groups. This is useful when groups are created while the target descriptor file is received from the remote.

This can be the case of a soft core target processor where registers/groups can change.

gdb/ChangeLog:

2017-06-06 Franck Jullien <[email protected]>
Stafford Horne <[email protected]>

* target-descriptions.c (tdesc_register_in_reggroup_p): Support arbitrary strings.
* NEWS (Changes since GDB 8.0): Announce that GDB supports arbitrary reggroups.

gdb/testsuite/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* gdb.xml/extra-regs.xml: Add example foo reggroup.
* gdb.xml/tdesc-regs.exp: Add test to check for foo reggroup.

gdb/doc/ChangeLog:

2017-06-06 Stafford Horne <[email protected]>

* gdb.texinfo (Target Description Format): Explain that arbitrary strings are now allowed for register groups.

---
 gdb/NEWS                             |  7 ++++
 gdb/doc/gdb.texinfo                  |  9 +++--
 gdb/target-descriptions.c            | 74 
++++++++++++++++++------------------
 gdb/testsuite/gdb.xml/extra-regs.xml |  1 +
 gdb/testsuite/gdb.xml/tdesc-regs.exp |  3 ++
 5 files changed, 53 insertions(+), 41 deletions(-)

gdb/NEWS +1 -0

index 44f481d1f5..7d27262aee 100644
@@ -3,6 +3,13 @@
 
 *** Changes since GDB 8.0
 
+* GDB now supports dynamically creating arbitrary register groups 

specified

gdb/NEWS +6 -0

+  in XML target descriptions.  This allows for finer grain grouping of
+  registers on systems with a large amount of registers.
+
+* On Unix systems, GDBserver now does globbing expansion and variable
+  substitution in inferior command line arguments.
+
 * The 'ptype' command now accepts a '/o' flag, which prints the
   offsets and sizes of fields in a struct, like the pahole(1) tool.

gdb/doc/gdb.texinfo +0 -0

index e169260e7e..a71faaa89c 100644
@@ -41761,10 +41761,11 @@ architecture's normal floating point format)

of the correct size for
@var{bitsize}. The default is @code{int}.

@item group

gdb/doc/gdb.texinfo +1 -4

-The register group to which this register belongs.  It must
-be either @code{general}, @code{float}, or @code{vector}.  If no
- at var{group} is specified, @value{GDBN} will not display the register
-in @code{info registers}.
+The register group to which this register belongs.  It can be one of 

the
+standard register groups @code{general}, @code{float}, @code{vector} or an
+arbitrary string. The string should be limited to alphanumeric characters
+and internal hyphens. If no @var{group} is specified, @value{GDBN} will

gdb/doc/gdb.texinfo +1 -0

+not display the register in @code{info registers}.
 
 @end table

gdb/target-descriptions.c +1 -6

index 88ac55f404..1435f3f4cf 100644
@@ -111,12 +111,11 @@ struct tdesc_reg : tdesc_element
   int save_restore;
 
   /* The name of the register group containing this register, or empty
-     if the group should be automatically determined from the
-     register's type.  If this is "general", "float", or "vector", the
-     corresponding "info" command should display this register's
-     value.  It can be an arbitrary string, but should be limited to
-     alphanumeric characters and internal hyphens.  Currently other
-     strings are ignored (treated as empty).  */
+     if the group should be automatically determined from the 

register's

gdb/target-descriptions.c +3 -0

+     type.  This is traditionally "general", "float", "vector" but can
+     also be an arbitrary string.  If defined the corresponding "info"
+     command should display this register's value.  The string should 

be

gdb/target-descriptions.c +2 -11

+     limited to alphanumeric characters and internal hyphens.  */
   std::string group;
 
   /* The size of the register, in bits.  */

@@ -1279,17 +1278,13 @@ tdesc_remote_register_number (struct gdbarch
 *gdbarch, int regno)
 }
 
 /* Check whether REGNUM is a member of REGGROUP.  Registers from the
-   target description may be classified as general, float, or vector.
-   Unlike a gdbarch register_reggroup_p method, this function will
-   return -1 if it does not know; the caller should handle registers
-   with no specified group.
-
-   Arbitrary strings (other than "general", "float", and "vector")
-   from the description are not used; they cause the register to be
-   displayed in "info all-registers" but excluded from "info
-   registers" et al.  The names of containing features are also not
-   used.  This might be extended to display registers in some more
-   useful groupings.
+   target description may be classified as general, float, vector or 

other

gdb/target-descriptions.c +2 -0

+   register groups registered with reggroup_add().  Unlike a gdbarch
+   register_reggroup_p method, this function will return -1 if it does 

not

gdb/target-descriptions.c +3 -0

+   know; the caller should handle registers with no specified group.
+
+   The names of containing features are not used.  This might be 

extended

gdb/target-descriptions.c +1 -13

+   to display registers in some more useful groupings.
 
    The save-restore flag is also implemented here.  */

@@ -1299,26 +1294,9 @@ tdesc_register_in_reggroup_p (struct gdbarch
 *gdbarch, int regno,
 {
   struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno);
 
-  if (reg != NULL && !reg->group.empty ())
-    {
-      int general_p = 0, float_p = 0, vector_p = 0;
-
-      if (reg->group == "general")
-	general_p = 1;
-      else if (reg->group == "float")
-	float_p = 1;
-      else if (reg->group == "vector")
-	vector_p = 1;
-
-      if (reggroup == float_reggroup)
-	return float_p;

gdb/target-descriptions.c +1 -0

+	return 1;
 

   if (reg != NULL
       && (reggroup == save_reggroup || reggroup == restore_reggroup))

gdb/target-descriptions.c +1 -0

@@ -1421,6 +1399,28 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 	void **slot = htab_find_slot (reg_hash, reg.get (), INSERT);
 
 	*slot = reg.get ();
+	/* Add reggroup if its new.  */

Date: Wed, 20 Dec 2017 21:40:10 -0500
From: Simon Marchi <[email protected]>
Cc: Stafford Horne <[email protected]>, gdb-patches at sourceware.org,
openrisc at lists.librecores.org

Quoted 4 lines

Quoted 1 line

+ at code{maint print reggroups}.

Please add here a cross-reference to the node where "maint print reggroups" is described.

Is it ok for a non-maint command to refer to a maint command?

AFAIK, we don't expect an average user to have to use maintenance commands when using GDB. So maybe "maint print reggroups" should be promoted to a non-maint command (e.g. info register-groups)?

Quoted 17 lines · Stafford Horne

gdb/target-descriptions.c +0 -12

@@ -1299,26 +1294,9 @@ tdesc_register_in_reggroup_p (struct gdbarch
 *gdbarch, int regno,
 {
   struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno);
 
-  if (reg != NULL && !reg->group.empty ())
-    {
-      int general_p = 0, float_p = 0, vector_p = 0;
-
-      if (reg->group == "general")
-	general_p = 1;
-      else if (reg->group == "float")
-	float_p = 1;
-      else if (reg->group == "vector")
-	vector_p = 1;
-
-      if (reggroup == float_reggroup)

gdb/target-descriptions.c tdesc_register_in_reggroup_p (struct gdbarch · line 1299

Quoted 10 lines

gdb/target-descriptions.c +2 -8

-	return float_p;
-
-      if (reggroup == vector_reggroup)
-	return vector_p;
-
-      if (reggroup == general_reggroup)
-	return general_p;
-    }
+  if (reg != NULL && !reg->group.empty ()
+      && (strcmp (reg->group.c_str (), reggroup_name (reggroup)) == 0))

I suggest

reg->group == reggroup_name (reggroup)

Quoted 7 lines

gdb/target-descriptions.c +1 -0

+	return 1;
 
   if (reg != NULL

       && (reggroup == save_reggroup || reggroup == restore_reggroup))

gdb/target-descriptions.c +0 -0

@@ -1421,6 +1399,28 @@ tdesc_use_registers (struct gdbarch *gdbarch,
 	void **slot = htab_find_slot (reg_hash, reg.get (), INSERT);
 

gdb/target-descriptions.c tdesc_use_registers (struct gdbarch *gdbarch, · line 1412

Quoted 12 lines

gdb/target-descriptions.c +11 -0

 	*slot = reg.get ();
+	/* Add reggroup if its new.  */
+	if (!reg->group.empty ())
+	  {
+	    struct reggroup *group;
+	    bool group_exists = false;
+
+	    for (group = reggroup_next (gdbarch, NULL);
+		 group != NULL;
+		 group = reggroup_next (gdbarch, group))
+	      {
+		if (strcmp (reg->group.c_str (), reggroup_name (group)) == 0)

Here too.

gdb/target-descriptions.c tdesc_use_registers (struct gdbarch *gdbarch, · line 1423

Quoted 11 lines

gdb/target-descriptions.c +11 -0

+		  {
+		    group_exists = true;
+		    break;
+		  }
+	      }
+
+	    if (!group_exists)
+	      reggroup_add (gdbarch, reggroup_gdbarch_new (gdbarch,
+							   reg->group.c_str (),
+							   USER_REGGROUP));
+	  }

Could you factor this out in a separate function? It would probably be useful to have a general-purpose function

reggroup *reggroup_find (struct gdbarch *gdbarch, const char *name);

Which you could then use here

  if (reggroup_find (...) == NULL)
    {
      // Create group
    }

Quoted 3 lines · Stafford Horne

Until now this feature has existed but was not documented. Adding docs and tests.

Quoted 11 lines

gdb/testsuite/gdb.base/reggroups.exp new file +11 -0

+proc fetch_reggroups {test} {
+    global gdb_prompt
+    global expect_out
+
+    set reggroups {}
+    gdb_test_multiple "maint print reggroups" "get reggroups" {
+	-re "maint print reggroups\r\n" {
+	    exp_continue
+	}
+	-re "^ Group\[ \t\]+Type\[ \t\]+\r\n" {
+	    exp_continue

gdb/testsuite/gdb.base/reggroups.exp line 47

Quoted 6 lines

gdb/testsuite/gdb.base/reggroups.exp new file +6 -0

+	}
+	-re "^ (\[0-9a-zA-Z\-\]+)\[ \t\]+(user|internal)\[ \t\]+\r\n" {
+	    lappend reggroups $expect_out(1,string)
+	    exp_continue
+	}
+	-re ".*$gdb_prompt $" {

The .* should not be necessary here.

Quoted 6 lines

gdb/testsuite/gdb.base/reggroups.exp new file +6 -0

+	    if { [llength $reggroups] != 0 } {
+		pass $test
+	    } else {
+		fail "$test - didn't fetch any reggroups"
+	    }
+	}

gdb/testsuite/gdb.base/reggroups.exp line 63

Quoted 10 lines

gdb/testsuite/gdb.base/reggroups.exp new file +10 -0

+    }
+
+    return $reggroups
+}
+
+set reggroups [fetch_reggroups "fetch reggroups"]
+
+foreach reggroup $reggroups {
+    gdb_test "info reg $reggroup" ".*" "info reg $reggroup"
+}

This doesn't really test anything. If you change the line to

gdb_test "info reg hello$reggroup" ".*" "info reg $reggroup"

to fake that the command doesn't work, the test still passes. So if something breaks the feature of "info registers" handling reg groups, the test won't catch it. But I understand the problem, the output is not really predictable. I thought about matching at least one $hex number, but it's not even guaranteed (some groups like mmx output nothing).

What we could do (I'm open to better suggestions) is at least validate that it doesn't output "Invalid register", which is the message given when passing a register that doesn't exist. But then, if that message changes one day for some reason, the test will become moot again (because GDB will output something else than "Invalid register" if the functionality breaks, but the test won't catch it). So in addition, we could also validate that "info registers a_non_existent_register" does output "Invalid register". This way, if some GDB developers of the future change the message, the test will fail, they will go look at your test file, read the comment that you will have left, and update the test accordingly.