Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato <ysato at users dot sourceforge dot jp>]

At Tue, 04 Feb 2014 19:37:40 +0000,
Pedro Alves wrote:
> 
> On 02/01/2014 12:43 PM, Yoshinori Sato wrote:
> > Following result in h8300-elf-gdb.
> > 
> >  (gdb) info registers ccr
> >  memory clobbered past end of allocated block
> > 
> > This cause of missing register size.
> 
> I'm guessing the patch makes the raw ccr register visible in
> info registers, because unnamed registers are hidden, and you're
> now naming it.
> 
> infcmd.c:default_print_registers_info:
> 
>       /* If the register name is empty, it is undefined for this
>          processor, so don't display anything.  */
>       if (gdbarch_register_name (gdbarch, i) == NULL
> 	  || *(gdbarch_register_name (gdbarch, i)) == '\0')
> 	continue;
> 
> 
> I don't know much about the h8300 port, but the fact that
> there's a pseudo ccr register makes me thing this patch
> isn't correct.  E.g.,
> 
> static void
> h8300_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file,
> 			    struct frame_info *frame, int regno, int cpregs)
> {
> ...
>   if (regno < 0)
>     {
>       for (regno = E_R0_REGNUM; regno <= E_SP_REGNUM; ++regno)
> 	h8300_print_register (gdbarch, file, frame, regno);
>       h8300_print_register (gdbarch, file, frame,
> 			    E_PSEUDO_CCR_REGNUM (gdbarch));
> 
> The loop prints all raw registers, and skips the raw ccr,
> and then we print the pseudo ccr.  With your patch, I think
> we'll print the raw ccr register too.

No. Don't show ccr value.
This case undefined raw ccr.
 
> Can you explain the patch to me a little bit more, please?
> It's not obvious to me at all what the register names
> have to with the proposed change.

Current h8300-tdep have two problems.
1. Size mismatch in pseudo register vs raw register.
   So overflow on register cache.
2. Undisplay pseudo regsters.

I will fix it.

> Also, this would need a ChangeLog entry.

OK. update patch.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fcec355..7499f78 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2014-02-06  Yoshinori Sato <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_register_name): Assign nsme for raw ccr.
+	(h8300s_register_name): Ditto.
+	(h8300sx_register_name): Ditto.
+	(h8300_register_type): Fix raw ccr and exr size.
+
 2014-02-05  Mark Kettenis  <kettenis@gnu.org>
 
 	* c-exp.y (YYPRINT, c_print_token): Only define if YYBISON is
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index ffffbc9..621cfbc 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -946,7 +946,7 @@ h8300_register_name (struct gdbarch *gdbarch, int regno)
      type is selected.  */
   static char *register_names[] = {
     "r0", "r1", "r2", "r3", "r4", "r5", "r6",
-    "sp", "", "pc", "cycles", "tick", "inst",
+    "sp", "raw_ccr", "pc", "cycles", "tick", "inst",
     "ccr",			/* pseudo register */
   };
   if (regno < 0
@@ -963,7 +963,7 @@ h8300s_register_name (struct gdbarch *gdbarch, int regno)
 {
   static char *register_names[] = {
     "er0", "er1", "er2", "er3", "er4", "er5", "er6",
-    "sp", "", "pc", "cycles", "", "tick", "inst",
+    "sp", "raw_ccr", "pc", "cycles", "raw_exr", "tick", "inst",
     "mach", "macl",
     "ccr", "exr"		/* pseudo registers */
   };
@@ -981,7 +981,7 @@ h8300sx_register_name (struct gdbarch *gdbarch, int regno)
 {
   static char *register_names[] = {
     "er0", "er1", "er2", "er3", "er4", "er5", "er6",
-    "sp", "", "pc", "cycles", "", "tick", "inst",
+    "sp", "raw_ccr", "pc", "cycles", "raw_exr", "tick", "inst",
     "mach", "macl", "sbr", "vbr",
     "ccr", "exr"		/* pseudo registers */
   };
@@ -1136,9 +1136,9 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
 	case E_FP_REGNUM:
 	  return builtin_type (gdbarch)->builtin_data_ptr;
 	default:
-	  if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
+	  if (regno == E_PSEUDO_CCR_REGNUM (gdbarch) || regno == E_CCR_REGNUM)
 	    return builtin_type (gdbarch)->builtin_uint8;
-	  else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
+	  else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch)  || regno == E_EXR_REGNUM)
 	    return builtin_type (gdbarch)->builtin_uint8;
 	  else if (is_h8300hmode (gdbarch))
 	    return builtin_type (gdbarch)->builtin_int32;

> > 
> > diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
> > index ffffbc9..a21f7de 100644
> > --- a/gdb/h8300-tdep.c
> > +++ b/gdb/h8300-tdep.c
> > @@ -946,7 +946,7 @@ h8300_register_name (struct gdbarch *gdbarch, int regno)
> >       type is selected.  */
> >    static char *register_names[] = {
> >      "r0", "r1", "r2", "r3", "r4", "r5", "r6",
> > -    "sp", "", "pc", "cycles", "tick", "inst",
> > +    "sp", "ccr", "pc", "cycles", "tick", "inst",
> >      "ccr",			/* pseudo register */
> >    };
> >    if (regno < 0
> > @@ -963,7 +963,7 @@ h8300s_register_name (struct gdbarch *gdbarch, int regno)
> >  {
> >    static char *register_names[] = {
> >      "er0", "er1", "er2", "er3", "er4", "er5", "er6",
> > -    "sp", "", "pc", "cycles", "", "tick", "inst",
> > +    "sp", "ccr", "pc", "cycles", "exr", "tick", "inst",
> >      "mach", "macl",
> >      "ccr", "exr"		/* pseudo registers */
> >    };
> > @@ -981,7 +981,7 @@ h8300sx_register_name (struct gdbarch *gdbarch, int regno)
> >  {
> >    static char *register_names[] = {
> >      "er0", "er1", "er2", "er3", "er4", "er5", "er6",
> > -    "sp", "", "pc", "cycles", "", "tick", "inst",
> > +    "sp", "ccr", "pc", "cycles", "exr", "tick", "inst",
> >      "mach", "macl", "sbr", "vbr",
> >      "ccr", "exr"		/* pseudo registers */
> >    };
> > @@ -1136,9 +1136,9 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
> >  	case E_FP_REGNUM:
> >  	  return builtin_type (gdbarch)->builtin_data_ptr;
> >  	default:
> > -	  if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
> > +	  if (regno == E_PSEUDO_CCR_REGNUM (gdbarch) || regno == E_CCR_REGNUM)
> >  	    return builtin_type (gdbarch)->builtin_uint8;
> > -	  else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
> > +	  else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch) || regno == E_EXR_REGNUM)
> >  	    return builtin_type (gdbarch)->builtin_uint8;
> >  	  else if (is_h8300hmode (gdbarch))
> >  	    return builtin_type (gdbarch)->builtin_int32;
> > 
> 
> 
> -- 
> Pedro Alves

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>