Re: [PATCH] h8300 "info registers" broken

[Pedro Alves <palves@redhat.com>]

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.

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.

Also, this would need a ChangeLog entry.

> 
> 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