Re: [PATCH] h8300 "info registers" broken

[Yoshinori Sato <ysato@users.sourceforge.jp>]

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>