Re: [PATCH] h8300 "info registers" broken

[Pedro Alves <palves at redhat dot com>]

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