[RFC] Info registers command produces no output for "standard register names"

[RFC] Info registers command produces no output for "standard register names"

[Fred Fish]

The "info register sp" command prints no output of any kind, not even
a message that this is not supported.  Same thing for the other "user
registers" (fp, pc, ps).

For example, run any executable to a breakpoint and try "info register sp".

 (gdb) br main
 Breakpoint 1 at 0x8048350: file t.c, line 3.
 (gdb) run
 Starting program: /links/build/sourceware/gdb/i686-pc-linux-gnu/gdb/t

 Breakpoint 1, main () at t.c:3
 3       }
 (gdb) info register sp
 (gdb) info register esp
 esp            0xbffff5d0       0xbffff5d0
 (gdb)

The attached patch fixes the problem, but may or may not be the best
way to deal with the issue.  Perhaps gdb should simply say that info
registers only supports printing using the canonical mnemonics for
registers (to use the terms in the gdb documentation).

This is reported as gdb bug #1926:

  http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=1926

-Fred

============================================================================

	2005-05-27  Fred Fish  <fnf@specifixinc.com>
	* user-regs.h (default_print_user_registers_info): Declare.
	* user-regs.c: Include value.h and regcache.h.
	(default_print_user_registers_info): New function.
	* infcmd.c: Include user-regs.h.
	(default_print_registers_info): Handle user registers by
	calling default_print_user_registers_info.


Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.136
diff -c -p -r1.136 infcmd.c
*** infcmd.c	8 Mar 2005 22:17:34 -0000	1.136
--- infcmd.c	25 Apr 2005 14:25:39 -0000
***************
*** 46,51 ****
--- 46,52 ----
  #include "block.h"
  #include <ctype.h>
  #include "gdb_assert.h"
+ #include "user-regs.h"
  
  /* Functions exported for general use, in inferior.h: */
  
*************** default_print_registers_info (struct gdb
*** 1525,1530 ****
--- 1526,1538 ----
    const int numregs = NUM_REGS + NUM_PSEUDO_REGS;
    char buffer[MAX_REGISTER_SIZE];
  
+   /* If this is a user register, try to print it */
+   if (regnum >= numregs)
+     {
+       default_print_user_registers_info (gdbarch, file, frame, regnum);
+       return;
+     }
+ 
    for (i = 0; i < numregs; i++)
      {
        /* Decide between printing all regs, non-float / vector regs, or
Index: user-regs.c
===================================================================
RCS file: /cvs/src/src/gdb/user-regs.c,v
retrieving revision 1.5
diff -c -p -r1.5 user-regs.c
*** user-regs.c	15 Mar 2004 20:38:08 -0000	1.5
--- user-regs.c	25 Apr 2005 14:25:39 -0000
***************
*** 27,32 ****
--- 27,34 ----
  #include "gdb_string.h"
  #include "gdb_assert.h"
  #include "frame.h"
+ #include "value.h"
+ #include "regcache.h"
  
  /* A table of user registers.
  
*************** value_of_user_reg (int regnum, struct fr
*** 202,207 ****
--- 204,251 ----
    return reg->read (frame);
  }
  
+ /* Print user register in same format as "info registers" so natural
+    things like "info register sp" works as expected.
+ 
+    FIXME: Assumes size and type of user reg is same as size of first
+    architecture register.
+ 
+    FIXME: Needs to track any changes in default_print_registers_info
+    formatting. */
+ 
+ void
+ default_print_user_registers_info (struct gdbarch *gdbarch,
+ 				   struct ui_file *file,
+ 				   struct frame_info *frame,
+ 				   int regnum)
+ {
+   const char *regname;
+   char raw_buffer[MAX_REGISTER_SIZE];
+   struct value *regval;
+   struct type *regtype;
+   int regsize;
+ 
+   regname = user_reg_map_regnum_to_name (gdbarch, regnum);
+   fputs_filtered (regname, file);
+   print_spaces_filtered (15 - strlen (regname), file);
+ 
+   regval = value_of_user_reg (regnum, frame);
+   if (! regval)
+     {
+       fprintf_filtered (file, "*value not available*\n");
+     }
+   else
+     {
+       regtype = register_type (gdbarch, 0);	/* FIXME */
+       regsize = register_size (gdbarch, 0);	/* FIXME */
+       memcpy (raw_buffer, value_contents_raw (regval), regsize);
+       val_print (regtype, raw_buffer, 0, 0, file, 'x', 1, 0, Val_pretty_default);
+       fprintf_filtered (file, "\t");
+       val_print (regtype, raw_buffer, 0, 0, file, 0, 1, 0, Val_pretty_default);
+       fprintf_filtered (file, "\n");
+     }
+ }
+ 
  extern initialize_file_ftype _initialize_user_regs; /* -Wmissing-prototypes */
  
  void
Index: user-regs.h
===================================================================
RCS file: /cvs/src/src/gdb/user-regs.h,v
retrieving revision 1.2
diff -c -p -r1.2 user-regs.h
*** user-regs.h	18 Jul 2003 19:01:14 -0000	1.2
--- user-regs.h	25 Apr 2005 14:25:39 -0000
***************
*** 37,43 ****
  /* TODO: cagney/2003-06-27: Need to think more about how these
     registers are added, read, and modified.  At present they are kind
     of assumed to be read-only.  Should it, for instance, return a
!    register descriptor that contains all the relvent access methods.  */
  
  struct frame_info;
  struct gdbarch;
--- 37,43 ----
  /* TODO: cagney/2003-06-27: Need to think more about how these
     registers are added, read, and modified.  At present they are kind
     of assumed to be read-only.  Should it, for instance, return a
!    register descriptor that contains all the relevent access methods.  */
  
  struct frame_info;
  struct gdbarch;
*************** extern const char *user_reg_map_regnum_t
*** 55,65 ****
  
     Note; These methods return a "struct value" instead of the raw
     bytes as, at the time the register is being added, the type needed
!    to describe the register has not bee initialized.  */
  
  typedef struct value *(user_reg_read_ftype) (struct frame_info *frame);
  extern struct value *value_of_user_reg (int regnum, struct frame_info *frame);
  
  /* Add a builtin register (present in all architectures).  */
  extern void user_reg_add_builtin (const char *name,
  				  user_reg_read_ftype *read);
--- 55,71 ----
  
     Note; These methods return a "struct value" instead of the raw
     bytes as, at the time the register is being added, the type needed
!    to describe the register has not been initialized.  */
  
  typedef struct value *(user_reg_read_ftype) (struct frame_info *frame);
  extern struct value *value_of_user_reg (int regnum, struct frame_info *frame);
  
+ /* Print user register in same format as "info registers". */
+ extern void default_print_user_registers_info (struct gdbarch *gdbarch,
+ 					       struct ui_file *file,
+ 					       struct frame_info *frame,
+ 					       int regnum);
+ 
  /* Add a builtin register (present in all architectures).  */
  extern void user_reg_add_builtin (const char *name,
  				  user_reg_read_ftype *read);

Re: [RFC] Info registers command produces no output for "standard register names"

[Daniel Jacobowitz]

On Fri, May 27, 2005 at 05:37:58PM -0400, Fred Fish wrote:
> The "info register sp" command prints no output of any kind, not even
> a message that this is not supported.  Same thing for the other "user
> registers" (fp, pc, ps).
> 
> For example, run any executable to a breakpoint and try "info register sp".
> 
>  (gdb) br main
>  Breakpoint 1 at 0x8048350: file t.c, line 3.
>  (gdb) run
>  Starting program: /links/build/sourceware/gdb/i686-pc-linux-gnu/gdb/t
> 
>  Breakpoint 1, main () at t.c:3
>  3       }
>  (gdb) info register sp
>  (gdb) info register esp
>  esp            0xbffff5d0       0xbffff5d0
>  (gdb)
> 
> The attached patch fixes the problem, but may or may not be the best
> way to deal with the issue.  Perhaps gdb should simply say that info
> registers only supports printing using the canonical mnemonics for
> registers (to use the terms in the gdb documentation).

Well, I agree that we shouldn't leave it the way it is.  The final
result should be more-or-less consistent across targets; it looks like
every target with its own print_registers_info method will already do
this.  I only tested MIPS.

Your patch is overcomplicated for the problem.  The value of a user
register _can_ be read by value_of_user_reg, but it can also be read
just like any other register.  The only reason there's a problem is
that we iterate in [0, NUM_REGS + NUM_PSEUDO_REGS) and the user
registers are numbered outside that range.  Hmm, and we need a
different hook to get the register name.

How do you feel about this patch instead?  It brings the default
implementation in line with the few specializations; if you ask fo the
value of a user register, that's what you get.

While proofreading I noticed that some targets will issue an internal
error if you try to do this - for example SH.  There needs to be a test
case for this; I'm not going to commit the patch until I've done that.

However, the patch has raised some questions for me about register
names... the fact that there are acceptable REGNUM values that can't be
passed to gdbarch_register_name(), or most other gdbarch methods, is
really quite ugly.  I need to spend some more time thinking about this.
Also, there should probably be a register_name() function encapsulating
the current uses of user_reg_map_regnum_to_name.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-05-28  Daniel Jacobowitz  <dan@codesourcery.com>

	gdb/1926
	* Makefile.in (infcmd.o): Update dependencies.
	* infcmd.c: Include "user-regs.h".
	(default_print_register): New function, based on
	default_print_registers_info.  Use user_reg_map_regnum_to_name
	and common_val_print.
	(default_print_registers_info): Use it.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.733
diff -u -p -r1.733 Makefile.in
--- Makefile.in	22 May 2005 20:36:18 -0000	1.733
+++ Makefile.in	28 May 2005 18:03:08 -0000
@@ -2113,7 +2113,7 @@ infcmd.o: infcmd.c $(defs_h) $(gdb_strin
 	$(symfile_h) $(gdbcore_h) $(target_h) $(language_h) $(symfile_h) \
 	$(objfiles_h) $(completer_h) $(ui_out_h) $(event_top_h) \
 	$(parser_defs_h) $(regcache_h) $(reggroups_h) $(block_h) \
-	$(solib_h) $(gdb_assert_h)
+	$(solib_h) $(user_regs_h) $(gdb_assert_h)
 inf-loop.o: inf-loop.c $(defs_h) $(inferior_h) $(target_h) $(event_loop_h) \
 	$(event_top_h) $(inf_loop_h) $(remote_h) $(exceptions_h)
 inflow.o: inflow.c $(defs_h) $(frame_h) $(inferior_h) $(command_h) \
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.138
diff -u -p -r1.138 infcmd.c
--- infcmd.c	22 May 2005 14:53:34 -0000	1.138
+++ infcmd.c	28 May 2005 18:03:09 -0000
@@ -45,6 +45,8 @@
 #include "reggroups.h"
 #include "block.h"
 #include "solib.h"
+#include "user-regs.h"
+
 #include <ctype.h>
 #include "gdb_assert.h"
 
@@ -1505,9 +1507,77 @@ path_command (char *dirname, int from_tt
 }
 \f
 
-/* Print out the machine register regnum. If regnum is -1, print all
-   registers (print_all == 1) or all non-float and non-vector
-   registers (print_all == 0).
+/* Print the value of machine register REGNUM from FRAME to FILE, in the
+   default format.  REGNUM must be a valid register number.  */
+
+void
+default_print_register (struct gdbarch *gdbarch, struct ui_file *file,
+			struct frame_info *frame, int regnum)
+{
+  struct value *regval;
+  const char *name;
+
+  name = user_reg_map_regnum_to_name (gdbarch, regnum);
+
+  /* If the register name is empty, it is undefined for this processor, so
+     don't display anything.  */
+  if (name == NULL)
+    return;
+
+  fputs_filtered (name, file);
+  print_spaces_filtered (15 - strlen (name), file);
+
+  /* Read the value.  */
+  regval = value_of_register (regnum, frame);
+  if (regval == NULL)
+    {
+      fprintf_filtered (file, "*value not available*\n");
+      return;
+    }
+
+  /* If virtual format is floating, print it that way, and in raw
+     hex.  */
+  if (TYPE_CODE (value_type (regval)) == TYPE_CODE_FLT)
+    {
+      int j, len;
+      const gdb_byte *buffer;
+
+      common_val_print (regval, file, 0, 1, 0, Val_pretty_default);
+
+      fprintf_filtered (file, "\t(raw 0x");
+      len = TYPE_LENGTH (value_type (regval));
+      buffer = value_contents (regval);
+      for (j = 0; j < len; j++)
+	{
+	  int idx;
+	  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
+	    idx = j;
+	  else
+	    idx = len - 1 - j;
+	  fprintf_filtered (file, "%02x", (unsigned char) buffer[idx]);
+	}
+      fprintf_filtered (file, ")");
+    }
+  else
+    {
+      /* Print the register in hex.  */
+      common_val_print (regval, file, 'x', 1, 0, Val_pretty_default);
+
+      /* If not a vector register, print it also according to its
+	 natural format.  */
+      if (TYPE_VECTOR (value_type (regval)) == 0)
+	{
+	  fprintf_filtered (file, "\t");
+	  common_val_print (regval, file, 0, 1, 0, Val_pretty_default);
+	}
+    }
+
+  fprintf_filtered (file, "\n");
+}
+
+/* Print out the machine register REGNUM.  If regnum is -1, print all
+   registers (PRINT_ALL == 1) or all non-float and non-vector
+   registers (PRINT_ALL == 0).
 
    For most machines, having all_registers_info() print the
    register(s) one per line is good enough.  If a different format is
@@ -1524,83 +1594,22 @@ default_print_registers_info (struct gdb
 {
   int i;
   const int numregs = NUM_REGS + NUM_PSEUDO_REGS;
-  gdb_byte buffer[MAX_REGISTER_SIZE];
 
-  for (i = 0; i < numregs; i++)
+  if (regnum >= 0)
     {
-      /* Decide between printing all regs, non-float / vector regs, or
-         specific reg.  */
-      if (regnum == -1)
-	{
-	  if (print_all)
-	    {
-	      if (!gdbarch_register_reggroup_p (gdbarch, i, all_reggroup))
-		continue;
-	    }
-	  else
-	    {
-	      if (!gdbarch_register_reggroup_p (gdbarch, i, general_reggroup))
-		continue;
-	    }
-	}
-      else
-	{
-	  if (i != regnum)
-	    continue;
-	}
-
-      /* If the register name is empty, it is undefined for this
-         processor, so don't display anything.  */
-      if (REGISTER_NAME (i) == NULL || *(REGISTER_NAME (i)) == '\0')
-	continue;
-
-      fputs_filtered (REGISTER_NAME (i), file);
-      print_spaces_filtered (15 - strlen (REGISTER_NAME (i)), file);
-
-      /* Get the data in raw format.  */
-      if (! frame_register_read (frame, i, buffer))
-	{
-	  fprintf_filtered (file, "*value not available*\n");
-	  continue;
-	}
-
-      /* If virtual format is floating, print it that way, and in raw
-         hex.  */
-      if (TYPE_CODE (register_type (current_gdbarch, i)) == TYPE_CODE_FLT)
-	{
-	  int j;
-
-	  val_print (register_type (current_gdbarch, i), buffer, 0, 0,
-		     file, 0, 1, 0, Val_pretty_default);
-
-	  fprintf_filtered (file, "\t(raw 0x");
-	  for (j = 0; j < register_size (current_gdbarch, i); j++)
-	    {
-	      int idx;
-	      if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
-		idx = j;
-	      else
-		idx = register_size (current_gdbarch, i) - 1 - j;
-	      fprintf_filtered (file, "%02x", (unsigned char) buffer[idx]);
-	    }
-	  fprintf_filtered (file, ")");
-	}
-      else
-	{
-	  /* Print the register in hex.  */
-	  val_print (register_type (current_gdbarch, i), buffer, 0, 0,
-		     file, 'x', 1, 0, Val_pretty_default);
-          /* If not a vector register, print it also according to its
-             natural format.  */
-	  if (TYPE_VECTOR (register_type (current_gdbarch, i)) == 0)
-	    {
-	      fprintf_filtered (file, "\t");
-	      val_print (register_type (current_gdbarch, i), buffer, 0, 0,
-			 file, 0, 1, 0, Val_pretty_default);
-	    }
-	}
+      default_print_register (gdbarch, file, frame, regnum);
+      return;
+    }
 
-      fprintf_filtered (file, "\n");
+  for (i = 0; i < numregs; i++)
+    {
+      /* Decide between printing all regs or non-float / vector regs.  */
+      if (print_all
+	  && gdbarch_register_reggroup_p (gdbarch, i, all_reggroup))
+	default_print_register (gdbarch, file, frame, i);
+      else if (!print_all
+	       && gdbarch_register_reggroup_p (gdbarch, i, general_reggroup))
+	default_print_register (gdbarch, file, frame, i);
     }
 }
 

Re: [RFC] Info registers command produces no output for "standard register names"

[Eli Zaretskii]

> Date: Sat, 28 May 2005 14:06:57 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sources.redhat.com
> 
> +      fprintf_filtered (file, "\t(raw 0x");
> +      len = TYPE_LENGTH (value_type (regval));
> +      buffer = value_contents (regval);
> +      for (j = 0; j < len; j++)
> +	{
> +	  int idx;
> +	  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
> +	    idx = j;
> +	  else
> +	    idx = len - 1 - j;
> +	  fprintf_filtered (file, "%02x", (unsigned char) buffer[idx]);
> +	}
> +      fprintf_filtered (file, ")");

Daniel, this kind of output generation loop is bad for i18n.  So how
about if we produce the entire numerical string in memory, then print
it all, as a string, in one go, together with its decorations?  (Yes,
I know you just copied the old code, but as long as we are
changing...)

Re: [RFC] Info registers command produces no output for "standard register names"

[Daniel Jacobowitz]

On Sun, May 29, 2005 at 08:53:50AM +0300, Eli Zaretskii wrote:
> > Date: Sat, 28 May 2005 14:06:57 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: gdb-patches@sources.redhat.com
> > 
> > +      fprintf_filtered (file, "\t(raw 0x");
> > +      len = TYPE_LENGTH (value_type (regval));
> > +      buffer = value_contents (regval);
> > +      for (j = 0; j < len; j++)
> > +	{
> > +	  int idx;
> > +	  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
> > +	    idx = j;
> > +	  else
> > +	    idx = len - 1 - j;
> > +	  fprintf_filtered (file, "%02x", (unsigned char) buffer[idx]);
> > +	}
> > +      fprintf_filtered (file, ")");
> 
> Daniel, this kind of output generation loop is bad for i18n.  So how
> about if we produce the entire numerical string in memory, then print
> it all, as a string, in one go, together with its decorations?  (Yes,
> I know you just copied the old code, but as long as we are
> changing...)

Good point.  In this case I don't think it makes much difference, but
it's definitely a good habit to get into; I'll fix that if the patch
goes in.

-- 
Daniel Jacobowitz
CodeSourcery, LLC