[PATCH] h8300 "info registers" broken

[PATCH] h8300 "info registers" broken

[Yoshinori Sato]

Following result in h8300-elf-gdb.

 (gdb) info registers ccr
 memory clobbered past end of allocated block

This cause of missing register size.

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;

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

Re: [PATCH] h8300 "info registers" broken

[Pedro Alves]

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

Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato]

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>

Re: [PATCH] h8300 "info registers" broken

[Mark Kettenis]

> Date: Thu, 06 Feb 2014 02:51:11 +0900
> From: Yoshinori Sato <ysato@users.sourceforge.jp>
> > 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.

That doesn't make sense.  Pseudo registers don't live in the register
cache.  Instead they are synthesised from the contents of the register
cache.

> 2. Undisplay pseudo regsters.

Not sure what you're trying to say here.  But if you don't want to
show a pseudo register, why do you have it in the first place?

Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato]

At Wed, 5 Feb 2014 18:59:00 +0100 (CET),
Mark Kettenis wrote:
> 
> > Date: Thu, 06 Feb 2014 02:51:11 +0900
> > From: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > 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.
> 
> That doesn't make sense.  Pseudo registers don't live in the register
> cache.  Instead they are synthesised from the contents of the register
> cache.
> 
> > 2. Undisplay pseudo regsters.
> 
> Not sure what you're trying to say here.  But if you don't want to
> show a pseudo register, why do you have it in the first place?

OK.
I trying latest version.
I got result bellow.

ysato@sa76r4:~/dev/binutils-gdb/h8300$ LANG=C gdb gdb/gdb
GNU gdb (GDB) 7.6.2 (Debian 7.6.2-1)
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/ysato/dev/binutils-gdb/h8300/gdb/gdb...done.
warning: File "/home/ysato/dev/binutils-gdb/h8300/gdb/gdb-gdb.gdb" auto-loading has been declined by your `auto-load safe-path' set to "$debugdir:$datadir/auto-load".
To enable execution of this file add
	add-auto-load-safe-path /home/ysato/dev/binutils-gdb/h8300/gdb/gdb-gdb.gdb
line to your configuration file "/home/ysato/.gdbinit".
To completely disable this security protection add
	set auto-load safe-path /
line to your configuration file "/home/ysato/.gdbinit".
For more information about this security protection see the
"Auto-loading safe path" section in the GDB manual.  E.g., run from the shell:
	info "(gdb)Auto-loading safe path"
(gdb) r test
Starting program: /home/ysato/dev/binutils-gdb/h8300/gdb/gdb test
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
GNU gdb (GDB) 7.7.50.20140208-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=x86_64-unknown-linux-gnu --target=h8300-elf".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from test...(no debugging symbols found)...done.
(gdb) target sim
Connected to the simulator.
(gdb) load
Loading section .text, size 0x4 vma 0x100
Start address 0x100
Transfer rate: 32 bits in <1 sec.
(gdb) b *0x100
Breakpoint 1 at 0x100
(gdb) r
Starting program: /home/ysato/dev/binutils-gdb/h8300/test 

Breakpoint 1, 0x00000100 in start ()
(gdb) si
0x00000102 in start ()
(gdb) 
0x00000102 in start ()
(gdb) info registers 
r0             0x00000000  0
r1             0x00000000  0
r2             0x00000000  0
r3             0x00000000  0
r4             0x00000000  0
r5             0x00000000  0
r6             0x00000000  0
sp             0x00000000  0
memory clobbered past end of allocated block

Program received signal SIGABRT, Aborted.
0x00007ffff6aaf1d5 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff6aaf1d5 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff6ab2388 in __GI_abort () at abort.c:90
#2  0x00007ffff6aea7bb in __libc_message (do_abort=do_abort@entry=1, 
    fmt=fmt@entry=0x7ffff6be4bee "%s")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:199
#3  0x00007ffff6aea89e in __GI___libc_fatal (
    message=0x7ffff6be8328 "memory clobbered past end of allocated block\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:210
#4  0x00007ffff6afaca5 in mabort (status=<optimized out>) at mcheck.c:364
#5  0x00007ffff6afad6b in checkhdr (hdr=<optimized out>) at mcheck.c:115
#6  checkhdr (hdr=<optimized out>) at mcheck.c:86
#7  0x00007ffff6afb0f5 in freehook (ptr=0xd90040, 
    caller=0x49fedc <value_free+60>) at mcheck.c:188
#8  0x000000000049fedc in value_free (val=0xd90440) at ../../gdb/value.c:1437
#9  0x00000000005b919f in frame_register_unwind (frame=<optimized out>, 
    regnum=regnum@entry=13, optimizedp=optimizedp@entry=0x7fffffffe088, 
    unavailablep=0x7fff00000003, unavailablep@entry=0x7fffffffe08c, 
    lvalp=lvalp@entry=0x7fffffffe094, addrp=addrp@entry=0x7fffffffe098, 
    realnump=realnump@entry=0x7fffffffe090, bufferp=0x7fffffffe0b0 "")
    at ../../gdb/frame.c:1032
#10 0x00000000005b954e in frame_unwind_register (frame=<optimized out>, 
    regnum=13, buf=<optimized out>) at ../../gdb/frame.c:1064
---Type <return> to continue, or q <return> to quit--- 
#11 0x00000000005b960c in frame_unwind_register_signed (frame=0xd453f0, 
    regnum=regnum@entry=13) at ../../gdb/frame.c:1162
#12 0x00000000005b963c in get_frame_register_signed (
    frame=frame@entry=0xd454b0, regnum=regnum@entry=13)
    at ../../gdb/frame.c:1169
#13 0x0000000000408c9b in h8300_print_register (
    gdbarch=gdbarch@entry=0xd768c0, file=file@entry=0xd66eb0, 
    frame=frame@entry=0xd454b0, regno=13) at ../../gdb/h8300-tdep.c:1007
#14 0x00000000004090d8 in h8300_print_registers_info (gdbarch=0xd768c0, 
    file=0xd66eb0, frame=0xd454b0, regno=8, cpregs=<optimized out>)
    at ../../gdb/h8300-tdep.c:1082
#15 0x00000000005af307 in execute_command (p=<optimized out>, 
    p@entry=0xcbe3a0 "info registers ", from_tty=1) at ../../gdb/top.c:458
#16 0x00000000004faf11 in command_handler (command=0xcbe3a0 "info registers ")
    at ../../gdb/event-top.c:435
#17 0x00000000004fb37c in command_line_handler (rl=<optimized out>)
    at ../../gdb/event-top.c:632
#18 0x0000000000615980 in rl_callback_read_char ()
    at ../../readline/callback.c:220
#19 0x00000000004faf79 in rl_callback_read_char_wrapper (
    client_data=<optimized out>) at ../../gdb/event-top.c:164
#20 0x00000000004f9c43 in process_event () at ../../gdb/event-loop.c:342
#21 0x00000000004f9f97 in gdb_do_one_event () at ../../gdb/event-loop.c:406
---Type <return> to continue, or q <return> to quit---
#22 0x00000000004fa1b7 in start_event_loop () at ../../gdb/event-loop.c:431
#23 0x00000000004f3b23 in captured_command_loop (data=data@entry=0x0)
    at ../../gdb/main.c:266
#24 0x00000000004f134a in catch_errors (
    func=func@entry=0x4f3b10 <captured_command_loop>, 
    func_args=func_args@entry=0x0, errstring=errstring@entry=0x68cdb5 "", 
    mask=mask@entry=RETURN_MASK_ALL) at ../../gdb/exceptions.c:524
#25 0x00000000004f4826 in captured_main (data=data@entry=0x7fffffffe490)
    at ../../gdb/main.c:1054
#26 0x00000000004f134a in catch_errors (
    func=func@entry=0x4f3e00 <captured_main>, 
    func_args=func_args@entry=0x7fffffffe490, 
    errstring=errstring@entry=0x68cdb5 "", mask=mask@entry=RETURN_MASK_ALL)
    at ../../gdb/exceptions.c:524
#27 0x00000000004f4cfb in gdb_main (args=args@entry=0x7fffffffe490)
    at ../../gdb/main.c:1062
#28 0x0000000000407815 in main (argc=<optimized out>, argv=<optimized out>)
    at ../../gdb/gdb.c:33
(gdb) 

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

Re: [PATCH] h8300 "info registers" broken

[Pedro Alves]

On 02/08/2014 06:36 PM, Yoshinori Sato wrote:


> #10 0x00000000005b954e in frame_unwind_register (frame=<optimized out>, 
>     regnum=13, buf=<optimized out>) at ../../gdb/frame.c:1064

Hard to reason about an optimized build...  Please try with "-g3 -O0".

I don't have a h8300-elf toolchain handy, and the h8300-linux
toolchain I found doesn't seem to want to link executables,
but I managed to try something by building an .o file, and debugging
that.  I don't see a crash, but instead GDB complains CCR
is unavailable.

(gdb) info target
Symbols from "/home/pedro/h8300-main.o".
simulator:
        Attached to sim running program /home/pedro/h8300-main.o

#instructions executed           0
#cycles (v approximate)          0
#real time taken            0.0000
#virtual time taken         0.0000
#compiles                        0
#cache size                   1024
        While running this, GDB does not access memory from...
Local exec file:
        `/home/pedro/h8300-main.o', file type elf32-h8300.
        Entry point: 0x0
        0x00000000 - 0x00000024 is .text
        0x00000024 - 0x00000024 is .data
        0x00000024 - 0x00000024 is .bss
(gdb) b *0
Breakpoint 1 at 0x0: file main.c, line 4.
(gdb) r
Starting program: /home/pedro/h8300-main.o

Breakpoint 1, foo (i=0x0 <foo>) at main.c:4
4       {
(gdb) info registers
r0             0x0000  0
r1             0x0000  0
r2             0x0000  0
r3             0x0000  0
r4             0x0000  0
r5             0x0000  0
r6             0x0000  0
sp             0x0000  0
Register 13 is not available
(gdb) info registers ccr
Register 13 is not available

The problem seems to me that the h8300 port does not define
a register_sim_regno gdbarch hook, and thus when fetching
registers off of the sim, we end up in legacy_register_sim_regno
trying to figure out the sim register number for the raw CCR register:

int
legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
{
  /* Only makes sense to supply raw registers.  */
  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
  /* NOTE: cagney/2002-05-13: The old code did it this way and it is
     suspected that some GDB/SIM combinations may rely on this
     behavour.  The default should be one2one_register_sim_regno
     (below).  */
  if (gdbarch_register_name (gdbarch, regnum) != NULL
      && gdbarch_register_name (gdbarch, regnum)[0] != '\0')
    return regnum;
  else
    return LEGACY_SIM_REGNO_IGNORE;
}

And because the raw ccr register does not have a name, that returns
LEGACY_SIM_REGNO_IGNORE.  Which means that we never actually
read the ccr raw value.  Before the <unavailable> support, this
must have meant that ccr was _always_ read as 0...  At least, I'm
not seeing how this ever worked.

Looking at sim/h8300/sim-main.h, it seems like the sim's register
numbers are compatible with gdb's.

This patch below "works" for me, as in, I can now print CCR,
but that's about all I tested (and am willing to test) myself.

Look me know how this looks to you.

---
 gdb/h8300-tdep.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index ffffbc9..ac34a9b 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -939,6 +939,20 @@ h8300h_return_value (struct gdbarch *gdbarch, struct value *function,

 static struct cmd_list_element *setmachinelist;

+static int
+h8300_register_sim_regno (struct gdbarch *gdbarch, int regnum)
+{
+  /* Only makes sense to supply raw registers.  */
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
+
+  /* We hide the raw ccr from the user by making it nameless.  Because
+     the default register_sim_regno hook returns
+     LEGACY_SIM_REGNO_IGNORE for unnamed registers, we need to
+     override it.  The sim register numbering is compatible with
+     gdb's, so there isn't anything to do.  */
+  return regnum;
+}
+
 static const char *
 h8300_register_name (struct gdbarch *gdbarch, int regno)
 {
@@ -1230,6 +1244,8 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)

   gdbarch = gdbarch_alloc (&info, 0);

+  set_gdbarch_register_sim_regno (gdbarch, h8300_register_sim_regno);
+
   switch (info.bfd_arch_info->mach)
     {
     case bfd_mach_h8300:
-- 
1.7.11.7

Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato]

At Mon, 10 Feb 2014 15:30:02 +0000,
Pedro Alves wrote:
> 
> On 02/08/2014 06:36 PM, Yoshinori Sato wrote:
> 
> 
> > #10 0x00000000005b954e in frame_unwind_register (frame=<optimized out>, 
> >     regnum=13, buf=<optimized out>) at ../../gdb/frame.c:1064
> 
> Hard to reason about an optimized build...  Please try with "-g3 -O0".
> 
> I don't have a h8300-elf toolchain handy, and the h8300-linux
> toolchain I found doesn't seem to want to link executables,
> but I managed to try something by building an .o file, and debugging
> that.  I don't see a crash, but instead GDB complains CCR
> is unavailable.
> 
> (gdb) info target
> Symbols from "/home/pedro/h8300-main.o".
> simulator:
>         Attached to sim running program /home/pedro/h8300-main.o
> 
> #instructions executed           0
> #cycles (v approximate)          0
> #real time taken            0.0000
> #virtual time taken         0.0000
> #compiles                        0
> #cache size                   1024
>         While running this, GDB does not access memory from...
> Local exec file:
>         `/home/pedro/h8300-main.o', file type elf32-h8300.
>         Entry point: 0x0
>         0x00000000 - 0x00000024 is .text
>         0x00000024 - 0x00000024 is .data
>         0x00000024 - 0x00000024 is .bss
> (gdb) b *0
> Breakpoint 1 at 0x0: file main.c, line 4.
> (gdb) r
> Starting program: /home/pedro/h8300-main.o
> 
> Breakpoint 1, foo (i=0x0 <foo>) at main.c:4
> 4       {
> (gdb) info registers
> r0             0x0000  0
> r1             0x0000  0
> r2             0x0000  0
> r3             0x0000  0
> r4             0x0000  0
> r5             0x0000  0
> r6             0x0000  0
> sp             0x0000  0
> Register 13 is not available
> (gdb) info registers ccr
> Register 13 is not available
> 
> The problem seems to me that the h8300 port does not define
> a register_sim_regno gdbarch hook, and thus when fetching
> registers off of the sim, we end up in legacy_register_sim_regno
> trying to figure out the sim register number for the raw CCR register:
> 
> int
> legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
> {
>   /* Only makes sense to supply raw registers.  */
>   gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
>   /* NOTE: cagney/2002-05-13: The old code did it this way and it is
>      suspected that some GDB/SIM combinations may rely on this
>      behavour.  The default should be one2one_register_sim_regno
>      (below).  */
>   if (gdbarch_register_name (gdbarch, regnum) != NULL
>       && gdbarch_register_name (gdbarch, regnum)[0] != '\0')
>     return regnum;
>   else
>     return LEGACY_SIM_REGNO_IGNORE;
> }
> 
> And because the raw ccr register does not have a name, that returns
> LEGACY_SIM_REGNO_IGNORE.  Which means that we never actually
> read the ccr raw value.  Before the <unavailable> support, this
> must have meant that ccr was _always_ read as 0...  At least, I'm
> not seeing how this ever worked.
> 
> Looking at sim/h8300/sim-main.h, it seems like the sim's register
> numbers are compatible with gdb's.
> 
> This patch below "works" for me, as in, I can now print CCR,
> but that's about all I tested (and am willing to test) myself.
> 
> Look me know how this looks to you.
>

It works fine (add my workaround).
But still abort.
I think reproduce "MALLOC_CHECK_=3 gdb".
backtrace in bellow.

Program received signal SIGABRT, Aborted.
0x00007ffff6aaf1d5 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff6aaf1d5 in __GI_raise (sig=sig@entry=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff6ab2388 in __GI_abort () at abort.c:90
#2  0x00007ffff6aea7bb in __libc_message (do_abort=do_abort@entry=1, 
    fmt=fmt@entry=0x7ffff6be4bee "%s")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:199
#3  0x00007ffff6aea89e in __GI___libc_fatal (
    message=0x7ffff6be8328 "memory clobbered past end of allocated block\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:210
#4  0x00007ffff6afaca5 in mabort (status=<optimized out>) at mcheck.c:364
#5  0x00007ffff6afad6b in checkhdr (hdr=<optimized out>) at mcheck.c:115
#6  checkhdr (hdr=<optimized out>) at mcheck.c:86
#7  0x00007ffff6afb0f5 in freehook (ptr=0xe804a0, caller=0x674f6e <xfree+31>)
    at mcheck.c:188
#8  0x0000000000674f6e in xfree (ptr=0xe804a0)
    at ../../gdb/common/common-utils.c:108
#9  0x00000000004c8cc9 in value_free (val=0xe7e350) at ../../gdb/value.c:1437
#10 0x0000000000641c4b in frame_register_unwind (frame=0xe33370, regnum=13, 
    optimizedp=0x7fffffffdcec, unavailablep=0x7fffffffdce8, 
    lvalp=0x7fffffffdcd8, addrp=0x7fffffffdce0, realnump=0x7fffffffdcdc, 
    bufferp=0x7fffffffdd10 "") at ../../gdb/frame.c:1032
#11 0x0000000000641dfc in frame_unwind_register (frame=0xe33370, regnum=13, 
    buf=0x7fffffffdd10 "") at ../../gdb/frame.c:1064
---Type <return> to continue, or q <return> to quit---
#12 0x00000000006421e2 in frame_unwind_register_signed (frame=0xe33370, 
    regnum=13) at ../../gdb/frame.c:1162
#13 0x000000000064221f in get_frame_register_signed (frame=0xe33430, regnum=13)
    at ../../gdb/frame.c:1169
#14 0x0000000000407da4 in h8300_print_register (gdbarch=0xe64970, 
    file=0xe54f70, frame=0xe33430, regno=13) at ../../gdb/h8300-tdep.c:1021
#15 0x00000000004084aa in h8300_print_registers_info (gdbarch=0xe64970, 
    file=0xe54f70, frame=0xe33430, regno=13, cpregs=0)
    at ../../gdb/h8300-tdep.c:1131
#16 0x0000000000544d01 in gdbarch_print_registers_info (gdbarch=0xe64970, 
    file=0xe54f70, frame=0xe33430, regnum=13, all=0)
    at ../../gdb/gdbarch.c:2357
#17 0x00000000005149b1 in registers_info (addr_exp=0xdac3b2 "", fpregs=0)
    at ../../gdb/infcmd.c:2212
#18 0x0000000000514b18 in nofp_registers_info (addr_exp=0xdac3af "ccr", 
    from_tty=1) at ../../gdb/infcmd.c:2264
#19 0x0000000000442f49 in do_cfunc (c=0xe12130, args=0xdac3af "ccr", 
    from_tty=1) at ../../gdb/cli/cli-decode.c:107
#20 0x000000000044603d in cmd_func (cmd=0xe12130, args=0xdac3af "ccr", 
    from_tty=1) at ../../gdb/cli/cli-decode.c:1886
#21 0x000000000063673d in execute_command (p=0xdac3b1 "r", from_tty=1)
    at ../../gdb/top.c:458
#22 0x000000000053d1d9 in command_handler (
---Type <return> to continue, or q <return> to quit---
    command=0xdac3a0 "info registers ccr") at ../../gdb/event-top.c:435
#23 0x000000000053d792 in command_line_handler (rl=0xe806c0 "")
    at ../../gdb/event-top.c:632
#24 0x00000000006c45b8 in rl_callback_read_char ()
    at ../../readline/callback.c:220
#25 0x000000000053cd0c in rl_callback_read_char_wrapper (client_data=0x0)
    at ../../gdb/event-top.c:164
#26 0x000000000053d0f0 in stdin_event_handler (error=0, client_data=0x0)
    at ../../gdb/event-top.c:375
#27 0x000000000053bcd7 in handle_file_event (data=...)
    at ../../gdb/event-loop.c:768
#28 0x000000000053b1b9 in process_event () at ../../gdb/event-loop.c:342
#29 0x000000000053b280 in gdb_do_one_event () at ../../gdb/event-loop.c:406
#30 0x000000000053b2d0 in start_event_loop () at ../../gdb/event-loop.c:431
#31 0x000000000053cd3e in cli_command_loop (data=0x0)
    at ../../gdb/event-top.c:179
#32 0x00000000005333ab in current_interp_command_loop ()
    at ../../gdb/interps.c:327
#33 0x000000000053441d in captured_command_loop (data=0x0)
    at ../../gdb/main.c:266
#34 0x0000000000530872 in catch_errors (func=0x534402 <captured_command_loop>, 
    func_args=0x0, errstring=0x7c8b42 "", mask=RETURN_MASK_ALL)
    at ../../gdb/exceptions.c:524
---Type <return> to continue, or q <return> to quit---
#35 0x00000000005357a1 in captured_main (data=0x7fffffffe3f0)
    at ../../gdb/main.c:1054
#36 0x0000000000530872 in catch_errors (func=0x5346b5 <captured_main>, 
    func_args=0x7fffffffe3f0, errstring=0x7c8b42 "", mask=RETURN_MASK_ALL)
    at ../../gdb/exceptions.c:524
#37 0x00000000005357ca in gdb_main (args=0x7fffffffe3f0)
    at ../../gdb/main.c:1062
#38 0x0000000000406584 in main (argc=2, argv=0x7fffffffe4f8)
    at ../../gdb/gdb.c:33

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

Re: [PATCH] h8300 "info registers" broken

[Pedro Alves]

On 02/11/2014 10:29 AM, Yoshinori Sato wrote:

> It works fine (add my workaround).
> But still abort.
> I think reproduce "MALLOC_CHECK_=3 gdb".
> backtrace in bellow.

OK, I can reproduce that way.  But Valgrind is much better
to debug this sort of thing.  See:

(gdb) info registers 
r0             0x0000  0
r1             0x0000  0
r2             0x0000  0
r3             0x0000  0
r4             0x0000  0
r5             0x0000  0
r6             0x0000  0
sp             0x0000  0
==23225== Invalid write of size 1
==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)
==23225==    by 0x5B694B: gdbarch_pseudo_register_read (gdbarch.c:1926)
==23225==    by 0x52DADB: regcache_cooked_read (regcache.c:740)
==23225==    by 0x52DC10: regcache_cooked_read_value (regcache.c:765)
==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
==23225==  Address 0x4f7b031 is 0 bytes after a block of size 1 alloc'd
==23225==    at 0x4A06B0F: calloc (vg_replace_malloc.c:593)
==23225==    by 0x6EB754: xcalloc (common-utils.c:91)
==23225==    by 0x6EB793: xzalloc (common-utils.c:101)
==23225==    by 0x53A782: allocate_value_contents (value.c:854)
==23225==    by 0x53A7B4: allocate_value (value.c:864)
==23225==    by 0x52DBC8: regcache_cooked_read_value (regcache.c:757)
==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
==23225== 
ccr            0x00        0    I-0 UI-0 H-0 U-0 N-0 Z-0 V-0 C-0 u> u>= != >= > 
pc             0x0000  0
cycles         0x0000  0
tick           0x0000  0
inst           0x0000  0
(gdb) q



This bit:

 ==23225== Invalid write of size 1
 ==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
 ==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
 ==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)

shows the problem.  The CCR pseudo register has type length of 1,
while the corresponding CCR raw register has a length of 2 or 4 (depending
on mode).  In sim/h8300/compile.c:sim_{fetch|store}_register
we see that the sim also treats those raw registers (CCR/EXR) as 2
or 4 bytes length.  Changing the GDB size of the raw registers as in your
patch to 1 byte length would then cause a mismatch with the sim,
and also break for remote targets, because it'd change the g/G
packets layout, in absence of target description support in
this target.

Please try this.

---------------
Subject: [PATCH] h8300

---
 gdb/h8300-tdep.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index ffffbc9..1be1f1e 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -939,6 +939,20 @@ h8300h_return_value (struct gdbarch *gdbarch, struct value *function,
 
 static struct cmd_list_element *setmachinelist;
 
+static int
+h8300_register_sim_regno (struct gdbarch *gdbarch, int regnum)
+{
+  /* Only makes sense to supply raw registers.  */
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
+
+  /* We hide the raw ccr from the user by making it nameless.  Because
+     the default register_sim_regno hook returns
+     LEGACY_SIM_REGNO_IGNORE for unnamed registers, we need to
+     override it.  The sim register numbering is compatible with
+     gdb's, so there isn't anything to do.  */
+  return regnum;
+}
+
 static const char *
 h8300_register_name (struct gdbarch *gdbarch, int regno)
 {
@@ -1148,15 +1162,55 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
     }
 }
 
+/* Helpers for h8300_pseudo_register_read.  We expose ccr/exr as
+   pseudo-registers to users with smaller sizes than the corresponding
+   raw registers.  These helpers extend/narrow the values.  */
+
+static enum register_status
+pseudo_from_raw_register (struct gdbarch *gdbarch, struct regcache *regcache,
+			  gdb_byte *buf, int pseudo_regno, int raw_regno)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum register_status status;
+  ULONGEST val;
+
+  status = regcache_raw_read_unsigned (regcache, raw_regno, &val);
+  if (status == REG_VALID)
+    store_unsigned_integer (buf,
+			    register_size (gdbarch, pseudo_regno),
+			    byte_order, val);
+  return status;
+}
+
+/* See pseudo_from_raw_register.  */
+
+static void
+raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
+			  const gdb_byte *buf, int raw_regno, int pseudo_regno)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  ULONGEST val;
+
+  val = extract_unsigned_integer (buf, register_size (gdbarch, pseudo_regno),
+				  byte_order);
+  regcache_raw_write_unsigned (regcache, raw_regno, val);
+}
+
 static enum register_status
 h8300_pseudo_register_read (struct gdbarch *gdbarch,
 			    struct regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
-    return regcache_raw_read (regcache, E_CCR_REGNUM, buf);
+    {
+      return pseudo_from_raw_register (gdbarch, regcache, buf,
+				       regno, E_CCR_REGNUM);
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
-    return regcache_raw_read (regcache, E_EXR_REGNUM, buf);
+    {
+      return pseudo_from_raw_register (gdbarch, regcache, buf,
+				       regno, E_EXR_REGNUM);
+    }
   else
     return regcache_raw_read (regcache, regno, buf);
 }
@@ -1167,9 +1221,9 @@ h8300_pseudo_register_write (struct gdbarch *gdbarch,
 			     const gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
-    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
+    raw_from_pseudo_register (gdbarch, regcache, buf, E_CCR_REGNUM, regno);
   else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
-    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
+    raw_from_pseudo_register (gdbarch, regcache, buf, E_EXR_REGNUM, regno);
   else
     regcache_raw_write (regcache, regno, buf);
 }
@@ -1230,6 +1284,8 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   gdbarch = gdbarch_alloc (&info, 0);
 
+  set_gdbarch_register_sim_regno (gdbarch, h8300_register_sim_regno);
+
   switch (info.bfd_arch_info->mach)
     {
     case bfd_mach_h8300:
-- 
1.7.11.7

Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato]

At Tue, 11 Feb 2014 11:47:18 +0000,
Pedro Alves wrote:
> 
> On 02/11/2014 10:29 AM, Yoshinori Sato wrote:
> 
> > It works fine (add my workaround).
> > But still abort.
> > I think reproduce "MALLOC_CHECK_=3 gdb".
> > backtrace in bellow.
> 
> OK, I can reproduce that way.  But Valgrind is much better
> to debug this sort of thing.  See:
> 
> (gdb) info registers 
> r0             0x0000  0
> r1             0x0000  0
> r2             0x0000  0
> r3             0x0000  0
> r4             0x0000  0
> r5             0x0000  0
> r6             0x0000  0
> sp             0x0000  0
> ==23225== Invalid write of size 1
> ==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
> ==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
> ==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)
> ==23225==    by 0x5B694B: gdbarch_pseudo_register_read (gdbarch.c:1926)
> ==23225==    by 0x52DADB: regcache_cooked_read (regcache.c:740)
> ==23225==    by 0x52DC10: regcache_cooked_read_value (regcache.c:765)
> ==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
> ==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
> ==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
> ==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
> ==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
> ==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
> ==23225==  Address 0x4f7b031 is 0 bytes after a block of size 1 alloc'd
> ==23225==    at 0x4A06B0F: calloc (vg_replace_malloc.c:593)
> ==23225==    by 0x6EB754: xcalloc (common-utils.c:91)
> ==23225==    by 0x6EB793: xzalloc (common-utils.c:101)
> ==23225==    by 0x53A782: allocate_value_contents (value.c:854)
> ==23225==    by 0x53A7B4: allocate_value (value.c:864)
> ==23225==    by 0x52DBC8: regcache_cooked_read_value (regcache.c:757)
> ==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
> ==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
> ==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
> ==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
> ==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
> ==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
> ==23225== 
> ccr            0x00        0    I-0 UI-0 H-0 U-0 N-0 Z-0 V-0 C-0 u> u>= != >= > 
> pc             0x0000  0
> cycles         0x0000  0
> tick           0x0000  0
> inst           0x0000  0
> (gdb) q
> 
> 
> 
> This bit:
> 
>  ==23225== Invalid write of size 1
>  ==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
>  ==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
>  ==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)
> 
> shows the problem.  The CCR pseudo register has type length of 1,
> while the corresponding CCR raw register has a length of 2 or 4 (depending
> on mode).  In sim/h8300/compile.c:sim_{fetch|store}_register
> we see that the sim also treats those raw registers (CCR/EXR) as 2
> or 4 bytes length.  Changing the GDB size of the raw registers as in your
> patch to 1 byte length would then cause a mismatch with the sim,
> and also break for remote targets, because it'd change the g/G
> packets layout, in absence of target description support in
> this target.
> 
> Please try this.

It works fine.
Thanks.
 
> ---------------
> Subject: [PATCH] h8300
> 
> ---
>  gdb/h8300-tdep.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
> index ffffbc9..1be1f1e 100644
> --- a/gdb/h8300-tdep.c
> +++ b/gdb/h8300-tdep.c
> @@ -939,6 +939,20 @@ h8300h_return_value (struct gdbarch *gdbarch, struct value *function,
>  
>  static struct cmd_list_element *setmachinelist;
>  
> +static int
> +h8300_register_sim_regno (struct gdbarch *gdbarch, int regnum)
> +{
> +  /* Only makes sense to supply raw registers.  */
> +  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
> +
> +  /* We hide the raw ccr from the user by making it nameless.  Because
> +     the default register_sim_regno hook returns
> +     LEGACY_SIM_REGNO_IGNORE for unnamed registers, we need to
> +     override it.  The sim register numbering is compatible with
> +     gdb's, so there isn't anything to do.  */
> +  return regnum;
> +}
> +
>  static const char *
>  h8300_register_name (struct gdbarch *gdbarch, int regno)
>  {
> @@ -1148,15 +1162,55 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
>      }
>  }
>  
> +/* Helpers for h8300_pseudo_register_read.  We expose ccr/exr as
> +   pseudo-registers to users with smaller sizes than the corresponding
> +   raw registers.  These helpers extend/narrow the values.  */
> +
> +static enum register_status
> +pseudo_from_raw_register (struct gdbarch *gdbarch, struct regcache *regcache,
> +			  gdb_byte *buf, int pseudo_regno, int raw_regno)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum register_status status;
> +  ULONGEST val;
> +
> +  status = regcache_raw_read_unsigned (regcache, raw_regno, &val);
> +  if (status == REG_VALID)
> +    store_unsigned_integer (buf,
> +			    register_size (gdbarch, pseudo_regno),
> +			    byte_order, val);
> +  return status;
> +}
> +
> +/* See pseudo_from_raw_register.  */
> +
> +static void
> +raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
> +			  const gdb_byte *buf, int raw_regno, int pseudo_regno)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  ULONGEST val;
> +
> +  val = extract_unsigned_integer (buf, register_size (gdbarch, pseudo_regno),
> +				  byte_order);
> +  regcache_raw_write_unsigned (regcache, raw_regno, val);
> +}
> +
>  static enum register_status
>  h8300_pseudo_register_read (struct gdbarch *gdbarch,
>  			    struct regcache *regcache, int regno,
>  			    gdb_byte *buf)
>  {
>    if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
> -    return regcache_raw_read (regcache, E_CCR_REGNUM, buf);
> +    {
> +      return pseudo_from_raw_register (gdbarch, regcache, buf,
> +				       regno, E_CCR_REGNUM);
> +    }
>    else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
> -    return regcache_raw_read (regcache, E_EXR_REGNUM, buf);
> +    {
> +      return pseudo_from_raw_register (gdbarch, regcache, buf,
> +				       regno, E_EXR_REGNUM);
> +    }
>    else
>      return regcache_raw_read (regcache, regno, buf);
>  }
> @@ -1167,9 +1221,9 @@ h8300_pseudo_register_write (struct gdbarch *gdbarch,
>  			     const gdb_byte *buf)
>  {
>    if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
> -    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
> +    raw_from_pseudo_register (gdbarch, regcache, buf, E_CCR_REGNUM, regno);
>    else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
> -    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
> +    raw_from_pseudo_register (gdbarch, regcache, buf, E_EXR_REGNUM, regno);
>    else
>      regcache_raw_write (regcache, regno, buf);
>  }
> @@ -1230,6 +1284,8 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  
>    gdbarch = gdbarch_alloc (&info, 0);
>  
> +  set_gdbarch_register_sim_regno (gdbarch, h8300_register_sim_regno);
> +
>    switch (info.bfd_arch_info->mach)
>      {
>      case bfd_mach_h8300:
> -- 
> 1.7.11.7
> 
> 

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

[PATCH 0/2] H8/300: Fix registers

[Pedro Alves]

On 02/11/2014 01:11 PM, Yoshinori Sato wrote:
> Pedro Alves wrote:
>> Please try this.
> 
> It works fine.
> Thanks.

Alright, I've split this in two patches, and pushed them.

Pedro Alves (2):
  H8/300: Fix gdb<->sim register mapping.
  H8/300: Fix pseudo registers reads/writes.

 gdb/ChangeLog    | 13 +++++++++++
 gdb/h8300-tdep.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 75 insertions(+), 4 deletions(-)

-- 
1.7.11.7

[PATCH 2/2] H8/300: Fix pseudo registers reads/writes.

[Pedro Alves]

'info registers ccr' corrupts memory.

Debugging gdb under Valgrind, we see:

 (gdb) info registers ccr
 ==23225== Invalid write of size 1
 ==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
 ==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
 ==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)
 ==23225==    by 0x5B694B: gdbarch_pseudo_register_read (gdbarch.c:1926)
 ==23225==    by 0x52DADB: regcache_cooked_read (regcache.c:740)
 ==23225==    by 0x52DC10: regcache_cooked_read_value (regcache.c:765)
 ==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
 ==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
 ==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
 ==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
 ==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
 ==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
 ==23225==  Address 0x4f7b031 is 0 bytes after a block of size 1 alloc'd
 ==23225==    at 0x4A06B0F: calloc (vg_replace_malloc.c:593)
 ==23225==    by 0x6EB754: xcalloc (common-utils.c:91)
 ==23225==    by 0x6EB793: xzalloc (common-utils.c:101)
 ==23225==    by 0x53A782: allocate_value_contents (value.c:854)
 ==23225==    by 0x53A7B4: allocate_value (value.c:864)
 ==23225==    by 0x52DBC8: regcache_cooked_read_value (regcache.c:757)
 ==23225==    by 0x68CA41: sentinel_frame_prev_register (sentinel-frame.c:52)
 ==23225==    by 0x6B80CB: frame_unwind_register_value (frame.c:1105)
 ==23225==    by 0x6B7C97: frame_register_unwind (frame.c:1010)
 ==23225==    by 0x6B7F73: frame_unwind_register (frame.c:1064)
 ==23225==    by 0x6B8359: frame_unwind_register_signed (frame.c:1162)
 ==23225==    by 0x6B8396: get_frame_register_signed (frame.c:1169)
 ==23225==
 ccr            0x00        0    I-0 UI-0 H-0 U-0 N-0 Z-0 V-0 C-0 u> u>= != >= >
 (gdb)

This bit:

 ==23225== Invalid write of size 1
 ==23225==    at 0x4A0A308: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:881)
 ==23225==    by 0x52D334: regcache_raw_read (regcache.c:625)
 ==23225==    by 0x45E4D8: h8300_pseudo_register_read (h8300-tdep.c:1171)

shows the problem.  The CCR pseudo register has type length of 1,
while the corresponding CCR raw register has a length of 2 or 4
(depending on mode).  In
sim/h8300/compile.c:sim_{fetch|store}_register we see that the sim
also treats those raw registers (CCR/EXR) as 2 or 4 bytes length.

gdb/
2014-02-12  Pedro Alves  <palves@redhat.com>

	* h8300-tdep.c (pseudo_from_raw_register)
	(raw_from_pseudo_register): New functions.
	(h8300_pseudo_register_read, h8300_pseudo_register_write): Use
	them.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/h8300-tdep.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6f0f903..5e9e9b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2014-02-12  Pedro Alves  <palves@redhat.com>
 
+	* h8300-tdep.c (pseudo_from_raw_register)
+	(raw_from_pseudo_register): New functions.
+	(h8300_pseudo_register_read, h8300_pseudo_register_write): Use
+	them.
+
+2014-02-12  Pedro Alves  <palves@redhat.com>
+
 	* h8300-tdep.c (h8300_register_sim_regno): New function.
 	(h8300_gdbarch_init): Install h8300_register_sim_regno as
 	gdbarch_register_sim_regno hook.
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 4193287..98343e0 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -1164,15 +1164,55 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
     }
 }
 
+/* Helpers for h8300_pseudo_register_read.  We expose ccr/exr as
+   pseudo-registers to users with smaller sizes than the corresponding
+   raw registers.  These helpers extend/narrow the values.  */
+
+static enum register_status
+pseudo_from_raw_register (struct gdbarch *gdbarch, struct regcache *regcache,
+			  gdb_byte *buf, int pseudo_regno, int raw_regno)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum register_status status;
+  ULONGEST val;
+
+  status = regcache_raw_read_unsigned (regcache, raw_regno, &val);
+  if (status == REG_VALID)
+    store_unsigned_integer (buf,
+			    register_size (gdbarch, pseudo_regno),
+			    byte_order, val);
+  return status;
+}
+
+/* See pseudo_from_raw_register.  */
+
+static void
+raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
+			  const gdb_byte *buf, int raw_regno, int pseudo_regno)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  ULONGEST val;
+
+  val = extract_unsigned_integer (buf, register_size (gdbarch, pseudo_regno),
+				  byte_order);
+  regcache_raw_write_unsigned (regcache, raw_regno, val);
+}
+
 static enum register_status
 h8300_pseudo_register_read (struct gdbarch *gdbarch,
 			    struct regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
-    return regcache_raw_read (regcache, E_CCR_REGNUM, buf);
+    {
+      return pseudo_from_raw_register (gdbarch, regcache, buf,
+				       regno, E_CCR_REGNUM);
+    }
   else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
-    return regcache_raw_read (regcache, E_EXR_REGNUM, buf);
+    {
+      return pseudo_from_raw_register (gdbarch, regcache, buf,
+				       regno, E_EXR_REGNUM);
+    }
   else
     return regcache_raw_read (regcache, regno, buf);
 }
@@ -1183,9 +1223,9 @@ h8300_pseudo_register_write (struct gdbarch *gdbarch,
 			     const gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
-    regcache_raw_write (regcache, E_CCR_REGNUM, buf);
+    raw_from_pseudo_register (gdbarch, regcache, buf, E_CCR_REGNUM, regno);
   else if (regno == E_PSEUDO_EXR_REGNUM (gdbarch))
-    regcache_raw_write (regcache, E_EXR_REGNUM, buf);
+    raw_from_pseudo_register (gdbarch, regcache, buf, E_EXR_REGNUM, regno);
   else
     regcache_raw_write (regcache, regno, buf);
 }
-- 
1.7.11.7

Re: [PATCH 2/2] H8/300: Fix pseudo registers reads/writes.

[Mark Kettenis]

> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 12 Feb 2014 12:42:07 +0000
> 
> shows the problem.  The CCR pseudo register has type length of 1,
> while the corresponding CCR raw register has a length of 2 or 4
> (depending on mode).  In
> sim/h8300/compile.c:sim_{fetch|store}_register we see that the sim
> also treats those raw registers (CCR/EXR) as 2 or 4 bytes length.
> 
> gdb/
> 2014-02-12  Pedro Alves  <palves@redhat.com>
> 
> 	* h8300-tdep.c (pseudo_from_raw_register)
> 	(raw_from_pseudo_register): New functions.
> 	(h8300_pseudo_register_read, h8300_pseudo_register_write): Use
> 	them.

Since the whole point of the pseudo registers seems to be to handle
the size difference, this looks totally ok to me.

[PATCH 1/2] H8/300: Fix gdb<->sim register mapping.

[Pedro Alves]

Currently, printing the H8/300 ccr register when debugging with the
sim is broken:

 (gdb) target sim
 ...
 (gdb) load
 ...
 (gdb) start
 ...
 Breakpoint 1, foo (i=0x0 <foo>) at main.c:4
 4       {
 (gdb) info registers ccr
 Register 13 is not available

'13' is the ccr pseudo-register.  This pseudo-register provides an
8-bit view into the raw ccr register (regno=8).

The problem is that the H8/300 port does not define a
register_sim_regno gdbarch hook, and thus when fetching the raw
register off of the sim, we end up in legacy_register_sim_regno trying
to figure out the sim register number for the raw CCR register:

 int
 legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
 {
   /* Only makes sense to supply raw registers.  */
   gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
   /* NOTE: cagney/2002-05-13: The old code did it this way and it is
      suspected that some GDB/SIM combinations may rely on this
      behavour.  The default should be one2one_register_sim_regno
      (below).  */
   if (gdbarch_register_name (gdbarch, regnum) != NULL
       && gdbarch_register_name (gdbarch, regnum)[0] != '\0')
     return regnum;
   else
     return LEGACY_SIM_REGNO_IGNORE;
 }

Because the raw ccr register does not have a name (so that it is
hidden from the user), that returns LEGACY_SIM_REGNO_IGNORE.  That
means that we never actually read the value of the raw ccr register.

Before the <unavailable> support, this must have meant that ccr was
_always_ read as 0...  At least, I'm not seeing how this ever worked.

The fix for that is adding a gdbarch_register_sim_regno hook that maps
all raw registers.  Looking at sim/h8300/sim-main.h, I believe the
sim's register numbers are compatible with gdb's, so no actual
convertion is necessary.

gdb/
2014-02-12  Pedro Alves  <palves@redhat.com>

	* h8300-tdep.c (h8300_register_sim_regno): New function.
	(h8300_gdbarch_init): Install h8300_register_sim_regno as
	gdbarch_register_sim_regno hook.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/h8300-tdep.c | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bb977b5..6f0f903 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-02-12  Pedro Alves  <palves@redhat.com>
+
+	* h8300-tdep.c (h8300_register_sim_regno): New function.
+	(h8300_gdbarch_init): Install h8300_register_sim_regno as
+	gdbarch_register_sim_regno hook.
+
 2014-02-12  Sanimir Agovic  <sanimir.agovic@intel.com>
 
 	* nios2-tdep.c (nios2_stub_frame_base): Remove global.
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index ffffbc9..4193287 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -939,6 +939,22 @@ h8300h_return_value (struct gdbarch *gdbarch, struct value *function,
 
 static struct cmd_list_element *setmachinelist;
 
+/* Implementation of 'register_sim_regno' gdbarch method.  */
+
+static int
+h8300_register_sim_regno (struct gdbarch *gdbarch, int regnum)
+{
+  /* Only makes sense to supply raw registers.  */
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch));
+
+  /* We hide the raw ccr from the user by making it nameless.  Because
+     the default register_sim_regno hook returns
+     LEGACY_SIM_REGNO_IGNORE for unnamed registers, we need to
+     override it.  The sim register numbering is compatible with
+     gdb's.  */
+  return regnum;
+}
+
 static const char *
 h8300_register_name (struct gdbarch *gdbarch, int regno)
 {
@@ -1230,6 +1246,8 @@ h8300_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   gdbarch = gdbarch_alloc (&info, 0);
 
+  set_gdbarch_register_sim_regno (gdbarch, h8300_register_sim_regno);
+
   switch (info.bfd_arch_info->mach)
     {
     case bfd_mach_h8300:
-- 
1.7.11.7