public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 06/10] Replace regcache::dump with class register_dump
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (6 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 04/10] Class readonly_detached_regcache Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 09/10] Move register_dump to regcache-dump.c Yao Qi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

Nowadays, we need to dump registers contents from "readwrite" regcache and
"readonly" regcache,

  if (target_has_registers)
    get_current_regcache ()->dump (out, what_to_dump);
  else
    {
      /* For the benefit of "maint print registers" & co when
         debugging an executable, allow dumping a regcache even when
         there is no thread selected / no registers.  */
      regcache dummy_regs (target_gdbarch ());
      dummy_regs.dump (out, what_to_dump);
    }

since we'll have two different types/classes for "readwrite" regcache and
"readonly" regcache, we have to move dump method to their parent class,
reg_buffer.  However, the functionality of "dump" looks unnecessary to
reg_buffer (because some dump modes like regcache_dump_none,
regcache_dump_remote and regcache_dump_groups don't need reg_buffer at
all, they need gdbarch to do the dump), so I decide to move "dump" into a
separate classes, and each sub-class is about each mode of dump.

gdb:

2017-11-28  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (class register_dump): New class.
	(register_dump_regcache, register_dump_none): New class.
	(register_dump_remote, register_dump_groups): New class.
	(regcache_print): Update.
	* regcache.h (regcache_dump_what): Move it to regcache.c.
	(regcache) <dump>: Remove.
---
 gdb/regcache.c | 483 ++++++++++++++++++++++++++++++++++-----------------------
 gdb/regcache.h |   9 --
 2 files changed, 285 insertions(+), 207 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index c757231..389c016 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1315,231 +1315,284 @@ reg_flush_command (const char *command, int from_tty)
     printf_filtered (_("Register cache flushed.\n"));
 }
 
-void
-regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
+/* An abstract base class for register dump.  */
+
+class register_dump
 {
-  struct gdbarch *gdbarch = m_descr->gdbarch;
-  int regnum;
-  int footnote_nr = 0;
-  int footnote_register_offset = 0;
-  int footnote_register_type_name_null = 0;
-  long register_offset = 0;
+public:
+  void dump (ui_file *file)
+  {
+    auto descr = regcache_descr (m_gdbarch);
+    int regnum;
+    int footnote_nr = 0;
+    int footnote_register_offset = 0;
+    int footnote_register_type_name_null = 0;
+    long register_offset = 0;
+
+    gdb_assert (descr->nr_cooked_registers
+		== (gdbarch_num_regs (m_gdbarch)
+		    + gdbarch_num_pseudo_regs (m_gdbarch)));
+
+    for (regnum = -1; regnum < descr->nr_cooked_registers; regnum++)
+      {
+	/* Name.  */
+	if (regnum < 0)
+	  fprintf_unfiltered (file, " %-10s", "Name");
+	else
+	  {
+	    const char *p = gdbarch_register_name (m_gdbarch, regnum);
 
-  gdb_assert (m_descr->nr_cooked_registers
-	      == (gdbarch_num_regs (gdbarch)
-		  + gdbarch_num_pseudo_regs (gdbarch)));
+	    if (p == NULL)
+	      p = "";
+	    else if (p[0] == '\0')
+	      p = "''";
+	    fprintf_unfiltered (file, " %-10s", p);
+	  }
 
-  for (regnum = -1; regnum < m_descr->nr_cooked_registers; regnum++)
-    {
-      /* Name.  */
-      if (regnum < 0)
-	fprintf_unfiltered (file, " %-10s", "Name");
-      else
-	{
-	  const char *p = gdbarch_register_name (gdbarch, regnum);
+	/* Number.  */
+	if (regnum < 0)
+	  fprintf_unfiltered (file, " %4s", "Nr");
+	else
+	  fprintf_unfiltered (file, " %4d", regnum);
 
-	  if (p == NULL)
-	    p = "";
-	  else if (p[0] == '\0')
-	    p = "''";
-	  fprintf_unfiltered (file, " %-10s", p);
-	}
+	/* Relative number.  */
+	if (regnum < 0)
+	  fprintf_unfiltered (file, " %4s", "Rel");
+	else if (regnum < gdbarch_num_regs (m_gdbarch))
+	  fprintf_unfiltered (file, " %4d", regnum);
+	else
+	  fprintf_unfiltered (file, " %4d",
+			      (regnum - gdbarch_num_regs (m_gdbarch)));
 
-      /* Number.  */
-      if (regnum < 0)
-	fprintf_unfiltered (file, " %4s", "Nr");
-      else
-	fprintf_unfiltered (file, " %4d", regnum);
+	/* Offset.  */
+	if (regnum < 0)
+	  fprintf_unfiltered (file, " %6s  ", "Offset");
+	else
+	  {
+	    fprintf_unfiltered (file, " %6ld",
+				descr->register_offset[regnum]);
+	    if (register_offset != descr->register_offset[regnum]
+		|| (regnum > 0
+		    && (descr->register_offset[regnum]
+			!= (descr->register_offset[regnum - 1]
+			    + descr->sizeof_register[regnum - 1])))
+		)
+	      {
+		if (!footnote_register_offset)
+		  footnote_register_offset = ++footnote_nr;
+		fprintf_unfiltered (file, "*%d", footnote_register_offset);
+	      }
+	    else
+	      fprintf_unfiltered (file, "  ");
+	    register_offset = (descr->register_offset[regnum]
+			       + descr->sizeof_register[regnum]);
+	  }
 
-      /* Relative number.  */
-      if (regnum < 0)
-	fprintf_unfiltered (file, " %4s", "Rel");
-      else if (regnum < gdbarch_num_regs (gdbarch))
-	fprintf_unfiltered (file, " %4d", regnum);
-      else
-	fprintf_unfiltered (file, " %4d",
-			    (regnum - gdbarch_num_regs (gdbarch)));
+	/* Size.  */
+	if (regnum < 0)
+	  fprintf_unfiltered (file, " %5s ", "Size");
+	else
+	  fprintf_unfiltered (file, " %5ld", descr->sizeof_register[regnum]);
 
-      /* Offset.  */
-      if (regnum < 0)
-	fprintf_unfiltered (file, " %6s  ", "Offset");
-      else
+	/* Type.  */
 	{
-	  fprintf_unfiltered (file, " %6ld",
-			      m_descr->register_offset[regnum]);
-	  if (register_offset != m_descr->register_offset[regnum]
-	      || (regnum > 0
-		  && (m_descr->register_offset[regnum]
-		      != (m_descr->register_offset[regnum - 1]
-			  + m_descr->sizeof_register[regnum - 1])))
-	      )
+	  const char *t;
+	  std::string name_holder;
+
+	  if (regnum < 0)
+	    t = "Type";
+	  else
 	    {
-	      if (!footnote_register_offset)
-		footnote_register_offset = ++footnote_nr;
-	      fprintf_unfiltered (file, "*%d", footnote_register_offset);
+	      static const char blt[] = "builtin_type";
+
+	      t = TYPE_NAME (register_type (m_gdbarch, regnum));
+	      if (t == NULL)
+		{
+		  if (!footnote_register_type_name_null)
+		    footnote_register_type_name_null = ++footnote_nr;
+		  name_holder = string_printf ("*%d",
+					       footnote_register_type_name_null);
+		  t = name_holder.c_str ();
+		}
+	      /* Chop a leading builtin_type.  */
+	      if (startswith (t, blt))
+		t += strlen (blt);
 	    }
-	  else
-	    fprintf_unfiltered (file, "  ");
-	  register_offset = (m_descr->register_offset[regnum]
-			     + m_descr->sizeof_register[regnum]);
+	  fprintf_unfiltered (file, " %-15s", t);
 	}
 
-      /* Size.  */
-      if (regnum < 0)
-	fprintf_unfiltered (file, " %5s ", "Size");
-      else
-	fprintf_unfiltered (file, " %5ld", m_descr->sizeof_register[regnum]);
+	/* Leading space always present.  */
+	fprintf_unfiltered (file, " ");
 
-      /* Type.  */
-      {
-	const char *t;
-	std::string name_holder;
+	dump_reg (file, regnum);
 
-	if (regnum < 0)
-	  t = "Type";
+	fprintf_unfiltered (file, "\n");
+      }
+
+    if (footnote_register_offset)
+      fprintf_unfiltered (file, "*%d: Inconsistent register offsets.\n",
+			  footnote_register_offset);
+    if (footnote_register_type_name_null)
+      fprintf_unfiltered (file,
+			  "*%d: Register type's name NULL.\n",
+			  footnote_register_type_name_null);
+  }
+
+  virtual ~register_dump () {};
+
+protected:
+  register_dump (gdbarch *arch)
+    : m_gdbarch (arch)
+  {}
+
+  /* Dump the register REGNUM contents.  If REGNUM is -1, print the
+     header.  */
+  virtual void dump_reg (ui_file *file, int regnum) = 0;
+
+  gdbarch *m_gdbarch;
+};
+
+/* Dump registers from regcache, used for dump raw registers and
+   cooked registers.  */
+
+class register_dump_regcache : public register_dump
+{
+public:
+  register_dump_regcache (regcache *regcache, bool dump_pseudo)
+    : register_dump (regcache->arch ()), m_regcache (regcache),
+      m_dump_pseudo (dump_pseudo)
+  {
+  }
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	if (m_dump_pseudo)
+	  fprintf_unfiltered (file, "Cooked value");
 	else
+	  fprintf_unfiltered (file, "Raw value");
+      }
+    else
+      {
+	if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
 	  {
-	    static const char blt[] = "builtin_type";
+	    auto size = register_size (m_gdbarch, regnum);
+
+	    if (size == 0)
+	      return;
 
-	    t = TYPE_NAME (register_type (arch (), regnum));
-	    if (t == NULL)
+	    gdb::def_vector<gdb_byte> buf (size);
+	    auto status = m_regcache->cooked_read (regnum, buf.data ());
+
+	    if (status == REG_UNKNOWN)
+	      fprintf_unfiltered (file, "<invalid>");
+	    else if (status == REG_UNAVAILABLE)
+	      fprintf_unfiltered (file, "<unavailable>");
+	    else
 	      {
-		if (!footnote_register_type_name_null)
-		  footnote_register_type_name_null = ++footnote_nr;
-		name_holder = string_printf ("*%d",
-					     footnote_register_type_name_null);
-		t = name_holder.c_str ();
+		print_hex_chars (file, buf.data (), size,
+				 gdbarch_byte_order (m_gdbarch), true);
 	      }
-	    /* Chop a leading builtin_type.  */
-	    if (startswith (t, blt))
-	      t += strlen (blt);
 	  }
-	fprintf_unfiltered (file, " %-15s", t);
+	else
+	  {
+	    /* Just print "<cooked>" for pseudo register when
+	       regcache_dump_raw.  */
+	    fprintf_unfiltered (file, "<cooked>");
+	  }
       }
+  }
 
-      /* Leading space always present.  */
-      fprintf_unfiltered (file, " ");
+private:
+  regcache *m_regcache;
 
-      /* Value, raw.  */
-      if (what_to_dump == regcache_dump_raw)
-	{
-	  if (regnum < 0)
-	    fprintf_unfiltered (file, "Raw value");
-	  else if (regnum >= num_raw_registers ())
-	    fprintf_unfiltered (file, "<cooked>");
-	  else if (get_register_status (regnum) == REG_UNKNOWN)
-	    fprintf_unfiltered (file, "<invalid>");
-	  else if (get_register_status (regnum) == REG_UNAVAILABLE)
-	    fprintf_unfiltered (file, "<unavailable>");
-	  else
-	    {
-	      raw_update (regnum);
-	      print_hex_chars (file, register_buffer (regnum),
-			       m_descr->sizeof_register[regnum],
-			       gdbarch_byte_order (gdbarch), true);
-	    }
-	}
+  /* Dump pseudo registers or not.  */
+  const bool m_dump_pseudo;
+};
 
-      /* Value, cooked.  */
-      if (what_to_dump == regcache_dump_cooked)
-	{
-	  if (regnum < 0)
-	    fprintf_unfiltered (file, "Cooked value");
-	  else
-	    {
-	      const gdb_byte *buf = NULL;
-	      enum register_status status;
-	      struct value *value = NULL;
+/* For "maint print registers".  */
 
-	      if (regnum < num_raw_registers ())
-		{
-		  raw_update (regnum);
-		  status = get_register_status (regnum);
-		  buf = register_buffer (regnum);
-		}
-	      else
-		{
-		  value = cooked_read_value (regnum);
-
-		  if (!value_optimized_out (value)
-		      && value_entirely_available (value))
-		    {
-		      status = REG_VALID;
-		      buf = value_contents_all (value);
-		    }
-		  else
-		    status = REG_UNAVAILABLE;
-		}
+class register_dump_none : public register_dump
+{
+public:
+  register_dump_none (gdbarch *arch)
+    : register_dump (arch)
+  {}
 
-	      if (status == REG_UNKNOWN)
-		fprintf_unfiltered (file, "<invalid>");
-	      else if (status == REG_UNAVAILABLE)
-		fprintf_unfiltered (file, "<unavailable>");
-	      else
-		print_hex_chars (file, buf,
-				 m_descr->sizeof_register[regnum],
-				 gdbarch_byte_order (gdbarch), true);
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {}
+};
 
-	      if (value != NULL)
-		{
-		  release_value (value);
-		  value_free (value);
-		}
-	    }
-	}
+/* For "maint print remote-registers".  */
 
-      /* Group members.  */
-      if (what_to_dump == regcache_dump_groups)
-	{
-	  if (regnum < 0)
-	    fprintf_unfiltered (file, "Groups");
-	  else
-	    {
-	      const char *sep = "";
-	      struct reggroup *group;
+class register_dump_remote : public register_dump
+{
+public:
+  register_dump_remote (gdbarch *arch)
+    : register_dump (arch)
+  {}
 
-	      for (group = reggroup_next (gdbarch, NULL);
-		   group != NULL;
-		   group = reggroup_next (gdbarch, group))
-		{
-		  if (gdbarch_register_reggroup_p (gdbarch, regnum, group))
-		    {
-		      fprintf_unfiltered (file,
-					  "%s%s", sep, reggroup_name (group));
-		      sep = ",";
-		    }
-		}
-	    }
-	}
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
+      }
+    else if (regnum < gdbarch_num_regs (m_gdbarch))
+      {
+	int pnum, poffset;
 
-      /* Remote packet configuration.  */
-      if (what_to_dump == regcache_dump_remote)
-	{
-	  if (regnum < 0)
-	    {
-	      fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
-	    }
-	  else if (regnum < num_raw_registers ())
-	    {
-	      int pnum, poffset;
+	if (remote_register_number_and_offset (m_gdbarch, regnum,
+					       &pnum, &poffset))
+	  fprintf_unfiltered (file, "%7d %11d", pnum, poffset);
+      }
+  }
+};
 
-	      if (remote_register_number_and_offset (arch (), regnum,
-						     &pnum, &poffset))
-		fprintf_unfiltered (file, "%7d %11d", pnum, poffset);
-	    }
-	}
+/* For "maint print register-groups".  */
 
-      fprintf_unfiltered (file, "\n");
-    }
+class register_dump_groups : public register_dump
+{
+public:
+  register_dump_groups (gdbarch *arch)
+    : register_dump (arch)
+  {}
 
-  if (footnote_register_offset)
-    fprintf_unfiltered (file, "*%d: Inconsistent register offsets.\n",
-			footnote_register_offset);
-  if (footnote_register_type_name_null)
-    fprintf_unfiltered (file, 
-			"*%d: Register type's name NULL.\n",
-			footnote_register_type_name_null);
-}
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      fprintf_unfiltered (file, "Groups");
+    else
+      {
+	const char *sep = "";
+	struct reggroup *group;
+
+	for (group = reggroup_next (m_gdbarch, NULL);
+	     group != NULL;
+	     group = reggroup_next (m_gdbarch, group))
+	  {
+	    if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
+	      {
+		fprintf_unfiltered (file,
+				    "%s%s", sep, reggroup_name (group));
+		sep = ",";
+	      }
+	  }
+      }
+  }
+};
+
+enum regcache_dump_what
+{
+  regcache_dump_none, regcache_dump_raw,
+  regcache_dump_cooked, regcache_dump_groups,
+  regcache_dump_remote
+};
 
 static void
 regcache_print (const char *args, enum regcache_dump_what what_to_dump)
@@ -1557,16 +1610,50 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
       out = &file;
     }
 
+  std::unique_ptr<register_dump> dump;
+  std::unique_ptr<regcache> regs;
+  gdbarch *gdbarch;
+
   if (target_has_registers)
-    get_current_regcache ()->dump (out, what_to_dump);
+    gdbarch = get_current_regcache ()->arch ();
   else
+    gdbarch = target_gdbarch ();
+
+  switch (what_to_dump)
     {
-      /* For the benefit of "maint print registers" & co when
-	 debugging an executable, allow dumping a regcache even when
-	 there is no thread selected / no registers.  */
-      regcache dummy_regs (target_gdbarch ());
-      dummy_regs.dump (out, what_to_dump);
+    case regcache_dump_none:
+      dump.reset (new register_dump_none (gdbarch));
+      break;
+    case regcache_dump_remote:
+      dump.reset (new register_dump_remote (gdbarch));
+      break;
+    case regcache_dump_groups:
+      dump.reset (new register_dump_groups (gdbarch));
+      break;
+    case regcache_dump_raw:
+    case regcache_dump_cooked:
+      {
+	regcache *reg;
+
+	if (target_has_registers)
+	  reg = get_current_regcache ();
+	else
+	  {
+	    /* For the benefit of "maint print registers" & co when
+	       debugging an executable, allow dumping a regcache even when
+	       there is no thread selected / no registers.  */
+	    reg = new regcache (target_gdbarch ());
+	    regs.reset (reg);
+	  }
+
+	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
+
+	dump.reset (new register_dump_regcache (reg, dump_pseudo));
+      }
+      break;
     }
+
+  dump->dump (out);
 }
 
 static void
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 9a1ac41..0bca9f3 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -194,13 +194,6 @@ typedef enum register_status (regcache_cooked_read_ftype) (void *src,
 							   int regnum,
 							   gdb_byte *buf);
 
-enum regcache_dump_what
-{
-  regcache_dump_none, regcache_dump_raw,
-  regcache_dump_cooked, regcache_dump_groups,
-  regcache_dump_remote
-};
-
 /* A (register_number, register_value) pair.  */
 
 typedef struct cached_reg
@@ -374,8 +367,6 @@ public:
   void collect_regset (const struct regset *regset, int regnum,
 		       void *buf, size_t size) const;
 
-  void dump (ui_file *file, enum regcache_dump_what what_to_dump);
-
   ptid_t ptid () const
   {
     return m_ptid;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 07/10] No longer create readonly regcache
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
  2018-02-07 10:33 ` [PATCH 10/10] Pass readable_regcache to gdbarch method read_pc Yao Qi
  2018-02-07 10:33 ` [PATCH 03/10] Remove regcache_save and regcache_cpy Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 01/10] Class reg_buffer Yao Qi
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

Nowadays, we create a readonly regcache in get_return_value, and pass it
to gdbarch_return_value to get the return value.  In theory, we can pass a
readable_regcache instance and get the return value, because we don't need
to modify the regcache.  Unfortunately, gdbarch_return_value is designed
to multiplex regcache, according to READBUF and WRITEBUF.

 # If READBUF is not NULL, extract the return value and save it in this
 # buffer.
 #
 # If WRITEBUF is not NULL, it contains a return value which will be
 # stored into the appropriate register.

In fact, gdbarch_return_value should be split to three functions, 1) only
return return_value_convention, 2) pass regcache_readonly and readbuf, 3)
pass regcache and writebuf.  These changes are out of the scope of this
patch series, so I pass regcache to gdbarch_return_value even for read,
and trust each gdbarch backend doesn't modify regcache.

gdb:

2017-11-02  Yao Qi  <yao.qi@linaro.org>

	* infcmd.c (get_return_value): Let stop_regs point to
	get_current_regcache.
	* regcache.c (regcache::regcache): Remove.
	(register_dump_reg_buffer): New class.
	(regcache_print): Adjust.
	* regcache.h (regcache): Remove constructors.
---
 gdb/infcmd.c   |  6 ++---
 gdb/regcache.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/regcache.h | 10 ---------
 3 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3879df3..170c8d5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1624,8 +1624,8 @@ advance_command (const char *arg, int from_tty)
 struct value *
 get_return_value (struct value *function, struct type *value_type)
 {
-  regcache stop_regs (regcache::readonly, *get_current_regcache ());
-  struct gdbarch *gdbarch = stop_regs.arch ();
+  regcache *stop_regs = get_current_regcache ();
+  struct gdbarch *gdbarch = stop_regs->arch ();
   struct value *value;
 
   value_type = check_typedef (value_type);
@@ -1645,7 +1645,7 @@ get_return_value (struct value *function, struct type *value_type)
     case RETURN_VALUE_ABI_RETURNS_ADDRESS:
     case RETURN_VALUE_ABI_PRESERVES_ADDRESS:
       value = allocate_value (value_type);
-      gdbarch_return_value (gdbarch, function, value_type, &stop_regs,
+      gdbarch_return_value (gdbarch, function, value_type, stop_regs,
 			    value_contents_raw (value), NULL);
       break;
     case RETURN_VALUE_STRUCT_CONVENTION:
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 389c016..8f81163 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -219,13 +219,6 @@ do_cooked_read (void *src, int regnum, gdb_byte *buf)
   return regcache_cooked_read (regcache, regnum, buf);
 }
 
-regcache::regcache (readonly_t, const regcache &src)
-  : regcache (src.arch (), nullptr, true)
-{
-  gdb_assert (!src.m_readonly_p);
-  save (do_cooked_read, (void *) &src);
-}
-
 readonly_detached_regcache::readonly_detached_regcache (const regcache &src)
   : readonly_detached_regcache (src.arch (), do_cooked_read, (void *) &src)
 {
@@ -1512,6 +1505,55 @@ private:
   const bool m_dump_pseudo;
 };
 
+/* Dump from reg_buffer, used when there is no thread or
+   registers.  */
+
+class register_dump_reg_buffer : public register_dump, reg_buffer
+{
+public:
+  register_dump_reg_buffer (gdbarch *gdbarch, bool dump_pseudo)
+    : register_dump (gdbarch), reg_buffer (gdbarch, dump_pseudo)
+  {
+  }
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	if (m_has_pseudo)
+	  fprintf_unfiltered (file, "Cooked value");
+	else
+	  fprintf_unfiltered (file, "Raw value");
+      }
+    else
+      {
+	if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
+	  {
+	    auto size = register_size (m_gdbarch, regnum);
+
+	    if (size == 0)
+	      return;
+
+	    auto status = get_register_status (regnum);
+
+	    gdb_assert (status != REG_VALID);
+
+	    if (status == REG_UNKNOWN)
+	      fprintf_unfiltered (file, "<invalid>");
+	    else
+	      fprintf_unfiltered (file, "<unavailable>");
+	  }
+	else
+	  {
+	    /* Just print "<cooked>" for pseudo register when
+	       regcache_dump_raw.  */
+	    fprintf_unfiltered (file, "<cooked>");
+	  }
+      }
+  }
+};
+
 /* For "maint print registers".  */
 
 class register_dump_none : public register_dump
@@ -1633,22 +1675,19 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
     case regcache_dump_raw:
     case regcache_dump_cooked:
       {
-	regcache *reg;
+	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
 
 	if (target_has_registers)
-	  reg = get_current_regcache ();
+	  dump.reset (new register_dump_regcache (get_current_regcache (),
+						  dump_pseudo));
 	else
 	  {
 	    /* For the benefit of "maint print registers" & co when
 	       debugging an executable, allow dumping a regcache even when
 	       there is no thread selected / no registers.  */
-	    reg = new regcache (target_gdbarch ());
-	    regs.reset (reg);
+	    dump.reset (new register_dump_reg_buffer (target_gdbarch (),
+						      dump_pseudo));
 	  }
-
-	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
-
-	dump.reset (new register_dump_regcache (reg, dump_pseudo));
       }
       break;
     }
@@ -1937,7 +1976,7 @@ cooked_read_test (struct gdbarch *gdbarch)
       mock_target.reset ();
     }
 
-  regcache readonly (regcache::readonly, readwrite);
+  readonly_detached_regcache readonly (readwrite);
 
   /* GDB may go to target layer to fetch all registers and memory for
      readonly regcache.  */
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 0bca9f3..8a65d38 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -307,16 +307,6 @@ class readonly_detached_regcache;
 class regcache : public detached_regcache
 {
 public:
-  regcache (gdbarch *gdbarch)
-    : regcache (gdbarch, nullptr, true)
-  {}
-
-  struct readonly_t {};
-  static constexpr readonly_t readonly {};
-
-  /* Create a readonly regcache from a non-readonly regcache.  */
-  regcache (readonly_t, const regcache &src);
-
   DISABLE_COPY_AND_ASSIGN (regcache);
 
   /* Return REGCACHE's address space.  */
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 03/10] Remove regcache_save and regcache_cpy
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
  2018-02-07 10:33 ` [PATCH 10/10] Pass readable_regcache to gdbarch method read_pc Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-17  2:07   ` Simon Marchi
  2018-02-07 10:33 ` [PATCH 07/10] No longer create readonly regcache Yao Qi
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

... instead we start to use regcache methods save and restore.  It is
quite straightforward to replace regcache_save with regcache->save.

regcache_cpy has some asserts, some of them not necessary, like

 gdb_assert (src != dst);

because we already assert !m_readonly_p and src->m_readonly_p, so
src isn't dst.  Some of the asserts are moved to ::restore.

gdb:

2017-10-30  Yao Qi  <yao.qi@linaro.org>

	* frame.c (frame_save_as_regcache): Use regcache method save.
	(frame_pop): Use regcache method restore.
	* infrun.c (restore_infcall_suspend_state): Likewise.
	* linux-fork.c (fork_load_infrun_state): Likewise.
	* ppc-linux-tdep.c (ppu2spu_sniffer): User regcache method
	save.
	* regcache.c (regcache_save): Remove.
	(regcache::restore): More asserts.
	(regcache_cpy): Remove.
	* regcache.h (regcache_save): Remove the declaration.
	(regcache::restore): Move from private to public.
	Remove the friend declaration of regcache_cpy.
	(regcache_cpy): Remove declaration.
---
 gdb/frame.c          |  7 +++----
 gdb/infrun.c         |  2 +-
 gdb/linux-fork.c     |  2 +-
 gdb/ppc-linux-tdep.c |  2 +-
 gdb/regcache.c       | 22 ++++------------------
 gdb/regcache.h       | 30 +++++++++---------------------
 6 files changed, 19 insertions(+), 46 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 1384ecc..773fd04 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1023,7 +1023,7 @@ frame_save_as_regcache (struct frame_info *this_frame)
   std::unique_ptr<struct regcache> regcache
     (new struct regcache (get_frame_arch (this_frame)));
 
-  regcache_save (regcache.get (), do_frame_register_read, this_frame);
+  regcache->save (do_frame_register_read, this_frame);
   return regcache;
 }
 
@@ -1068,9 +1068,8 @@ frame_pop (struct frame_info *this_frame)
      Unfortunately, they don't implement it.  Their lack of a formal
      definition can lead to targets writing back bogus values
      (arguably a bug in the target code mind).  */
-  /* Now copy those saved registers into the current regcache.
-     Here, regcache_cpy() calls regcache_restore().  */
-  regcache_cpy (get_current_regcache (), scratch.get ());
+  /* Now copy those saved registers into the current regcache.  */
+  get_current_regcache ()->restore (scratch.get ());
 
   /* We've made right mess of GDB's local state, just discard
      everything.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 45fe36a..742d130 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8896,7 +8896,7 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
      (and perhaps other times).  */
   if (target_has_execution)
     /* NB: The register write goes through to the target.  */
-    regcache_cpy (regcache, inf_state->registers);
+    regcache->restore (inf_state->registers);
 
   discard_infcall_suspend_state (inf_state);
 }
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 8561157..df7ea4e 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -261,7 +261,7 @@ fork_load_infrun_state (struct fork_info *fp)
   linux_nat_switch_fork (fp->ptid);
 
   if (fp->savedregs && fp->clobber_regs)
-    regcache_cpy (get_current_regcache (), fp->savedregs);
+    get_current_regcache ()->restore (fp->savedregs);
 
   registers_changed ();
   reinit_frame_cache ();
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index ed0ea13..13a50d6 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1372,7 +1372,7 @@ ppu2spu_sniffer (const struct frame_unwind *self,
 	  std::unique_ptr<struct regcache> regcache
 	    (new struct regcache (data.gdbarch));
 
-	  regcache_save (regcache.get (), ppu2spu_unwind_register, &data);
+	  regcache->save (ppu2spu_unwind_register, &data);
 
 	  cache->frame_id = frame_id_build (base, func);
 	  cache->regcache = regcache.release ();
diff --git a/gdb/regcache.c b/gdb/regcache.c
index ad5e0a2..0df6a88 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -282,13 +282,6 @@ reg_buffer::register_buffer (int regnum) const
 }
 
 void
-regcache_save (struct regcache *regcache,
-	       regcache_cooked_read_ftype *cooked_read, void *src)
-{
-  regcache->save (cooked_read, src);
-}
-
-void
 regcache::save (regcache_cooked_read_ftype *cooked_read,
 		void *src)
 {
@@ -329,10 +322,14 @@ regcache::restore (struct regcache *src)
   struct gdbarch *gdbarch = m_descr->gdbarch;
   int regnum;
 
+  gdb_assert (src != NULL);
   /* The dst had better not be read-only.  If it is, the `restore'
      doesn't make much sense.  */
   gdb_assert (!m_readonly_p);
   gdb_assert (src->m_readonly_p);
+
+  gdb_assert (gdbarch == src->arch ());
+
   /* Copy over any registers, being careful to only restore those that
      were both saved and need to be restored.  The full [0 .. gdbarch_num_regs
      + gdbarch_num_pseudo_regs) range is checked since some architectures need
@@ -347,17 +344,6 @@ regcache::restore (struct regcache *src)
     }
 }
 
-void
-regcache_cpy (struct regcache *dst, struct regcache *src)
-{
-  gdb_assert (src != NULL && dst != NULL);
-  gdb_assert (src->m_descr->gdbarch == dst->m_descr->gdbarch);
-  gdb_assert (src != dst);
-  gdb_assert (src->m_readonly_p && !dst->m_readonly_p);
-
-  dst->restore (src);
-}
-
 struct regcache *
 regcache_dup (struct regcache *src)
 {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 1c7ee8c..e1ab2e7 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -198,20 +198,10 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum);
    
 extern int register_size (struct gdbarch *gdbarch, int regnum);
 
-
-/* Save/restore a register cache.  The set of registers saved /
-   restored into the DST regcache determined by the save_reggroup /
-   restore_reggroup respectively.  COOKED_READ returns zero iff the
-   register's value can't be returned.  */
-
 typedef enum register_status (regcache_cooked_read_ftype) (void *src,
 							   int regnum,
 							   gdb_byte *buf);
 
-extern void regcache_save (struct regcache *dst,
-			   regcache_cooked_read_ftype *cooked_read,
-			   void *cooked_read_context);
-
 enum regcache_dump_what
 {
   regcache_dump_none, regcache_dump_raw,
@@ -317,7 +307,14 @@ public:
     return m_aspace;
   }
 
+/* Save/restore a register cache.  The set of registers saved /
+   restored into the regcache determined by the save_reggroup /
+   restore_reggroup respectively.  COOKED_READ returns zero iff the
+   register's value can't be returned.  */
   void save (regcache_cooked_read_ftype *cooked_read, void *src);
+  /* Writes to regcache will go through to the target.  SRC is a
+     read-only register cache.  */
+  void restore (struct regcache *src);
 
   void cooked_write (int regnum, const gdb_byte *buf);
 
@@ -383,7 +380,6 @@ protected:
   static std::forward_list<regcache *> current_regcache;
 
 private:
-  void restore (struct regcache *src);
 
   void transfer_regset (const struct regset *regset,
 			struct regcache *out_regcache,
@@ -401,9 +397,8 @@ private:
   /* Is this a read-only cache?  A read-only cache is used for saving
      the target's register state (e.g, across an inferior function
      call or just before forcing a function return).  A read-only
-     cache can only be updated via the methods regcache_dup() and
-     regcache_cpy().  The actual contents are determined by the
-     reggroup_save and reggroup_restore methods.  */
+     cache can only be created via a constructor.  The actual contents
+     are determined by the save and restore methods.  */
   const bool m_readonly_p;
   /* If this is a read-write cache, which thread's registers is
      it connected to?  */
@@ -415,19 +410,12 @@ private:
 
   friend void
   registers_changed_ptid (ptid_t ptid);
-
-  friend void
-  regcache_cpy (struct regcache *dst, struct regcache *src);
 };
 
 /* Duplicate the contents of a register cache to a read-only register
    cache.  The operation is pass-through.  */
 extern struct regcache *regcache_dup (struct regcache *regcache);
 
-/* Writes to DEST will go through to the target.  SRC is a read-only
-   register cache.  */
-extern void regcache_cpy (struct regcache *dest, struct regcache *src);
-
 extern void registers_changed (void);
 extern void registers_changed_ptid (ptid_t);
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (7 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 06/10] Replace regcache::dump with class register_dump Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-17  3:50   ` Simon Marchi
  2018-02-07 10:33 ` [PATCH 05/10] Class detached_regcache Yao Qi
  2018-02-17  3:53 ` [PATCH 00/10 v2] Remove regcache::m_readonly_p Simon Marchi
  10 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

gdb:

2017-11-30  Yao Qi  <yao.qi@linaro.org>

	* Makefile.in (COMMON_SFILES): Add regcache-dump.c
	* regcache-dump.c: New file.
	* regcache.c: Move register_dump to regcache-dump.c.
	(maintenance_print_registers): Likewise.
	(maintenance_print_raw_registers): Likewise.
	(maintenance_print_cooked_registers): Likewise.
	(maintenance_print_register_groups): Likewise.
	(maintenance_print_remote_registers): Likewise.
	(_initialize_regcache): Likewise.
	* regcache.h (register_dump): Moved from regcache.c.
---
 gdb/Makefile.in     |   1 +
 gdb/regcache-dump.c | 335 ++++++++++++++++++++++++++++++++++
 gdb/regcache.c      | 505 +++++++++-------------------------------------------
 gdb/regcache.h      |  20 +++
 4 files changed, 445 insertions(+), 416 deletions(-)
 create mode 100644 gdb/regcache-dump.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0f87398..c85a94a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1043,6 +1043,7 @@ COMMON_SFILES = \
 	record-btrace.c \
 	record-full.c \
 	regcache.c \
+	regcache-dump.c \
 	reggroups.c \
 	registry.c \
 	remote.c \
diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
new file mode 100644
index 0000000..4780fc4
--- /dev/null
+++ b/gdb/regcache-dump.c
@@ -0,0 +1,335 @@
+/* Copyright (C) 1986-2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "gdbcmd.h"
+#include "regcache.h"
+#include "common/def-vector.h"
+#include "valprint.h"
+#include "remote.h"
+#include "reggroups.h"
+#include "target.h"
+
+/* Dump registers from regcache, used for dump raw registers and
+   cooked registers.  */
+
+class register_dump_regcache : public register_dump
+{
+public:
+  register_dump_regcache (regcache *regcache, bool dump_pseudo)
+    : register_dump (regcache->arch ()), m_regcache (regcache),
+      m_dump_pseudo (dump_pseudo)
+  {
+  }
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	if (m_dump_pseudo)
+	  fprintf_unfiltered (file, "Cooked value");
+	else
+	  fprintf_unfiltered (file, "Raw value");
+      }
+    else
+      {
+	if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
+	  {
+	    auto size = register_size (m_gdbarch, regnum);
+
+	    if (size == 0)
+	      return;
+
+	    gdb::def_vector<gdb_byte> buf (size);
+	    auto status = m_regcache->cooked_read (regnum, buf.data ());
+
+	    if (status == REG_UNKNOWN)
+	      fprintf_unfiltered (file, "<invalid>");
+	    else if (status == REG_UNAVAILABLE)
+	      fprintf_unfiltered (file, "<unavailable>");
+	    else
+	      {
+		print_hex_chars (file, buf.data (), size,
+				 gdbarch_byte_order (m_gdbarch), true);
+	      }
+	  }
+	else
+	  {
+	    /* Just print "<cooked>" for pseudo register when
+	       regcache_dump_raw.  */
+	    fprintf_unfiltered (file, "<cooked>");
+	  }
+      }
+  }
+
+private:
+  regcache *m_regcache;
+
+  /* Dump pseudo registers or not.  */
+  const bool m_dump_pseudo;
+};
+
+/* Dump from reg_buffer, used when there is no thread or
+   registers.  */
+
+class register_dump_reg_buffer : public register_dump, reg_buffer
+{
+public:
+  register_dump_reg_buffer (gdbarch *gdbarch, bool dump_pseudo)
+    : register_dump (gdbarch), reg_buffer (gdbarch, dump_pseudo)
+  {
+  }
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	if (m_has_pseudo)
+	  fprintf_unfiltered (file, "Cooked value");
+	else
+	  fprintf_unfiltered (file, "Raw value");
+      }
+    else
+      {
+	if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
+	  {
+	    auto size = register_size (m_gdbarch, regnum);
+
+	    if (size == 0)
+	      return;
+
+	    auto status = get_register_status (regnum);
+
+	    gdb_assert (status != REG_VALID);
+
+	    if (status == REG_UNKNOWN)
+	      fprintf_unfiltered (file, "<invalid>");
+	    else
+	      fprintf_unfiltered (file, "<unavailable>");
+	  }
+	else
+	  {
+	    /* Just print "<cooked>" for pseudo register when
+	       regcache_dump_raw.  */
+	    fprintf_unfiltered (file, "<cooked>");
+	  }
+      }
+  }
+};
+
+/* For "maint print registers".  */
+
+class register_dump_none : public register_dump
+{
+public:
+  register_dump_none (gdbarch *arch)
+    : register_dump (arch)
+  {}
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {}
+};
+
+/* For "maint print remote-registers".  */
+
+class register_dump_remote : public register_dump
+{
+public:
+  register_dump_remote (gdbarch *arch)
+    : register_dump (arch)
+  {}
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      {
+	fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
+      }
+    else if (regnum < gdbarch_num_regs (m_gdbarch))
+      {
+	int pnum, poffset;
+
+	if (remote_register_number_and_offset (m_gdbarch, regnum,
+					       &pnum, &poffset))
+	  fprintf_unfiltered (file, "%7d %11d", pnum, poffset);
+      }
+  }
+};
+
+/* For "maint print register-groups".  */
+
+class register_dump_groups : public register_dump
+{
+public:
+  register_dump_groups (gdbarch *arch)
+    : register_dump (arch)
+  {}
+
+protected:
+  void dump_reg (ui_file *file, int regnum) override
+  {
+    if (regnum < 0)
+      fprintf_unfiltered (file, "Groups");
+    else
+      {
+	const char *sep = "";
+	struct reggroup *group;
+
+	for (group = reggroup_next (m_gdbarch, NULL);
+	     group != NULL;
+	     group = reggroup_next (m_gdbarch, group))
+	  {
+	    if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
+	      {
+		fprintf_unfiltered (file,
+				    "%s%s", sep, reggroup_name (group));
+		sep = ",";
+	      }
+	  }
+      }
+  }
+};
+
+enum regcache_dump_what
+{
+  regcache_dump_none, regcache_dump_raw,
+  regcache_dump_cooked, regcache_dump_groups,
+  regcache_dump_remote
+};
+
+static void
+regcache_print (const char *args, enum regcache_dump_what what_to_dump)
+{
+  /* Where to send output.  */
+  stdio_file file;
+  ui_file *out;
+
+  if (args == NULL)
+    out = gdb_stdout;
+  else
+    {
+      if (!file.open (args, "w"))
+	perror_with_name (_("maintenance print architecture"));
+      out = &file;
+    }
+
+  std::unique_ptr<register_dump> dump;
+  std::unique_ptr<regcache> regs;
+  gdbarch *gdbarch;
+
+  if (target_has_registers)
+    gdbarch = get_current_regcache ()->arch ();
+  else
+    gdbarch = target_gdbarch ();
+
+  switch (what_to_dump)
+    {
+    case regcache_dump_none:
+      dump.reset (new register_dump_none (gdbarch));
+      break;
+    case regcache_dump_remote:
+      dump.reset (new register_dump_remote (gdbarch));
+      break;
+    case regcache_dump_groups:
+      dump.reset (new register_dump_groups (gdbarch));
+      break;
+    case regcache_dump_raw:
+    case regcache_dump_cooked:
+      {
+	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
+
+	if (target_has_registers)
+	  dump.reset (new register_dump_regcache (get_current_regcache (),
+						  dump_pseudo));
+	else
+	  {
+	    /* For the benefit of "maint print registers" & co when
+	       debugging an executable, allow dumping a regcache even when
+	       there is no thread selected / no registers.  */
+	    dump.reset (new register_dump_reg_buffer (target_gdbarch (),
+						      dump_pseudo));
+	  }
+      }
+      break;
+    }
+
+  dump->dump (out);
+}
+
+static void
+maintenance_print_registers (const char *args, int from_tty)
+{
+  regcache_print (args, regcache_dump_none);
+}
+
+static void
+maintenance_print_raw_registers (const char *args, int from_tty)
+{
+  regcache_print (args, regcache_dump_raw);
+}
+
+static void
+maintenance_print_cooked_registers (const char *args, int from_tty)
+{
+  regcache_print (args, regcache_dump_cooked);
+}
+
+static void
+maintenance_print_register_groups (const char *args, int from_tty)
+{
+  regcache_print (args, regcache_dump_groups);
+}
+
+static void
+maintenance_print_remote_registers (const char *args, int from_tty)
+{
+  regcache_print (args, regcache_dump_remote);
+}
+
+void
+_initialize_regcache_dump (void)
+{
+  add_cmd ("registers", class_maintenance, maintenance_print_registers,
+	   _("Print the internal register configuration.\n"
+	     "Takes an optional file parameter."), &maintenanceprintlist);
+  add_cmd ("raw-registers", class_maintenance,
+	   maintenance_print_raw_registers,
+	   _("Print the internal register configuration "
+	     "including raw values.\n"
+	     "Takes an optional file parameter."), &maintenanceprintlist);
+  add_cmd ("cooked-registers", class_maintenance,
+	   maintenance_print_cooked_registers,
+	   _("Print the internal register configuration "
+	     "including cooked values.\n"
+	     "Takes an optional file parameter."), &maintenanceprintlist);
+  add_cmd ("register-groups", class_maintenance,
+	   maintenance_print_register_groups,
+	   _("Print the internal register configuration "
+	     "including each register's group.\n"
+	     "Takes an optional file parameter."),
+	   &maintenanceprintlist);
+  add_cmd ("remote-registers", class_maintenance,
+	   maintenance_print_remote_registers, _("\
+Print the internal register configuration including each register's\n\
+remote register number and buffer offset in the g/G packets.\n\
+Takes an optional file parameter."),
+	   &maintenanceprintlist);
+}
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 082b62e..d99a624 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -25,8 +25,6 @@
 #include "regcache.h"
 #include "reggroups.h"
 #include "observer.h"
-#include "remote.h"
-#include "valprint.h"
 #include "regset.h"
 #include <forward_list>
 
@@ -1300,421 +1298,122 @@ reg_flush_command (const char *command, int from_tty)
     printf_filtered (_("Register cache flushed.\n"));
 }
 
-/* An abstract base class for register dump.  */
-
-class register_dump
+void register_dump::dump (ui_file *file)
 {
-public:
-  void dump (ui_file *file)
-  {
-    auto descr = regcache_descr (m_gdbarch);
-    int regnum;
-    int footnote_nr = 0;
-    int footnote_register_offset = 0;
-    int footnote_register_type_name_null = 0;
-    long register_offset = 0;
-
-    gdb_assert (descr->nr_cooked_registers
-		== (gdbarch_num_regs (m_gdbarch)
-		    + gdbarch_num_pseudo_regs (m_gdbarch)));
-
-    for (regnum = -1; regnum < descr->nr_cooked_registers; regnum++)
-      {
-	/* Name.  */
-	if (regnum < 0)
-	  fprintf_unfiltered (file, " %-10s", "Name");
-	else
-	  {
-	    const char *p = gdbarch_register_name (m_gdbarch, regnum);
+  auto descr = regcache_descr (m_gdbarch);
+  int regnum;
+  int footnote_nr = 0;
+  int footnote_register_offset = 0;
+  int footnote_register_type_name_null = 0;
+  long register_offset = 0;
 
-	    if (p == NULL)
-	      p = "";
-	    else if (p[0] == '\0')
-	      p = "''";
-	    fprintf_unfiltered (file, " %-10s", p);
-	  }
+  gdb_assert (descr->nr_cooked_registers
+	      == (gdbarch_num_regs (m_gdbarch)
+		  + gdbarch_num_pseudo_regs (m_gdbarch)));
 
-	/* Number.  */
-	if (regnum < 0)
-	  fprintf_unfiltered (file, " %4s", "Nr");
-	else
-	  fprintf_unfiltered (file, " %4d", regnum);
+  for (regnum = -1; regnum < descr->nr_cooked_registers; regnum++)
+    {
+      /* Name.  */
+      if (regnum < 0)
+	fprintf_unfiltered (file, " %-10s", "Name");
+      else
+	{
+	  const char *p = gdbarch_register_name (m_gdbarch, regnum);
 
-	/* Relative number.  */
-	if (regnum < 0)
-	  fprintf_unfiltered (file, " %4s", "Rel");
-	else if (regnum < gdbarch_num_regs (m_gdbarch))
-	  fprintf_unfiltered (file, " %4d", regnum);
-	else
-	  fprintf_unfiltered (file, " %4d",
-			      (regnum - gdbarch_num_regs (m_gdbarch)));
+	  if (p == NULL)
+	    p = "";
+	  else if (p[0] == '\0')
+	    p = "''";
+	  fprintf_unfiltered (file, " %-10s", p);
+	}
 
-	/* Offset.  */
-	if (regnum < 0)
-	  fprintf_unfiltered (file, " %6s  ", "Offset");
-	else
-	  {
-	    fprintf_unfiltered (file, " %6ld",
-				descr->register_offset[regnum]);
-	    if (register_offset != descr->register_offset[regnum]
-		|| (regnum > 0
-		    && (descr->register_offset[regnum]
-			!= (descr->register_offset[regnum - 1]
-			    + descr->sizeof_register[regnum - 1])))
-		)
-	      {
-		if (!footnote_register_offset)
-		  footnote_register_offset = ++footnote_nr;
-		fprintf_unfiltered (file, "*%d", footnote_register_offset);
-	      }
-	    else
-	      fprintf_unfiltered (file, "  ");
-	    register_offset = (descr->register_offset[regnum]
-			       + descr->sizeof_register[regnum]);
-	  }
+      /* Number.  */
+      if (regnum < 0)
+	fprintf_unfiltered (file, " %4s", "Nr");
+      else
+	fprintf_unfiltered (file, " %4d", regnum);
 
-	/* Size.  */
-	if (regnum < 0)
-	  fprintf_unfiltered (file, " %5s ", "Size");
-	else
-	  fprintf_unfiltered (file, " %5ld", descr->sizeof_register[regnum]);
+      /* Relative number.  */
+      if (regnum < 0)
+	fprintf_unfiltered (file, " %4s", "Rel");
+      else if (regnum < gdbarch_num_regs (m_gdbarch))
+	fprintf_unfiltered (file, " %4d", regnum);
+      else
+	fprintf_unfiltered (file, " %4d",
+			    (regnum - gdbarch_num_regs (m_gdbarch)));
 
-	/* Type.  */
+      /* Offset.  */
+      if (regnum < 0)
+	fprintf_unfiltered (file, " %6s  ", "Offset");
+      else
 	{
-	  const char *t;
-	  std::string name_holder;
-
-	  if (regnum < 0)
-	    t = "Type";
-	  else
+	  fprintf_unfiltered (file, " %6ld",
+			      descr->register_offset[regnum]);
+	  if (register_offset != descr->register_offset[regnum]
+	      || (regnum > 0
+		  && (descr->register_offset[regnum]
+		      != (descr->register_offset[regnum - 1]
+			  + descr->sizeof_register[regnum - 1])))
+	      )
 	    {
-	      static const char blt[] = "builtin_type";
-
-	      t = TYPE_NAME (register_type (m_gdbarch, regnum));
-	      if (t == NULL)
-		{
-		  if (!footnote_register_type_name_null)
-		    footnote_register_type_name_null = ++footnote_nr;
-		  name_holder = string_printf ("*%d",
-					       footnote_register_type_name_null);
-		  t = name_holder.c_str ();
-		}
-	      /* Chop a leading builtin_type.  */
-	      if (startswith (t, blt))
-		t += strlen (blt);
+	      if (!footnote_register_offset)
+		footnote_register_offset = ++footnote_nr;
+	      fprintf_unfiltered (file, "*%d", footnote_register_offset);
 	    }
-	  fprintf_unfiltered (file, " %-15s", t);
+	  else
+	    fprintf_unfiltered (file, "  ");
+	  register_offset = (descr->register_offset[regnum]
+			     + descr->sizeof_register[regnum]);
 	}
 
-	/* Leading space always present.  */
-	fprintf_unfiltered (file, " ");
-
-	dump_reg (file, regnum);
-
-	fprintf_unfiltered (file, "\n");
-      }
-
-    if (footnote_register_offset)
-      fprintf_unfiltered (file, "*%d: Inconsistent register offsets.\n",
-			  footnote_register_offset);
-    if (footnote_register_type_name_null)
-      fprintf_unfiltered (file,
-			  "*%d: Register type's name NULL.\n",
-			  footnote_register_type_name_null);
-  }
-
-  virtual ~register_dump () {};
-
-protected:
-  register_dump (gdbarch *arch)
-    : m_gdbarch (arch)
-  {}
-
-  /* Dump the register REGNUM contents.  If REGNUM is -1, print the
-     header.  */
-  virtual void dump_reg (ui_file *file, int regnum) = 0;
-
-  gdbarch *m_gdbarch;
-};
-
-/* Dump registers from regcache, used for dump raw registers and
-   cooked registers.  */
-
-class register_dump_regcache : public register_dump
-{
-public:
-  register_dump_regcache (regcache *regcache, bool dump_pseudo)
-    : register_dump (regcache->arch ()), m_regcache (regcache),
-      m_dump_pseudo (dump_pseudo)
-  {
-  }
-
-protected:
-  void dump_reg (ui_file *file, int regnum) override
-  {
-    if (regnum < 0)
-      {
-	if (m_dump_pseudo)
-	  fprintf_unfiltered (file, "Cooked value");
-	else
-	  fprintf_unfiltered (file, "Raw value");
-      }
-    else
-      {
-	if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo)
-	  {
-	    auto size = register_size (m_gdbarch, regnum);
-
-	    if (size == 0)
-	      return;
-
-	    gdb::def_vector<gdb_byte> buf (size);
-	    auto status = m_regcache->cooked_read (regnum, buf.data ());
-
-	    if (status == REG_UNKNOWN)
-	      fprintf_unfiltered (file, "<invalid>");
-	    else if (status == REG_UNAVAILABLE)
-	      fprintf_unfiltered (file, "<unavailable>");
-	    else
-	      {
-		print_hex_chars (file, buf.data (), size,
-				 gdbarch_byte_order (m_gdbarch), true);
-	      }
-	  }
-	else
-	  {
-	    /* Just print "<cooked>" for pseudo register when
-	       regcache_dump_raw.  */
-	    fprintf_unfiltered (file, "<cooked>");
-	  }
-      }
-  }
-
-private:
-  regcache *m_regcache;
-
-  /* Dump pseudo registers or not.  */
-  const bool m_dump_pseudo;
-};
-
-/* Dump from reg_buffer, used when there is no thread or
-   registers.  */
-
-class register_dump_reg_buffer : public register_dump, reg_buffer
-{
-public:
-  register_dump_reg_buffer (gdbarch *gdbarch, bool dump_pseudo)
-    : register_dump (gdbarch), reg_buffer (gdbarch, dump_pseudo)
-  {
-  }
+      /* Size.  */
+      if (regnum < 0)
+	fprintf_unfiltered (file, " %5s ", "Size");
+      else
+	fprintf_unfiltered (file, " %5ld", descr->sizeof_register[regnum]);
 
-protected:
-  void dump_reg (ui_file *file, int regnum) override
-  {
-    if (regnum < 0)
-      {
-	if (m_has_pseudo)
-	  fprintf_unfiltered (file, "Cooked value");
-	else
-	  fprintf_unfiltered (file, "Raw value");
-      }
-    else
+      /* Type.  */
       {
-	if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo)
-	  {
-	    auto size = register_size (m_gdbarch, regnum);
-
-	    if (size == 0)
-	      return;
-
-	    auto status = get_register_status (regnum);
+	const char *t;
+	std::string name_holder;
 
-	    gdb_assert (status != REG_VALID);
-
-	    if (status == REG_UNKNOWN)
-	      fprintf_unfiltered (file, "<invalid>");
-	    else
-	      fprintf_unfiltered (file, "<unavailable>");
-	  }
+	if (regnum < 0)
+	  t = "Type";
 	else
 	  {
-	    /* Just print "<cooked>" for pseudo register when
-	       regcache_dump_raw.  */
-	    fprintf_unfiltered (file, "<cooked>");
-	  }
-      }
-  }
-};
-
-/* For "maint print registers".  */
-
-class register_dump_none : public register_dump
-{
-public:
-  register_dump_none (gdbarch *arch)
-    : register_dump (arch)
-  {}
-
-protected:
-  void dump_reg (ui_file *file, int regnum) override
-  {}
-};
-
-/* For "maint print remote-registers".  */
-
-class register_dump_remote : public register_dump
-{
-public:
-  register_dump_remote (gdbarch *arch)
-    : register_dump (arch)
-  {}
+	    static const char blt[] = "builtin_type";
 
-protected:
-  void dump_reg (ui_file *file, int regnum) override
-  {
-    if (regnum < 0)
-      {
-	fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
-      }
-    else if (regnum < gdbarch_num_regs (m_gdbarch))
-      {
-	int pnum, poffset;
-
-	if (remote_register_number_and_offset (m_gdbarch, regnum,
-					       &pnum, &poffset))
-	  fprintf_unfiltered (file, "%7d %11d", pnum, poffset);
-      }
-  }
-};
-
-/* For "maint print register-groups".  */
-
-class register_dump_groups : public register_dump
-{
-public:
-  register_dump_groups (gdbarch *arch)
-    : register_dump (arch)
-  {}
-
-protected:
-  void dump_reg (ui_file *file, int regnum) override
-  {
-    if (regnum < 0)
-      fprintf_unfiltered (file, "Groups");
-    else
-      {
-	const char *sep = "";
-	struct reggroup *group;
-
-	for (group = reggroup_next (m_gdbarch, NULL);
-	     group != NULL;
-	     group = reggroup_next (m_gdbarch, group))
-	  {
-	    if (gdbarch_register_reggroup_p (m_gdbarch, regnum, group))
+	    t = TYPE_NAME (register_type (m_gdbarch, regnum));
+	    if (t == NULL)
 	      {
-		fprintf_unfiltered (file,
-				    "%s%s", sep, reggroup_name (group));
-		sep = ",";
+		if (!footnote_register_type_name_null)
+		  footnote_register_type_name_null = ++footnote_nr;
+		name_holder = string_printf ("*%d",
+					     footnote_register_type_name_null);
+		t = name_holder.c_str ();
 	      }
+	    /* Chop a leading builtin_type.  */
+	    if (startswith (t, blt))
+	      t += strlen (blt);
 	  }
+	fprintf_unfiltered (file, " %-15s", t);
       }
-  }
-};
 
-enum regcache_dump_what
-{
-  regcache_dump_none, regcache_dump_raw,
-  regcache_dump_cooked, regcache_dump_groups,
-  regcache_dump_remote
-};
+      /* Leading space always present.  */
+      fprintf_unfiltered (file, " ");
 
-static void
-regcache_print (const char *args, enum regcache_dump_what what_to_dump)
-{
-  /* Where to send output.  */
-  stdio_file file;
-  ui_file *out;
+      dump_reg (file, regnum);
 
-  if (args == NULL)
-    out = gdb_stdout;
-  else
-    {
-      if (!file.open (args, "w"))
-	perror_with_name (_("maintenance print architecture"));
-      out = &file;
+      fprintf_unfiltered (file, "\n");
     }
 
-  std::unique_ptr<register_dump> dump;
-  std::unique_ptr<regcache> regs;
-  gdbarch *gdbarch;
-
-  if (target_has_registers)
-    gdbarch = get_current_regcache ()->arch ();
-  else
-    gdbarch = target_gdbarch ();
-
-  switch (what_to_dump)
-    {
-    case regcache_dump_none:
-      dump.reset (new register_dump_none (gdbarch));
-      break;
-    case regcache_dump_remote:
-      dump.reset (new register_dump_remote (gdbarch));
-      break;
-    case regcache_dump_groups:
-      dump.reset (new register_dump_groups (gdbarch));
-      break;
-    case regcache_dump_raw:
-    case regcache_dump_cooked:
-      {
-	auto dump_pseudo = (what_to_dump == regcache_dump_cooked);
-
-	if (target_has_registers)
-	  dump.reset (new register_dump_regcache (get_current_regcache (),
-						  dump_pseudo));
-	else
-	  {
-	    /* For the benefit of "maint print registers" & co when
-	       debugging an executable, allow dumping a regcache even when
-	       there is no thread selected / no registers.  */
-	    dump.reset (new register_dump_reg_buffer (target_gdbarch (),
-						      dump_pseudo));
-	  }
-      }
-      break;
-    }
-
-  dump->dump (out);
-}
-
-static void
-maintenance_print_registers (const char *args, int from_tty)
-{
-  regcache_print (args, regcache_dump_none);
-}
-
-static void
-maintenance_print_raw_registers (const char *args, int from_tty)
-{
-  regcache_print (args, regcache_dump_raw);
-}
-
-static void
-maintenance_print_cooked_registers (const char *args, int from_tty)
-{
-  regcache_print (args, regcache_dump_cooked);
-}
-
-static void
-maintenance_print_register_groups (const char *args, int from_tty)
-{
-  regcache_print (args, regcache_dump_groups);
-}
-
-static void
-maintenance_print_remote_registers (const char *args, int from_tty)
-{
-  regcache_print (args, regcache_dump_remote);
+  if (footnote_register_offset)
+    fprintf_unfiltered (file, "*%d: Inconsistent register offsets.\n",
+			footnote_register_offset);
+  if (footnote_register_type_name_null)
+    fprintf_unfiltered (file,
+			"*%d: Register type's name NULL.\n",
+			footnote_register_type_name_null);
 }
 
 #if GDB_SELF_TEST
@@ -2175,32 +1874,6 @@ _initialize_regcache (void)
   add_com ("flushregs", class_maintenance, reg_flush_command,
 	   _("Force gdb to flush its register cache (maintainer command)"));
 
-  add_cmd ("registers", class_maintenance, maintenance_print_registers,
-	   _("Print the internal register configuration.\n"
-	     "Takes an optional file parameter."), &maintenanceprintlist);
-  add_cmd ("raw-registers", class_maintenance,
-	   maintenance_print_raw_registers,
-	   _("Print the internal register configuration "
-	     "including raw values.\n"
-	     "Takes an optional file parameter."), &maintenanceprintlist);
-  add_cmd ("cooked-registers", class_maintenance,
-	   maintenance_print_cooked_registers,
-	   _("Print the internal register configuration "
-	     "including cooked values.\n"
-	     "Takes an optional file parameter."), &maintenanceprintlist);
-  add_cmd ("register-groups", class_maintenance,
-	   maintenance_print_register_groups,
-	   _("Print the internal register configuration "
-	     "including each register's group.\n"
-	     "Takes an optional file parameter."),
-	   &maintenanceprintlist);
-  add_cmd ("remote-registers", class_maintenance,
-	   maintenance_print_remote_registers, _("\
-Print the internal register configuration including each register's\n\
-remote register number and buffer offset in the g/G packets.\n\
-Takes an optional file parameter."),
-	   &maintenanceprintlist);
-
 #if GDB_SELF_TEST
   selftests::register_test ("current_regcache", selftests::current_regcache_test);
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 179eed7..ba151ea 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -427,4 +427,24 @@ public:
 extern void registers_changed (void);
 extern void registers_changed_ptid (ptid_t);
 
+/* An abstract base class for register dump.  */
+
+class register_dump
+{
+public:
+  void dump (ui_file *file);
+  virtual ~register_dump () {};
+
+protected:
+  register_dump (gdbarch *arch)
+    : m_gdbarch (arch)
+  {}
+
+  /* Dump the register REGNUM contents.  If REGNUM is -1, print the
+     header.  */
+  virtual void dump_reg (ui_file *file, int regnum) = 0;
+
+  gdbarch *m_gdbarch;
+};
+
 #endif /* REGCACHE_H */
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 08/10] Remove regcache::m_readonly_p
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (4 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 02/10] class readable_regcache and pass readable_regcache to gdbarch pseudo_register_read and pseudo_register_read_value Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 04/10] Class readonly_detached_regcache Yao Qi
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

Now, m_readonly_p is always false, so we can remove it, and regcache no
longer includes pseudo registers.  Some regcache methods are lift up to
its parent class, like reg_buffer or detached_regcache.

gdb:

2017-11-07  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (regcache::regcache): Update.
	(regcache::invalidate): Move it to detached_regcache::invalidate.
	(get_thread_arch_aspace_regcache): Update.
	(regcache::raw_update): Update.
	(regcache::cooked_read): Remove some code.
	(regcache::cooked_read_value): Likewise.
	(regcache::raw_write): Remove assert on m_readonly_p.
	(regcache::raw_supply_integer): Move it to
	detached_regcache::raw_supply_integer.
	(regcache::raw_supply_zeroed): Likewise.
	* regcache.h (detached_regcache) <raw_supply_integer>: New
	declaration.
	<raw_supply_zeroed, invalidate>: Likewise.
	(regcache) <raw_supply_integer, raw_supply_zeroed>: Removed.
	<invalidate>: Likewise.
	<m_readonly_p>: Removed.
---
 gdb/regcache.c | 30 +++++++++++-------------------
 gdb/regcache.h | 22 ++++++++--------------
 2 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8f81163..082b62e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -200,13 +200,10 @@ reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo)
     }
 }
 
-regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
-		    bool readonly_p_)
-/* The register buffers.  A read-only register cache can hold the
-   full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a
-   read/write register cache can only hold [0 .. gdbarch_num_regs).  */
-  : detached_regcache (gdbarch, readonly_p_),
-    m_aspace (aspace_), m_readonly_p (readonly_p_)
+regcache::regcache (gdbarch *gdbarch, const address_space *aspace_)
+/* The register buffers.  A read/write register cache can only hold
+   [0 .. gdbarch_num_regs).  */
+  : detached_regcache (gdbarch, false), m_aspace (aspace_)
 {
   m_ptid = minus_one_ptid;
 }
@@ -319,7 +316,6 @@ regcache::restore (readonly_detached_regcache *src)
   int regnum;
 
   gdb_assert (src != NULL);
-  gdb_assert (!m_readonly_p);
   gdb_assert (src->m_has_pseudo);
 
   gdb_assert (gdbarch == src->arch ());
@@ -361,9 +357,8 @@ regcache_invalidate (struct regcache *regcache, int regnum)
 }
 
 void
-regcache::invalidate (int regnum)
+detached_regcache::invalidate (int regnum)
 {
-  gdb_assert (!m_readonly_p);
   assert_regnum (regnum);
   m_register_status[regnum] = REG_UNKNOWN;
 }
@@ -394,7 +389,7 @@ get_thread_arch_aspace_regcache (ptid_t ptid, struct gdbarch *gdbarch,
     if (ptid_equal (regcache->ptid (), ptid) && regcache->arch () == gdbarch)
       return regcache;
 
-  regcache *new_regcache = new regcache (gdbarch, aspace, false);
+  regcache *new_regcache = new regcache (gdbarch, aspace);
 
   regcache::current_regcache.push_front (new_regcache);
   new_regcache->set_ptid (ptid);
@@ -532,7 +527,7 @@ regcache::raw_update (int regnum)
      only there is still only one target side register cache.  Sigh!
      On the bright side, at least there is a regcache object.  */
 
-  if (!m_readonly_p && get_register_status (regnum) == REG_UNKNOWN)
+  if (get_register_status (regnum) == REG_UNKNOWN)
     {
       target_fetch_registers (this, regnum);
 
@@ -805,7 +800,6 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
 
   gdb_assert (buf != NULL);
   assert_regnum (regnum);
-  gdb_assert (!m_readonly_p);
 
   /* On the sparc, writing %g0 is a no-op, so we don't even want to
      change the registers array if something writes to this register.  */
@@ -1028,15 +1022,14 @@ detached_regcache::raw_supply (int regnum, const void *buf)
    most significant bytes of the integer will be truncated.  */
 
 void
-regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
-			      bool is_signed)
+detached_regcache::raw_supply_integer (int regnum, const gdb_byte *addr,
+				   int addr_len, bool is_signed)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
   gdb_byte *regbuf;
   size_t regsize;
 
   assert_regnum (regnum);
-  gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
   regsize = m_descr->sizeof_register[regnum];
@@ -1051,13 +1044,12 @@ regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
    unavailable).  */
 
 void
-regcache::raw_supply_zeroed (int regnum)
+detached_regcache::raw_supply_zeroed (int regnum)
 {
   void *regbuf;
   size_t size;
 
   assert_regnum (regnum);
-  gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
   size = m_descr->sizeof_register[regnum];
@@ -1871,7 +1863,7 @@ class readwrite_regcache : public regcache
 {
 public:
   readwrite_regcache (struct gdbarch *gdbarch)
-    : regcache (gdbarch, nullptr, false)
+    : regcache (gdbarch, nullptr)
   {}
 };
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 8a65d38..179eed7 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -297,6 +297,13 @@ public:
   void raw_update (int regnum) override
   {}
 
+  void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
+			   bool is_signed);
+
+  void raw_supply_zeroed (int regnum);
+
+  void invalidate (int regnum);
+
   DISABLE_COPY_AND_ASSIGN (detached_regcache);
 };
 
@@ -338,13 +345,6 @@ public:
   void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
 			    bool is_signed) const;
 
-  void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
-			   bool is_signed);
-
-  void raw_supply_zeroed (int regnum);
-
-  void invalidate (int regnum);
-
   void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
 
   void cooked_write_part (int regnum, int offset, int len,
@@ -373,7 +373,7 @@ public:
 
   static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
 protected:
-  regcache (gdbarch *gdbarch, const address_space *aspace_, bool readonly_p_);
+  regcache (gdbarch *gdbarch, const address_space *aspace_);
   static std::forward_list<regcache *> current_regcache;
 
 private:
@@ -391,12 +391,6 @@ private:
      makes sense, like PC or SP).  */
   const address_space * const m_aspace;
 
-  /* Is this a read-only cache?  A read-only cache is used for saving
-     the target's register state (e.g, across an inferior function
-     call or just before forcing a function return).  A read-only
-     cache can only be created via a constructor.  The actual contents
-     are determined by the save and restore methods.  */
-  const bool m_readonly_p;
   /* If this is a read-write cache, which thread's registers is
      it connected to?  */
   ptid_t m_ptid;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 10/10] Pass readable_regcache to gdbarch method read_pc
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 03/10] Remove regcache_save and regcache_cpy Yao Qi
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

We can pass readable_regcache to gdbarch method read_pc where it is
allowed to do read from regcache.

gdb:

2018-01-30  Yao Qi  <yao.qi@linaro.org>

	* avr-tdep.c (avr_read_pc): Change parameter type to
	readable_regcache.
	* gdbarch.sh (read_pc): Likewise.
	* gdbarch.c: Re-generated.
	* gdbarch.h: Re-generated.
	* hppa-tdep.c (hppa_read_pc): Change parameter type to
	readable_regcache.
	* ia64-tdep.c (ia64_read_pc): Likewise.
	* mips-tdep.c (mips_read_pc): Likewise.
	* spu-tdep.c (spu_read_pc): Likewise.
---
 gdb/avr-tdep.c  | 5 +++--
 gdb/gdbarch.c   | 2 +-
 gdb/gdbarch.h   | 4 ++--
 gdb/gdbarch.sh  | 2 +-
 gdb/hppa-tdep.c | 6 +++---
 gdb/ia64-tdep.c | 6 +++---
 gdb/mips-tdep.c | 4 ++--
 gdb/spu-tdep.c  | 5 +++--
 8 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 7f88e8f..c44a3aa 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -367,10 +367,11 @@ avr_integer_to_address (struct gdbarch *gdbarch,
 }
 
 static CORE_ADDR
-avr_read_pc (struct regcache *regcache)
+avr_read_pc (readable_regcache *regcache)
 {
   ULONGEST pc;
-  regcache_cooked_read_unsigned (regcache, AVR_PC_REGNUM, &pc);
+
+  regcache->cooked_read (AVR_PC_REGNUM, &pc);
   return avr_make_iaddr (pc);
 }
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 705a6d3..62cb9a5 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -1906,7 +1906,7 @@ gdbarch_read_pc_p (struct gdbarch *gdbarch)
 }
 
 CORE_ADDR
-gdbarch_read_pc (struct gdbarch *gdbarch, struct regcache *regcache)
+gdbarch_read_pc (struct gdbarch *gdbarch, readable_regcache *regcache)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->read_pc != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 249ad61..30c2bf3 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -245,8 +245,8 @@ extern void set_gdbarch_char_signed (struct gdbarch *gdbarch, int char_signed);
 
 extern int gdbarch_read_pc_p (struct gdbarch *gdbarch);
 
-typedef CORE_ADDR (gdbarch_read_pc_ftype) (struct regcache *regcache);
-extern CORE_ADDR gdbarch_read_pc (struct gdbarch *gdbarch, struct regcache *regcache);
+typedef CORE_ADDR (gdbarch_read_pc_ftype) (readable_regcache *regcache);
+extern CORE_ADDR gdbarch_read_pc (struct gdbarch *gdbarch, readable_regcache *regcache);
 extern void set_gdbarch_read_pc (struct gdbarch *gdbarch, gdbarch_read_pc_ftype *read_pc);
 
 extern int gdbarch_write_pc_p (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 8477dd7..10a2aa9 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -422,7 +422,7 @@ v;int;dwarf2_addr_size;;;sizeof (void*);0;gdbarch_ptr_bit (gdbarch) / TARGET_CHA
 # One if \`char' acts like \`signed char', zero if \`unsigned char'.
 v;int;char_signed;;;1;-1;1
 #
-F;CORE_ADDR;read_pc;struct regcache *regcache;regcache
+F;CORE_ADDR;read_pc;readable_regcache *regcache;regcache
 F;void;write_pc;struct regcache *regcache, CORE_ADDR val;regcache, val
 # Function for getting target's idea of a frame pointer.  FIXME: GDB's
 # whole scheme for dealing with "frames" and "frame pointers" needs a
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index cc9434e..84dbd66 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -1304,13 +1304,13 @@ hppa64_frame_align (struct gdbarch *gdbarch, CORE_ADDR addr)
 }
 
 CORE_ADDR
-hppa_read_pc (struct regcache *regcache)
+hppa_read_pc (readable_regcache *regcache)
 {
   ULONGEST ipsw;
   ULONGEST pc;
 
-  regcache_cooked_read_unsigned (regcache, HPPA_IPSW_REGNUM, &ipsw);
-  regcache_cooked_read_unsigned (regcache, HPPA_PCOQ_HEAD_REGNUM, &pc);
+  regcache->cooked_read (HPPA_IPSW_REGNUM, &ipsw);
+  regcache->cooked_read (HPPA_PCOQ_HEAD_REGNUM, &pc);
 
   /* If the current instruction is nullified, then we are effectively
      still executing the previous instruction.  Pretend we are still
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 6c9b341..4f02f05 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -876,13 +876,13 @@ ia64_breakpoint_from_pc (struct gdbarch *gdbarch,
 }
 
 static CORE_ADDR
-ia64_read_pc (struct regcache *regcache)
+ia64_read_pc (readable_regcache *regcache)
 {
   ULONGEST psr_value, pc_value;
   int slot_num;
 
-  regcache_cooked_read_unsigned (regcache, IA64_PSR_REGNUM, &psr_value);
-  regcache_cooked_read_unsigned (regcache, IA64_IP_REGNUM, &pc_value);
+  regcache->cooked_read (IA64_PSR_REGNUM, &psr_value);
+  regcache->cooked_read (IA64_IP_REGNUM, &pc_value);
   slot_num = (psr_value >> 41) & 3;
 
   return pc_value | (slot_num * SLOT_MULTIPLIER);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 2c1a8f0..05f27a5 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -1362,12 +1362,12 @@ mips_in_frame_stub (CORE_ADDR pc)
    all registers should be sign extended for simplicity?  */
 
 static CORE_ADDR
-mips_read_pc (struct regcache *regcache)
+mips_read_pc (readable_regcache *regcache)
 {
   int regnum = gdbarch_pc_regnum (regcache->arch ());
   LONGEST pc;
 
-  regcache_cooked_read_signed (regcache, regnum, &pc);
+  regcache->cooked_read (regnum, &pc);
   return pc;
 }
 
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index a632c06..695b5cc 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1176,11 +1176,12 @@ spu_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
 }
 
 static CORE_ADDR
-spu_read_pc (struct regcache *regcache)
+spu_read_pc (readable_regcache *regcache)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
   ULONGEST pc;
-  regcache_cooked_read_unsigned (regcache, SPU_PC_REGNUM, &pc);
+
+  regcache->cooked_read (SPU_PC_REGNUM, &pc);
   /* Mask off interrupt enable bit.  */
   return SPUADDR (tdep->id, pc & -4);
 }
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 02/10] class readable_regcache and pass readable_regcache to gdbarch pseudo_register_read and pseudo_register_read_value
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (3 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 01/10] Class reg_buffer Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 08/10] Remove regcache::m_readonly_p Yao Qi
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

pseudo registers are either from raw registers or memory, so
gdbarch methods pseudo_register_read and pseudo_register_read_value
should have regcache object which only have read methods.  In other
words, we should disallow writing to regcache in these two gdbarch
methods.  In order to apply this restriction, this patch adds a new
class readable_regcache, derived from reg_buffer, and it only has
raw_read and cooked_read methods.  regcache is derived from
readable_regcache.  This patch also passes readable_regcache instead of
regcache to gdbarch methods pseudo_register_read and
pseudo_register_read_value.

This patch moves raw_read* and cooked_read* methods to readable_regcache,
which is straightforward.  One thing not straightforward is that I split
regcache::xfer_part to readable_regcache::read_part and regcache::write_part,
because readable_regcache can only have methods to read.

readable_regcache is an abstract base class, and it has a pure virtual
function raw_update, because I don't want readable_regcache know where
these raw registers are from.  They can be from either the target
(readwrite regcache) or the regcache itself (readonly regcache).

gdb:

2017-11-29  Yao Qi  <yao.qi@linaro.org>

	* aarch64-tdep.c (aarch64_pseudo_register_read_value): Change
	parameter type to 'readable_regcache *'.
	* amd64-tdep.c (amd64_pseudo_register_read_value): Likewise.
	* arm-tdep.c (arm_neon_quad_read): Likewise.
	(arm_pseudo_read): Likewise.
	* avr-tdep.c (avr_pseudo_register_read): Likewise.
	* bfin-tdep.c (bfin_pseudo_register_read): Likewise.
	* frv-tdep.c (frv_pseudo_register_read): Likewise.
	* gdbarch.c: Re-generated.
	* gdbarch.h: Re-generated.
	* gdbarch.sh (pseudo_register_read): Change parameter type to
	'readable_regcache *'.
	(pseudo_register_read_value): Likewise.
	* h8300-tdep.c (pseudo_from_raw_register): Likewise.
	(h8300_pseudo_register_read): Likewise.
	* hppa-tdep.c (hppa_pseudo_register_read): Likewise.
	* i386-tdep.c (i386_mmx_regnum_to_fp_regnum): Likewise.
	(i386_pseudo_register_read_into_value): Likewise.
	(i386_pseudo_register_read_value): Likewise.
	* i386-tdep.h (i386_pseudo_register_read_into_value): Update
	declaration.
	* ia64-tdep.c (ia64_pseudo_register_read): Likewise.
	* m32c-tdep.c (m32c_raw_read): Likewise.
	(m32c_read_flg): Likewise.
	(m32c_banked_register): Likewise.
	(m32c_banked_read): Likewise.
	(m32c_sb_read): Likewise.
	(m32c_part_read): Likewise.
	(m32c_cat_read): Likewise.
	(m32c_r3r2r1r0_read): Likewise.
	(m32c_pseudo_register_read): Likewise.
	* m68hc11-tdep.c (m68hc11_pseudo_register_read): Likewise.
	* mep-tdep.c (mep_pseudo_cr32_read): Likewise.
	(mep_pseudo_cr64_read): Likewise.
	(mep_pseudo_register_read): Likewise.
	* mips-tdep.c (mips_pseudo_register_read): Likewise.
	* msp430-tdep.c (msp430_pseudo_register_read): Likewise.
	* nds32-tdep.c (nds32_pseudo_register_read): Likewise.
	* regcache.c (regcache::raw_read): Move it to readable_regcache.
	(regcache::cooked_read): Likewise.
	(regcache::cooked_read_value): Likewise.
	(regcache_cooked_read_signed):
	(regcache::cooked_read): Likewise.
	* regcache.h (readable_regcache): New class.
	(regcache): Inherit readable_regcache.  Move some methods to
	readable_regcache.
	* rl78-tdep.c (rl78_pseudo_register_read): Change
	parameter type to 'readable_regcache *'.
	* rs6000-tdep.c (do_regcache_raw_read): Remove.
	(e500_pseudo_register_read): Change parameter type to
	'readable_regcache *'.
	(dfp_pseudo_register_read): Likewise.
	(vsx_pseudo_register_read): Likewise.
	(efpr_pseudo_register_read): Likewise.
	* s390-tdep.c (s390_pseudo_register_read): Likewise.
	* sh-tdep.c (sh_pseudo_register_read): Likewise.
	* sh64-tdep.c (pseudo_register_read_portions): Likewise.
	(sh64_pseudo_register_read): Likewise.
	* sparc-tdep.c (sparc32_pseudo_register_read): Likewise.
	* sparc64-tdep.c (sparc64_pseudo_register_read): Likewise.
	* spu-tdep.c (spu_pseudo_register_read_spu): Likewise.
	(spu_pseudo_register_read): Likewise.
	* xtensa-tdep.c	(xtensa_register_read_masked): Likewise.
	(xtensa_pseudo_register_read): Likewise.
---
 gdb/aarch64-tdep.c |  2 +-
 gdb/amd64-tdep.c   |  2 +-
 gdb/arm-tdep.c     |  6 ++--
 gdb/avr-tdep.c     |  2 +-
 gdb/bfin-tdep.c    |  2 +-
 gdb/frv-tdep.c     |  2 +-
 gdb/gdbarch.c      |  4 +--
 gdb/gdbarch.h      |  8 +++---
 gdb/gdbarch.sh     |  4 +--
 gdb/h8300-tdep.c   |  4 +--
 gdb/hppa-tdep.c    |  2 +-
 gdb/i386-tdep.c    |  6 ++--
 gdb/i386-tdep.h    |  2 +-
 gdb/ia64-tdep.c    |  2 +-
 gdb/m32c-tdep.c    | 20 ++++++-------
 gdb/m68hc11-tdep.c |  2 +-
 gdb/mep-tdep.c     |  6 ++--
 gdb/mips-tdep.c    |  2 +-
 gdb/msp430-tdep.c  |  2 +-
 gdb/nds32-tdep.c   |  2 +-
 gdb/regcache.c     | 84 ++++++++++++++++++++++++++++++++++--------------------
 gdb/regcache.h     | 60 ++++++++++++++++++++++++--------------
 gdb/rl78-tdep.c    |  2 +-
 gdb/rs6000-tdep.c  | 52 ++++++++++++++++++++++-----------
 gdb/s390-tdep.c    |  2 +-
 gdb/sh-tdep.c      |  4 +--
 gdb/sh64-tdep.c    |  4 +--
 gdb/sparc-tdep.c   |  2 +-
 gdb/sparc64-tdep.c |  2 +-
 gdb/spu-tdep.c     |  4 +--
 gdb/xtensa-tdep.c  |  4 +--
 31 files changed, 180 insertions(+), 122 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 6b59f03..f08945e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2225,7 +2225,7 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 
 static struct value *
 aarch64_pseudo_read_value (struct gdbarch *gdbarch,
-			   struct regcache *regcache,
+			   readable_regcache *regcache,
 			   int regnum)
 {
   gdb_byte reg_buf[V_REGISTER_SIZE];
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index b589d93..6b92c92 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -347,7 +347,7 @@ amd64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 
 static struct value *
 amd64_pseudo_register_read_value (struct gdbarch *gdbarch,
-				  struct regcache *regcache,
+				  readable_regcache *regcache,
 				  int regnum)
 {
   gdb_byte *raw_buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 8be0d4d..ef7e66b 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -227,7 +227,7 @@ static void show_disassembly_style_sfunc (struct ui_file *, int,
 					  const char *);
 
 static enum register_status arm_neon_quad_read (struct gdbarch *gdbarch,
-						struct regcache *regcache,
+						readable_regcache *regcache,
 						int regnum, gdb_byte *buf);
 static void arm_neon_quad_write (struct gdbarch *gdbarch,
 				 struct regcache *regcache,
@@ -8678,7 +8678,7 @@ arm_write_pc (struct regcache *regcache, CORE_ADDR pc)
    the quad register, in [0, 15].  */
 
 static enum register_status
-arm_neon_quad_read (struct gdbarch *gdbarch, struct regcache *regcache,
+arm_neon_quad_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 		    int regnum, gdb_byte *buf)
 {
   char name_buf[4];
@@ -8711,7 +8711,7 @@ arm_neon_quad_read (struct gdbarch *gdbarch, struct regcache *regcache,
 }
 
 static enum register_status
-arm_pseudo_read (struct gdbarch *gdbarch, struct regcache *regcache,
+arm_pseudo_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 		 int regnum, gdb_byte *buf)
 {
   const int num_regs = gdbarch_num_regs (gdbarch);
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index 177717f..7f88e8f 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -382,7 +382,7 @@ avr_write_pc (struct regcache *regcache, CORE_ADDR val)
 }
 
 static enum register_status
-avr_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+avr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
                           int regnum, gdb_byte *buf)
 {
   ULONGEST val;
diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index db8de82..63fbf62 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -688,7 +688,7 @@ bfin_register_name (struct gdbarch *gdbarch, int i)
 }
 
 static enum register_status
-bfin_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+bfin_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int regnum, gdb_byte *buffer)
 {
   gdb_byte buf[BFIN_MAX_REGISTER_SIZE];
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index b6906fa..2f9a8d2 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -295,7 +295,7 @@ frv_register_type (struct gdbarch *gdbarch, int reg)
 }
 
 static enum register_status
-frv_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+frv_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
                           int reg, gdb_byte *buffer)
 {
   enum register_status status;
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index fe3c12e..705a6d3 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -1971,7 +1971,7 @@ gdbarch_pseudo_register_read_p (struct gdbarch *gdbarch)
 }
 
 enum register_status
-gdbarch_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, gdb_byte *buf)
+gdbarch_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_read != NULL);
@@ -1995,7 +1995,7 @@ gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch)
 }
 
 struct value *
-gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum)
+gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->pseudo_register_read_value != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5664c4d..249ad61 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -265,8 +265,8 @@ extern void set_gdbarch_virtual_frame_pointer (struct gdbarch *gdbarch, gdbarch_
 
 extern int gdbarch_pseudo_register_read_p (struct gdbarch *gdbarch);
 
-typedef enum register_status (gdbarch_pseudo_register_read_ftype) (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, gdb_byte *buf);
-extern enum register_status gdbarch_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum, gdb_byte *buf);
+typedef enum register_status (gdbarch_pseudo_register_read_ftype) (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf);
+extern enum register_status gdbarch_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum, gdb_byte *buf);
 extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_ftype *pseudo_register_read);
 
 /* Read a register into a new struct value.  If the register is wholly
@@ -276,8 +276,8 @@ extern void set_gdbarch_pseudo_register_read (struct gdbarch *gdbarch, gdbarch_p
 
 extern int gdbarch_pseudo_register_read_value_p (struct gdbarch *gdbarch);
 
-typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum);
-extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, struct regcache *regcache, int cookednum);
+typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum);
+extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, readable_regcache *regcache, int cookednum);
 extern void set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value);
 
 extern int gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index a929e13..8477dd7 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -429,12 +429,12 @@ F;void;write_pc;struct regcache *regcache, CORE_ADDR val;regcache, val
 # serious shakedown.
 m;void;virtual_frame_pointer;CORE_ADDR pc, int *frame_regnum, LONGEST *frame_offset;pc, frame_regnum, frame_offset;0;legacy_virtual_frame_pointer;;0
 #
-M;enum register_status;pseudo_register_read;struct regcache *regcache, int cookednum, gdb_byte *buf;regcache, cookednum, buf
+M;enum register_status;pseudo_register_read;readable_regcache *regcache, int cookednum, gdb_byte *buf;regcache, cookednum, buf
 # Read a register into a new struct value.  If the register is wholly
 # or partly unavailable, this should call mark_value_bytes_unavailable
 # as appropriate.  If this is defined, then pseudo_register_read will
 # never be called.
-M;struct value *;pseudo_register_read_value;struct regcache *regcache, int cookednum;regcache, cookednum
+M;struct value *;pseudo_register_read_value;readable_regcache *regcache, int cookednum;regcache, cookednum
 M;void;pseudo_register_write;struct regcache *regcache, int cookednum, const gdb_byte *buf;regcache, cookednum, buf
 #
 v;int;num_regs;;;0;-1
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index bf5b9ec..55e77b6 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -1160,7 +1160,7 @@ h8300_register_type (struct gdbarch *gdbarch, int regno)
    raw registers.  These helpers extend/narrow the values.  */
 
 static enum register_status
-pseudo_from_raw_register (struct gdbarch *gdbarch, struct regcache *regcache,
+pseudo_from_raw_register (struct gdbarch *gdbarch, readable_regcache *regcache,
 			  gdb_byte *buf, int pseudo_regno, int raw_regno)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
@@ -1191,7 +1191,7 @@ raw_from_pseudo_register (struct gdbarch *gdbarch, struct regcache *regcache,
 
 static enum register_status
 h8300_pseudo_register_read (struct gdbarch *gdbarch,
-			    struct regcache *regcache, int regno,
+			    readable_regcache *regcache, int regno,
 			    gdb_byte *buf)
 {
   if (regno == E_PSEUDO_CCR_REGNUM (gdbarch))
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 814b8a8..cc9434e 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -2747,7 +2747,7 @@ hppa_fetch_pointer_argument (struct frame_info *frame, int argi,
 }
 
 static enum register_status
-hppa_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+hppa_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index cd56642..6b59278 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3248,7 +3248,7 @@ i386_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
    the MMX registers need to be mapped onto floating point registers.  */
 
 static int
-i386_mmx_regnum_to_fp_regnum (struct regcache *regcache, int regnum)
+i386_mmx_regnum_to_fp_regnum (readable_regcache *regcache, int regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
   int mmxreg, fpreg;
@@ -3269,7 +3269,7 @@ i386_mmx_regnum_to_fp_regnum (struct regcache *regcache, int regnum)
 
 void
 i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-				      struct regcache *regcache,
+				      readable_regcache *regcache,
 				      int regnum,
 				      struct value *result_value)
 {
@@ -3448,7 +3448,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
 
 static struct value *
 i386_pseudo_register_read_value (struct gdbarch *gdbarch,
-				 struct regcache *regcache,
+				 readable_regcache *regcache,
 				 int regnum)
 {
   struct value *result;
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index a71c103..81a93f1 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -364,7 +364,7 @@ extern struct type *i386_pseudo_register_type (struct gdbarch *gdbarch,
 					       int regnum);
 
 extern void i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
-						  struct regcache *regcache,
+						  readable_regcache *regcache,
 						  int regnum,
 						  struct value *result);
 
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 18f517d..6c9b341 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -927,7 +927,7 @@ rse_address_add(CORE_ADDR addr, int nslots)
 }
 
 static enum register_status
-ia64_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+ia64_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
                            int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 45dc438..173e8a4 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -51,7 +51,7 @@ typedef enum register_status (m32c_write_reg_t) (struct m32c_reg *reg,
 						 const gdb_byte *buf);
 
 typedef enum register_status (m32c_read_reg_t) (struct m32c_reg *reg,
-						struct regcache *cache,
+						readable_regcache *cache,
 						gdb_byte *buf);
 
 struct m32c_reg
@@ -310,7 +310,7 @@ static m32c_write_reg_t m32c_r3r2r1r0_write;
 
 /* Copy the value of the raw register REG from CACHE to BUF.  */
 static enum register_status
-m32c_raw_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_raw_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   return cache->raw_read (reg->num, buf);
 }
@@ -329,7 +329,7 @@ m32c_raw_write (struct m32c_reg *reg, struct regcache *cache,
 
 /* Return the value of the 'flg' register in CACHE.  */
 static int
-m32c_read_flg (struct regcache *cache)
+m32c_read_flg (readable_regcache *cache)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (cache->arch ());
   ULONGEST flg;
@@ -341,7 +341,7 @@ m32c_read_flg (struct regcache *cache)
 
 /* Evaluate the real register number of a banked register.  */
 static struct m32c_reg *
-m32c_banked_register (struct m32c_reg *reg, struct regcache *cache)
+m32c_banked_register (struct m32c_reg *reg, readable_regcache *cache)
 {
   return ((m32c_read_flg (cache) & reg->n) ? reg->ry : reg->rx);
 }
@@ -352,7 +352,7 @@ m32c_banked_register (struct m32c_reg *reg, struct regcache *cache)
    masked in REG->n set, then read REG->ry.  Otherwise, read
    REG->rx.  */
 static enum register_status
-m32c_banked_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_banked_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   struct m32c_reg *bank_reg = m32c_banked_register (reg, cache);
   return cache->raw_read (bank_reg->num, buf);
@@ -377,7 +377,7 @@ m32c_banked_write (struct m32c_reg *reg, struct regcache *cache,
 /* Move the value of SB from CACHE to BUF.  On bfd_mach_m32c, SB is a
    banked register; on bfd_mach_m16c, it's not.  */
 static enum register_status
-m32c_sb_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_sb_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   if (gdbarch_bfd_arch_info (reg->arch)->mach == bfd_mach_m16c)
     return m32c_raw_read (reg->rx, cache, buf);
@@ -442,7 +442,7 @@ m32c_find_part (struct m32c_reg *reg, int *offset_p, int *len_p)
    REG->type values, where higher indices refer to more significant
    bits, read the value of the REG->n'th element.  */
 static enum register_status
-m32c_part_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_part_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   int offset, len;
 
@@ -473,7 +473,7 @@ m32c_part_write (struct m32c_reg *reg, struct regcache *cache,
    concatenation of the values of the registers REG->rx and REG->ry,
    with REG->rx contributing the more significant bits.  */
 static enum register_status
-m32c_cat_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_cat_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   int high_bytes = TYPE_LENGTH (reg->rx->type);
   int low_bytes  = TYPE_LENGTH (reg->ry->type);
@@ -528,7 +528,7 @@ m32c_cat_write (struct m32c_reg *reg, struct regcache *cache,
    the concatenation (from most significant to least) of r3, r2, r1,
    and r0.  */
 static enum register_status
-m32c_r3r2r1r0_read (struct m32c_reg *reg, struct regcache *cache, gdb_byte *buf)
+m32c_r3r2r1r0_read (struct m32c_reg *reg, readable_regcache *cache, gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (reg->arch);
   int len = TYPE_LENGTH (tdep->r0->type);
@@ -590,7 +590,7 @@ m32c_r3r2r1r0_write (struct m32c_reg *reg, struct regcache *cache,
 
 static enum register_status
 m32c_pseudo_register_read (struct gdbarch *arch,
-			   struct regcache *cache,
+			   readable_regcache *cache,
 			   int cookednum,
 			   gdb_byte *buf)
 {
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index b34548b..58ef4a3 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -279,7 +279,7 @@ m68hc11_which_soft_register (CORE_ADDR addr)
    fetch into a memory read.  */
 static enum register_status
 m68hc11_pseudo_register_read (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      readable_regcache *regcache,
 			      int regno, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index 7b1fb23..1cda2b3 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -1114,7 +1114,7 @@ mep_register_type (struct gdbarch *gdbarch, int reg_nr)
 
 static enum register_status
 mep_pseudo_cr32_read (struct gdbarch *gdbarch,
-                      struct regcache *regcache,
+		      readable_regcache *regcache,
                       int cookednum,
                       gdb_byte *buf)
 {
@@ -1140,7 +1140,7 @@ mep_pseudo_cr32_read (struct gdbarch *gdbarch,
 
 static enum register_status
 mep_pseudo_cr64_read (struct gdbarch *gdbarch,
-                      struct regcache *regcache,
+                      readable_regcache *regcache,
                       int cookednum,
                       gdb_byte *buf)
 {
@@ -1150,7 +1150,7 @@ mep_pseudo_cr64_read (struct gdbarch *gdbarch,
 
 static enum register_status
 mep_pseudo_register_read (struct gdbarch *gdbarch,
-                          struct regcache *regcache,
+			  readable_regcache *regcache,
                           int cookednum,
                           gdb_byte *buf)
 {
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6b78093..2c1a8f0 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -737,7 +737,7 @@ mips_tdesc_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
    registers.  Take care of alignment and size problems.  */
 
 static enum register_status
-mips_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+mips_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int cookednum, gdb_byte *buf)
 {
   int rawnum = cookednum % gdbarch_num_regs (gdbarch);
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 4ae71f7..169b7e9 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -218,7 +218,7 @@ msp430_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 
 static enum register_status
 msp430_pseudo_register_read (struct gdbarch *gdbarch,
-			     struct regcache *regcache,
+			     readable_regcache *regcache,
 			     int regnum, gdb_byte *buffer)
 {
   if (MSP430_NUM_REGS <= regnum && regnum < MSP430_NUM_TOTAL_REGS)
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 73a71b9..8cc9d5b 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -437,7 +437,7 @@ nds32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 nds32_pseudo_register_read (struct gdbarch *gdbarch,
-			    struct regcache *regcache, int regnum,
+			    readable_regcache *regcache, int regnum,
 			    gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8addc82..ad5e0a2 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -205,7 +205,7 @@ regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
 /* The register buffers.  A read-only register cache can hold the
    full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a
    read/write register cache can only hold [0 .. gdbarch_num_regs).  */
-  : reg_buffer (gdbarch, readonly_p_),
+  : readable_regcache (gdbarch, readonly_p_),
     m_aspace (aspace_), m_readonly_p (readonly_p_)
 {
   m_ptid = minus_one_ptid;
@@ -581,7 +581,7 @@ regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf)
 }
 
 enum register_status
-regcache::raw_read (int regnum, gdb_byte *buf)
+readable_regcache::raw_read (int regnum, gdb_byte *buf)
 {
   gdb_assert (buf != NULL);
   raw_update (regnum);
@@ -604,7 +604,7 @@ regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
 
 template<typename T, typename>
 enum register_status
-regcache::raw_read (int regnum, T *val)
+readable_regcache::raw_read (int regnum, T *val)
 {
   gdb_byte *buf;
   enum register_status status;
@@ -677,17 +677,15 @@ regcache_cooked_read (struct regcache *regcache, int regnum, gdb_byte *buf)
 }
 
 enum register_status
-regcache::cooked_read (int regnum, gdb_byte *buf)
+readable_regcache::cooked_read (int regnum, gdb_byte *buf)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
   if (regnum < num_raw_registers ())
     return raw_read (regnum, buf);
-  else if (m_readonly_p
+  else if (m_has_pseudo
 	   && m_register_status[regnum] != REG_UNKNOWN)
     {
-      /* Read-only register cache, perhaps the cooked value was
-	 cached?  */
       if (m_register_status[regnum] == REG_VALID)
 	memcpy (buf, register_buffer (regnum),
 		m_descr->sizeof_register[regnum]);
@@ -730,13 +728,13 @@ regcache_cooked_read_value (struct regcache *regcache, int regnum)
 }
 
 struct value *
-regcache::cooked_read_value (int regnum)
+readable_regcache::cooked_read_value (int regnum)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
 
   if (regnum < num_raw_registers ()
-      || (m_readonly_p && m_register_status[regnum] != REG_UNKNOWN)
+      || (m_has_pseudo && m_register_status[regnum] != REG_UNKNOWN)
       || !gdbarch_pseudo_register_read_value_p (m_descr->gdbarch))
     {
       struct value *result;
@@ -770,7 +768,7 @@ regcache_cooked_read_signed (struct regcache *regcache, int regnum,
 
 template<typename T, typename>
 enum register_status
-regcache::cooked_read (int regnum, T *val)
+readable_regcache::cooked_read (int regnum, T *val)
 {
   enum register_status status;
   gdb_byte *buf;
@@ -910,20 +908,49 @@ typedef void (regcache_write_ftype) (struct regcache *regcache, int regnum,
 				     const void *buf);
 
 enum register_status
-regcache::xfer_part (int regnum, int offset, int len, void *in,
+readable_regcache::read_part (int regnum, int offset, int len, void *in,
+			      bool is_raw)
+{
+  struct gdbarch *gdbarch = arch ();
+  gdb_byte *reg = (gdb_byte *) alloca (register_size (gdbarch, regnum));
+
+  gdb_assert (in != NULL);
+  gdb_assert (offset >= 0 && offset <= m_descr->sizeof_register[regnum]);
+  gdb_assert (len >= 0 && offset + len <= m_descr->sizeof_register[regnum]);
+  /* Something to do?  */
+  if (offset + len == 0)
+    return REG_VALID;
+  /* Read (when needed) ...  */
+  enum register_status status;
+
+  if (is_raw)
+    status = raw_read (regnum, reg);
+  else
+    status = cooked_read (regnum, reg);
+  if (status != REG_VALID)
+    return status;
+
+  /* ... modify ...  */
+  memcpy (in, reg + offset, len);
+
+  return REG_VALID;
+}
+
+enum register_status
+regcache::write_part (int regnum, int offset, int len,
 		     const void *out, bool is_raw)
 {
   struct gdbarch *gdbarch = arch ();
   gdb_byte *reg = (gdb_byte *) alloca (register_size (gdbarch, regnum));
 
+  gdb_assert (out != NULL);
   gdb_assert (offset >= 0 && offset <= m_descr->sizeof_register[regnum]);
   gdb_assert (len >= 0 && offset + len <= m_descr->sizeof_register[regnum]);
   /* Something to do?  */
   if (offset + len == 0)
     return REG_VALID;
   /* Read (when needed) ...  */
-  if (in != NULL
-      || offset > 0
+  if (offset > 0
       || offset + len < m_descr->sizeof_register[regnum])
     {
       enum register_status status;
@@ -935,19 +962,13 @@ regcache::xfer_part (int regnum, int offset, int len, void *in,
       if (status != REG_VALID)
 	return status;
     }
-  /* ... modify ...  */
-  if (in != NULL)
-    memcpy (in, reg + offset, len);
-  if (out != NULL)
-    memcpy (reg + offset, out, len);
+
+  memcpy (reg + offset, out, len);
   /* ... write (when needed).  */
-  if (out != NULL)
-    {
-      if (is_raw)
-	raw_write (regnum, reg);
-      else
-	cooked_write (regnum, reg);
-    }
+  if (is_raw)
+    raw_write (regnum, reg);
+  else
+    cooked_write (regnum, reg);
 
   return REG_VALID;
 }
@@ -960,10 +981,10 @@ regcache_raw_read_part (struct regcache *regcache, int regnum,
 }
 
 enum register_status
-regcache::raw_read_part (int regnum, int offset, int len, gdb_byte *buf)
+readable_regcache::raw_read_part (int regnum, int offset, int len, gdb_byte *buf)
 {
   assert_regnum (regnum);
-  return xfer_part (regnum, offset, len, buf, NULL, true);
+  return read_part (regnum, offset, len, buf, true);
 }
 
 void
@@ -978,7 +999,7 @@ regcache::raw_write_part (int regnum, int offset, int len,
 			  const gdb_byte *buf)
 {
   assert_regnum (regnum);
-  xfer_part (regnum, offset, len, NULL, buf, true);
+  write_part (regnum, offset, len, buf, true);
 }
 
 enum register_status
@@ -990,10 +1011,11 @@ regcache_cooked_read_part (struct regcache *regcache, int regnum,
 
 
 enum register_status
-regcache::cooked_read_part (int regnum, int offset, int len, gdb_byte *buf)
+readable_regcache::cooked_read_part (int regnum, int offset, int len,
+				     gdb_byte *buf)
 {
   gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
-  return xfer_part (regnum, offset, len, buf, NULL, false);
+  return read_part (regnum, offset, len, buf, false);
 }
 
 void
@@ -1008,7 +1030,7 @@ regcache::cooked_write_part (int regnum, int offset, int len,
 			     const gdb_byte *buf)
 {
   gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
-  xfer_part (regnum, offset, len, NULL, buf, false);
+  write_part (regnum, offset, len, buf, false);
 }
 
 /* Supply register REGNUM, whose contents are stored in BUF, to REGCACHE.  */
diff --git a/gdb/regcache.h b/gdb/regcache.h
index e2c7621..1c7ee8c 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -262,9 +262,41 @@ protected:
   signed char *m_register_status;
 };
 
+/* An abstract class which only has methods doing read.  */
+
+class readable_regcache : public reg_buffer
+{
+public:
+  readable_regcache (gdbarch *gdbarch, bool has_pseudo)
+    : reg_buffer (gdbarch, has_pseudo)
+  {}
+
+  enum register_status raw_read (int regnum, gdb_byte *buf);
+  template<typename T, typename = RequireLongest<T>>
+  enum register_status raw_read (int regnum, T *val);
+
+  enum register_status raw_read_part (int regnum, int offset, int len,
+				      gdb_byte *buf);
+
+  virtual void raw_update (int regnum) = 0;
+
+  enum register_status cooked_read (int regnum, gdb_byte *buf);
+  template<typename T, typename = RequireLongest<T>>
+  enum register_status cooked_read (int regnum, T *val);
+
+  enum register_status cooked_read_part (int regnum, int offset, int len,
+					 gdb_byte *buf);
+
+  struct value *cooked_read_value (int regnum);
+
+protected:
+  enum register_status read_part (int regnum, int offset, int len, void *in,
+				  bool is_raw);
+};
+
 /* The register cache for storing raw register values.  */
 
-class regcache : public reg_buffer
+class regcache : public readable_regcache
 {
 public:
   regcache (gdbarch *gdbarch)
@@ -287,28 +319,17 @@ public:
 
   void save (regcache_cooked_read_ftype *cooked_read, void *src);
 
-  enum register_status cooked_read (int regnum, gdb_byte *buf);
   void cooked_write (int regnum, const gdb_byte *buf);
 
-  enum register_status raw_read (int regnum, gdb_byte *buf);
-
   void raw_write (int regnum, const gdb_byte *buf);
 
   template<typename T, typename = RequireLongest<T>>
-  enum register_status raw_read (int regnum, T *val);
-
-  template<typename T, typename = RequireLongest<T>>
   void raw_write (int regnum, T val);
 
-  struct value *cooked_read_value (int regnum);
-
-  template<typename T, typename = RequireLongest<T>>
-  enum register_status cooked_read (int regnum, T *val);
-
   template<typename T, typename = RequireLongest<T>>
   void cooked_write (int regnum, T val);
 
-  void raw_update (int regnum);
+  void raw_update (int regnum) override;
 
   void raw_collect (int regnum, void *buf) const;
 
@@ -328,14 +349,8 @@ public:
 
   void invalidate (int regnum);
 
-  enum register_status raw_read_part (int regnum, int offset, int len,
-				      gdb_byte *buf);
-
   void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
 
-  enum register_status cooked_read_part (int regnum, int offset, int len,
-					 gdb_byte *buf);
-
   void cooked_write_part (int regnum, int offset, int len,
 			  const gdb_byte *buf);
 
@@ -370,14 +385,15 @@ protected:
 private:
   void restore (struct regcache *src);
 
-  enum register_status xfer_part (int regnum, int offset, int len, void *in,
-				  const void *out, bool is_raw);
-
   void transfer_regset (const struct regset *regset,
 			struct regcache *out_regcache,
 			int regnum, const void *in_buf,
 			void *out_buf, size_t size) const;
 
+  enum register_status write_part (int regnum, int offset, int len,
+				   const void *out, bool is_raw);
+
+
   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
   const address_space * const m_aspace;
diff --git a/gdb/rl78-tdep.c b/gdb/rl78-tdep.c
index 55653db..af6a089 100644
--- a/gdb/rl78-tdep.c
+++ b/gdb/rl78-tdep.c
@@ -640,7 +640,7 @@ rl78_make_data_address (CORE_ADDR addr)
 
 static enum register_status
 rl78_pseudo_register_read (struct gdbarch *gdbarch,
-                           struct regcache *regcache,
+			   readable_regcache *regcache,
                            int reg, gdb_byte *buffer)
 {
   enum register_status status;
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 70dc55f..a5b7091 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -2706,12 +2706,6 @@ e500_move_ev_register (move_ev_register_func move,
 }
 
 static enum register_status
-do_regcache_raw_read (struct regcache *regcache, int regnum, void *buffer)
-{
-  return regcache_raw_read (regcache, regnum, (gdb_byte *) buffer);
-}
-
-static enum register_status
 do_regcache_raw_write (struct regcache *regcache, int regnum, void *buffer)
 {
   regcache_raw_write (regcache, regnum, (const gdb_byte *) buffer);
@@ -2720,10 +2714,36 @@ do_regcache_raw_write (struct regcache *regcache, int regnum, void *buffer)
 }
 
 static enum register_status
-e500_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
-			   int reg_nr, gdb_byte *buffer)
+e500_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
+			   int ev_reg, gdb_byte *buffer)
 {
-  return e500_move_ev_register (do_regcache_raw_read, regcache, reg_nr, buffer);
+  struct gdbarch *arch = regcache->arch ();
+  struct gdbarch_tdep *tdep = gdbarch_tdep (arch);
+  int reg_index;
+  enum register_status status;
+
+  gdb_assert (IS_SPE_PSEUDOREG (tdep, ev_reg));
+
+  reg_index = ev_reg - tdep->ppc_ev0_regnum;
+
+  if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG)
+    {
+      status = regcache->raw_read (tdep->ppc_ev0_upper_regnum + reg_index,
+				   buffer);
+      if (status == REG_VALID)
+	status = regcache->raw_read (tdep->ppc_gp0_regnum + reg_index,
+				     buffer + 4);
+    }
+  else
+    {
+      status = regcache->raw_read (tdep->ppc_gp0_regnum + reg_index, buffer);
+      if (status == REG_VALID)
+	status = regcache->raw_read (tdep->ppc_ev0_upper_regnum + reg_index,
+				     buffer + 4);
+    }
+
+  return status;
+
 }
 
 static void
@@ -2736,7 +2756,7 @@ e500_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for DFP pseudo-registers.  */
 static enum register_status
-dfp_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+dfp_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2792,7 +2812,7 @@ dfp_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for POWER7 VSX pseudo-registers.  */
 static enum register_status
-vsx_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+vsx_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2857,7 +2877,7 @@ vsx_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 /* Read method for POWER7 Extended FP pseudo-registers.  */
 static enum register_status
-efpr_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+efpr_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -2865,9 +2885,9 @@ efpr_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
   int offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 0 : 8;
 
   /* Read the portion that overlaps the VMX register.  */
-  return regcache_raw_read_part (regcache, tdep->ppc_vr0_regnum + reg_index,
-				 offset, register_size (gdbarch, reg_nr),
-				 buffer);
+  return regcache->raw_read_part (tdep->ppc_vr0_regnum + reg_index,
+				  offset, register_size (gdbarch, reg_nr),
+				  buffer);
 }
 
 /* Write method for POWER7 Extended FP pseudo-registers.  */
@@ -2887,7 +2907,7 @@ efpr_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 
 static enum register_status
 rs6000_pseudo_register_read (struct gdbarch *gdbarch,
-			     struct regcache *regcache,
+			     readable_regcache *regcache,
 			     int reg_nr, gdb_byte *buffer)
 {
   struct gdbarch *regcache_arch = regcache->arch ();
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 9626e05..408bb87 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -1281,7 +1281,7 @@ s390_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
 /* Implement pseudo_register_read gdbarch method.  */
 
 static enum register_status
-s390_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+s390_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int regnum, gdb_byte *buf)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
index a82c5b3..75ea673 100644
--- a/gdb/sh-tdep.c
+++ b/gdb/sh-tdep.c
@@ -1628,7 +1628,7 @@ dr_reg_base_num (struct gdbarch *gdbarch, int dr_regnum)
 
 static enum register_status
 pseudo_register_read_portions (struct gdbarch *gdbarch,
-			       struct regcache *regcache,
+			       readable_regcache *regcache,
 			       int portions,
 			       int base_regnum, gdb_byte *buffer)
 {
@@ -1649,7 +1649,7 @@ pseudo_register_read_portions (struct gdbarch *gdbarch,
 }
 
 static enum register_status
-sh_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+sh_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			 int reg_nr, gdb_byte *buffer)
 {
   int base_regnum;
diff --git a/gdb/sh64-tdep.c b/gdb/sh64-tdep.c
index 5da517a..8eb88eb 100644
--- a/gdb/sh64-tdep.c
+++ b/gdb/sh64-tdep.c
@@ -1504,7 +1504,7 @@ sh64_register_convert_to_raw (struct gdbarch *gdbarch, struct type *type,
 
 static enum register_status
 pseudo_register_read_portions (struct gdbarch *gdbarch,
-			       struct regcache *regcache,
+			       readable_regcache *regcache,
 			       int portions,
 			       int base_regnum, gdb_byte *buffer)
 {
@@ -1525,7 +1525,7 @@ pseudo_register_read_portions (struct gdbarch *gdbarch,
 }
 
 static enum register_status
-sh64_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+sh64_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int reg_nr, gdb_byte *buffer)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index 1ce7ee0..633bd68 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -503,7 +503,7 @@ sparc32_register_type (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 sparc32_pseudo_register_read (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      readable_regcache *regcache,
 			      int regnum, gdb_byte *buf)
 {
   enum register_status status;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 67103cc..6f3ca19 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -899,7 +899,7 @@ sparc64_register_type (struct gdbarch *gdbarch, int regnum)
 
 static enum register_status
 sparc64_pseudo_register_read (struct gdbarch *gdbarch,
-			      struct regcache *regcache,
+			      readable_regcache *regcache,
 			      int regnum, gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index f2f4f65..da7b937 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -182,7 +182,7 @@ spu_register_type (struct gdbarch *gdbarch, int reg_nr)
 /* Pseudo registers for preferred slots - stack pointer.  */
 
 static enum register_status
-spu_pseudo_register_read_spu (struct regcache *regcache, const char *regname,
+spu_pseudo_register_read_spu (readable_regcache *regcache, const char *regname,
 			      gdb_byte *buf)
 {
   struct gdbarch *gdbarch = regcache->arch ();
@@ -207,7 +207,7 @@ spu_pseudo_register_read_spu (struct regcache *regcache, const char *regname,
 }
 
 static enum register_status
-spu_pseudo_register_read (struct gdbarch *gdbarch, struct regcache *regcache,
+spu_pseudo_register_read (struct gdbarch *gdbarch, readable_regcache *regcache,
                           int regnum, gdb_byte *buf)
 {
   gdb_byte reg[16];
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 6eb6fd3..a1ecf5f 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -453,7 +453,7 @@ xtensa_register_write_masked (struct regcache *regcache,
    of the registers and assemble them into a single value.  */
 
 static enum register_status
-xtensa_register_read_masked (struct regcache *regcache,
+xtensa_register_read_masked (readable_regcache *regcache,
 			     xtensa_register_t *reg, gdb_byte *buffer)
 {
   unsigned int value[(XTENSA_MAX_REGISTER_SIZE + 3) / 4];
@@ -547,7 +547,7 @@ xtensa_register_read_masked (struct regcache *regcache,
 
 static enum register_status
 xtensa_pseudo_register_read (struct gdbarch *gdbarch,
-			     struct regcache *regcache,
+			     readable_regcache *regcache,
 			     int regnum,
 			     gdb_byte *buffer)
 {
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 05/10] Class detached_regcache
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (8 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 09/10] Move register_dump to regcache-dump.c Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-17  3:15   ` Simon Marchi
  2018-02-17  3:53 ` [PATCH 00/10 v2] Remove regcache::m_readonly_p Simon Marchi
  10 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

jit.c uses the regcache in a slightly different way, the regcache dosn't
write through to target, but it has read and write methods.  If I apply
regcache in record-full.c, it has the similar use pattern.  This patch
adds a new class detached_regcache, a register buffer, but can be
red and written.

Since jit.c doesn't want to write registers through to target, it uses
regcache as a readonly regcache (because only readonly regcache
disconnects from the target), but it adds a hole in regcache
(raw_set_cached_value) in order to modify a readonly regcache.  This patch
fixes this hole completely.

regcache inherits detached_regcache, and detached_regcache inherits
readable_regcache.  The ideal design is that both detached_regcache and
readable_regcache inherit reg_buffer, and regcache inherit
detached_regcache and regcache_read (virtual inheritance).  I concern
about the performance overhead of virtual inheritance, so I don't do it in
the patch.

gdb:

2017-11-28  Yao Qi  <yao.qi@linaro.org>

	* jit.c (struct jit_unwind_private) <regcache>: Change its type to
	 reg_buffer_rw *.
	(jit_unwind_reg_set_impl): Call raw_supply.
	(jit_frame_sniffer): Use reg_buffer_rw.
	* record-full.c (record_full_core_regbuf): Change its type.
	(record_full_core_open_1): Use reg_buffer_rw.
	(record_full_close): Likewise.
	(record_full_core_fetch_registers): Use regcache->raw_supply.
	(record_full_core_store_registers): Likewise.
	* regcache.c (regcache::get_register_status): Move it to
	reg_buffer.
	(regcache_raw_set_cached_value): Remove.
	(regcache::raw_set_cached_value): Remove.
	(regcache::raw_write): Call raw_supply.
	(regcache::raw_supply): Move it to reg_buffer_rw.
	* regcache.h (regcache_raw_set_cached_value): Remove.
	(reg_buffer_rw): New class.
---
 gdb/jit.c         | 10 ++++------
 gdb/record-full.c | 21 +++++++++------------
 gdb/regcache.c    | 32 +++++---------------------------
 gdb/regcache.h    | 42 ++++++++++++++++++++++++++----------------
 4 files changed, 44 insertions(+), 61 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 01ead45..e67e1d2 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1097,7 +1097,7 @@ struct jit_unwind_private
 {
   /* Cached register values.  See jit_frame_sniffer to see how this
      works.  */
-  struct regcache *regcache;
+  detached_regcache *regcache;
 
   /* The frame being unwound.  */
   struct frame_info *this_frame;
@@ -1126,7 +1126,7 @@ jit_unwind_reg_set_impl (struct gdb_unwind_callbacks *cb, int dwarf_regnum,
       return;
     }
 
-  regcache_raw_set_cached_value (priv->regcache, gdb_reg, value->value);
+  priv->regcache->raw_supply (gdb_reg, value->value);
   value->free (value);
 }
 
@@ -1188,7 +1188,6 @@ jit_frame_sniffer (const struct frame_unwind *self,
   struct jit_unwind_private *priv_data;
   struct gdb_unwind_callbacks callbacks;
   struct gdb_reader_funcs *funcs;
-  struct gdbarch *gdbarch;
 
   callbacks.reg_get = jit_unwind_reg_get_impl;
   callbacks.reg_set = jit_unwind_reg_set_impl;
@@ -1201,11 +1200,10 @@ jit_frame_sniffer (const struct frame_unwind *self,
 
   gdb_assert (!*cache);
 
-  gdbarch = get_frame_arch (this_frame);
-
   *cache = XCNEW (struct jit_unwind_private);
   priv_data = (struct jit_unwind_private *) *cache;
-  priv_data->regcache = new regcache (gdbarch);
+  /* Take a snapshot of current regcache.  */
+  priv_data->regcache = new detached_regcache (get_frame_arch (this_frame), true);
   priv_data->this_frame = this_frame;
 
   callbacks.priv_data = priv_data;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 13a9738..82b3c74 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -168,7 +168,7 @@ struct record_full_core_buf_entry
 };
 
 /* Record buf with core target.  */
-static gdb_byte *record_full_core_regbuf = NULL;
+static detached_regcache *record_full_core_regbuf = NULL;
 static struct target_section *record_full_core_start;
 static struct target_section *record_full_core_end;
 static struct record_full_core_buf_entry *record_full_core_buf_list = NULL;
@@ -780,16 +780,16 @@ record_full_core_open_1 (const char *name, int from_tty)
 
   /* Get record_full_core_regbuf.  */
   target_fetch_registers (regcache, -1);
-  record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum);
+  record_full_core_regbuf = new detached_regcache (regcache->arch (), false);
+
   for (i = 0; i < regnum; i ++)
-    regcache_raw_collect (regcache, i,
-			  record_full_core_regbuf + MAX_REGISTER_SIZE * i);
+    record_full_core_regbuf->raw_supply (i, *regcache);
 
   /* Get record_full_core_start and record_full_core_end.  */
   if (build_section_table (core_bfd, &record_full_core_start,
 			   &record_full_core_end))
     {
-      xfree (record_full_core_regbuf);
+      delete record_full_core_regbuf;
       record_full_core_regbuf = NULL;
       error (_("\"%s\": Can't find sections: %s"),
 	     bfd_get_filename (core_bfd), bfd_errmsg (bfd_get_error ()));
@@ -869,7 +869,7 @@ record_full_close (struct target_ops *self)
   /* Release record_full_core_regbuf.  */
   if (record_full_core_regbuf)
     {
-      xfree (record_full_core_regbuf);
+      delete record_full_core_regbuf;
       record_full_core_regbuf = NULL;
     }
 
@@ -2030,12 +2030,10 @@ record_full_core_fetch_registers (struct target_ops *ops,
       int i;
 
       for (i = 0; i < num; i ++)
-        regcache_raw_supply (regcache, i,
-                             record_full_core_regbuf + MAX_REGISTER_SIZE * i);
+	regcache->raw_supply (i, *record_full_core_regbuf);
     }
   else
-    regcache_raw_supply (regcache, regno,
-                         record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
+    regcache->raw_supply (regno, *record_full_core_regbuf);
 }
 
 /* "to_prepare_to_store" method for prec over corefile.  */
@@ -2054,8 +2052,7 @@ record_full_core_store_registers (struct target_ops *ops,
                              int regno)
 {
   if (record_full_gdb_operation_disable)
-    regcache_raw_collect (regcache, regno,
-                          record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
+    record_full_core_regbuf->raw_supply (regno, *regcache);
   else
     error (_("You can't do that without a process to debug."));
 }
diff --git a/gdb/regcache.c b/gdb/regcache.c
index ebe3c7b..c757231 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -205,7 +205,7 @@ regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
 /* The register buffers.  A read-only register cache can hold the
    full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a
    read/write register cache can only hold [0 .. gdbarch_num_regs).  */
-  : readable_regcache (gdbarch, readonly_p_),
+  : detached_regcache (gdbarch, readonly_p_),
     m_aspace (aspace_), m_readonly_p (readonly_p_)
 {
   m_ptid = minus_one_ptid;
@@ -353,13 +353,9 @@ regcache_register_status (const struct regcache *regcache, int regnum)
 }
 
 enum register_status
-regcache::get_register_status (int regnum) const
+reg_buffer::get_register_status (int regnum) const
 {
-  gdb_assert (regnum >= 0);
-  if (m_readonly_p)
-    gdb_assert (regnum < m_descr->nr_cooked_registers);
-  else
-    gdb_assert (regnum < num_raw_registers ());
+  assert_regnum (regnum);
 
   return (enum register_status) m_register_status[regnum];
 }
@@ -802,23 +798,6 @@ regcache_cooked_write_unsigned (struct regcache *regcache, int regnum,
   regcache->cooked_write (regnum, val);
 }
 
-/* See regcache.h.  */
-
-void
-regcache_raw_set_cached_value (struct regcache *regcache, int regnum,
-			       const gdb_byte *buf)
-{
-  regcache->raw_set_cached_value (regnum, buf);
-}
-
-void
-regcache::raw_set_cached_value (int regnum, const gdb_byte *buf)
-{
-  memcpy (register_buffer (regnum), buf,
-	  m_descr->sizeof_register[regnum]);
-  m_register_status[regnum] = REG_VALID;
-}
-
 void
 regcache_raw_write (struct regcache *regcache, int regnum,
 		    const gdb_byte *buf)
@@ -848,7 +827,7 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
     return;
 
   target_prepare_to_store (this);
-  raw_set_cached_value (regnum, buf);
+  raw_supply (regnum, buf);
 
   /* Invalidate the register after it is written, in case of a
      failure.  */
@@ -1024,13 +1003,12 @@ regcache_raw_supply (struct regcache *regcache, int regnum, const void *buf)
 }
 
 void
-regcache::raw_supply (int regnum, const void *buf)
+detached_regcache::raw_supply (int regnum, const void *buf)
 {
   void *regbuf;
   size_t size;
 
   assert_regnum (regnum);
-  gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
   size = m_descr->sizeof_register[regnum];
diff --git a/gdb/regcache.h b/gdb/regcache.h
index d4a9d9b..9a1ac41 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -68,14 +68,6 @@ extern void regcache_raw_write_unsigned (struct regcache *regcache,
 extern LONGEST regcache_raw_get_signed (struct regcache *regcache,
 					int regnum);
 
-/* Set a raw register's value in the regcache's buffer.  Unlike
-   regcache_raw_write, this is not write-through.  The intention is
-   allowing to change the buffer contents of a read-only regcache
-   allocated with new.  */
-
-extern void regcache_raw_set_cached_value
-  (struct regcache *regcache, int regnum, const gdb_byte *buf);
-
 /* Partial transfer of raw registers.  These perform read, modify,
    write style operations.  The read variant returns the status of the
    register.  */
@@ -229,12 +221,13 @@ public:
   /* Return regcache's architecture.  */
   gdbarch *arch () const;
 
+  enum register_status get_register_status (int regnum) const;
+
   virtual ~reg_buffer ()
   {
     xfree (m_registers);
     xfree (m_register_status);
   }
-
 protected:
   /* Assert on the range of REGNUM.  */
   void assert_regnum (int regnum) const;
@@ -257,6 +250,7 @@ protected:
   signed char *m_register_status;
 
   friend class regcache;
+  friend class detached_regcache;
 };
 
 /* An abstract class which only has methods doing read.  */
@@ -291,11 +285,33 @@ protected:
 				  bool is_raw);
 };
 
+/* Buffer of registers, can be red and written.  */
+
+class detached_regcache : public readable_regcache
+{
+public:
+  detached_regcache (gdbarch *gdbarch, bool has_pseudo)
+    : readable_regcache (gdbarch, has_pseudo)
+  {}
+
+  void raw_supply (int regnum, const void *buf);
+
+  void raw_supply (int regnum, const reg_buffer &src)
+  {
+    raw_supply (regnum, src.register_buffer (regnum));
+  }
+
+  void raw_update (int regnum) override
+  {}
+
+  DISABLE_COPY_AND_ASSIGN (detached_regcache);
+};
+
 class readonly_detached_regcache;
 
 /* The register cache for storing raw register values.  */
 
-class regcache : public readable_regcache
+class regcache : public detached_regcache
 {
 public:
   regcache (gdbarch *gdbarch)
@@ -339,17 +355,11 @@ public:
   void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
 			    bool is_signed) const;
 
-  void raw_supply (int regnum, const void *buf);
-
   void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
 			   bool is_signed);
 
   void raw_supply_zeroed (int regnum);
 
-  enum register_status get_register_status (int regnum) const;
-
-  void raw_set_cached_value (int regnum, const gdb_byte *buf);
-
   void invalidate (int regnum);
 
   void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 04/10] Class readonly_detached_regcache
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (5 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 08/10] Remove regcache::m_readonly_p Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 06/10] Replace regcache::dump with class register_dump Yao Qi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

This patch adds a new class (type) for readonly regcache, which is
created via regcache::save.  readonly_detached_regcache inherits
readable_regcache.

gdb:

2017-11-28  Yao Qi  <yao.qi@linaro.org>

	* dummy-frame.c (dummy_frame_cache) <prev_regcache>: Use
	readonly_detached_regcache.
	(dummy_frame_prev_register): Use regcache->cooked_read.
	* frame.c (frame_save_as_regcache): Change return type.
	(frame_pop): Update.
	* frame.h (frame_save_as_regcache): Update declaration.
	* inferior.h (get_infcall_suspend_state_regcache): Update
	declaration.
	* infrun.c (infcall_suspend_state) <registers>: use
	readonly_detached_regcache.
	(save_infcall_suspend_state): Don't use regcache_dup.
	(get_infcall_suspend_state_regcache): Change return type.
	* linux-fork.c (struct fork_info) <savedregs>: Change to
	readonly_detached_regcache.
	<pc>: New field.
	(fork_save_infrun_state): Don't use regcache_dup.
	(info_checkpoints_command): Adjust.
	* mi/mi-main.c (register_changed_p): Update declaration.
	(mi_cmd_data_list_changed_registers): Use
	readonly_detached_regcache.
	(register_changed_p): Change parameter type to
	readonly_detached_regcache.
	* ppc-linux-tdep.c (ppu2spu_cache) <regcache>: Use
	readonly_detached_regcache.
	(ppu2spu_sniffer): Construct a new readonly_detached_regcache.
	* regcache.c (readonly_detached_regcache::readonly_detached_regcache):
	New.
	(regcache::save): Move it to reg_buffer.
	(regcache::restore): Change parameter type.
	(regcache_dup): Remove.
	* regcache.h (reg_buffer) <save>: New method.
	(readonly_detached_regcache): New class.
	* spu-tdep.c (spu2ppu_cache) <regcache>: Use
	readonly_detached_regcache.
	(spu2ppu_sniffer): Construct a new readonly_detached_regcache.
---
 gdb/dummy-frame.c    |  6 +++---
 gdb/frame.c          | 10 +++++-----
 gdb/frame.h          |  3 ++-
 gdb/inferior.h       |  2 +-
 gdb/infrun.c         |  6 +++---
 gdb/linux-fork.c     | 18 ++++++++----------
 gdb/mi/mi-main.c     | 12 ++++++------
 gdb/ppc-linux-tdep.c | 10 +++++-----
 gdb/regcache.c       | 27 +++++++++++----------------
 gdb/regcache.h       | 43 +++++++++++++++++++++++++++++++++----------
 gdb/spu-tdep.c       |  6 +++---
 11 files changed, 80 insertions(+), 63 deletions(-)

diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c
index db8ba1b..a67033e 100644
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -282,7 +282,7 @@ cleanup_dummy_frames (struct target_ops *target, int from_tty)
 struct dummy_frame_cache
 {
   struct frame_id this_id;
-  struct regcache *prev_regcache;
+  readonly_detached_regcache *prev_regcache;
 };
 
 static int
@@ -352,8 +352,8 @@ dummy_frame_prev_register (struct frame_info *this_frame,
   /* Use the regcache_cooked_read() method so that it, on the fly,
      constructs either a raw or pseudo register from the raw
      register cache.  */
-  regcache_cooked_read (cache->prev_regcache, regnum,
-			value_contents_writeable (reg_val));
+  cache->prev_regcache->cooked_read (regnum,
+				     value_contents_writeable (reg_val));
   return reg_val;
 }
 
diff --git a/gdb/frame.c b/gdb/frame.c
index 773fd04..0b04a4e 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1017,13 +1017,13 @@ do_frame_register_read (void *src, int regnum, gdb_byte *buf)
     return REG_VALID;
 }
 
-std::unique_ptr<struct regcache>
+std::unique_ptr<readonly_detached_regcache>
 frame_save_as_regcache (struct frame_info *this_frame)
 {
-  std::unique_ptr<struct regcache> regcache
-    (new struct regcache (get_frame_arch (this_frame)));
+  std::unique_ptr<readonly_detached_regcache> regcache
+    (new readonly_detached_regcache (get_frame_arch (this_frame),
+				     do_frame_register_read, this_frame));
 
-  regcache->save (do_frame_register_read, this_frame);
   return regcache;
 }
 
@@ -1057,7 +1057,7 @@ frame_pop (struct frame_info *this_frame)
      Save them in a scratch buffer so that there isn't a race between
      trying to extract the old values from the current regcache while
      at the same time writing new values into that same cache.  */
-  std::unique_ptr<struct regcache> scratch
+  std::unique_ptr<readonly_detached_regcache> scratch
     = frame_save_as_regcache (prev_frame);
 
   /* FIXME: cagney/2003-03-16: It should be possible to tell the
diff --git a/gdb/frame.h b/gdb/frame.h
index 8293a49..d5800b7 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -680,8 +680,9 @@ extern void *frame_obstack_zalloc (unsigned long size);
 #define FRAME_OBSTACK_CALLOC(NUMBER,TYPE) \
   ((TYPE *) frame_obstack_zalloc ((NUMBER) * sizeof (TYPE)))
 
+class readonly_detached_regcache;
 /* Create a regcache, and copy the frame's registers into it.  */
-std::unique_ptr<struct regcache> frame_save_as_regcache
+std::unique_ptr<readonly_detached_regcache> frame_save_as_regcache
     (struct frame_info *this_frame);
 
 extern const struct block *get_frame_block (struct frame_info *,
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 22008a0..391a5fd 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -70,7 +70,7 @@ extern struct cleanup *make_cleanup_restore_infcall_control_state
 extern void discard_infcall_suspend_state (struct infcall_suspend_state *);
 extern void discard_infcall_control_state (struct infcall_control_state *);
 
-extern struct regcache *
+extern readonly_detached_regcache *
   get_infcall_suspend_state_regcache (struct infcall_suspend_state *);
 
 extern void set_sigint_trap (void);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 742d130..92f668a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8809,7 +8809,7 @@ struct infcall_suspend_state
 
   /* Other fields:  */
   CORE_ADDR stop_pc;
-  struct regcache *registers;
+  readonly_detached_regcache *registers;
 
   /* Format of SIGINFO_DATA or NULL if it is not present.  */
   struct gdbarch *siginfo_gdbarch;
@@ -8865,7 +8865,7 @@ save_infcall_suspend_state (void)
 
   inf_state->stop_pc = stop_pc;
 
-  inf_state->registers = regcache_dup (regcache);
+  inf_state->registers = new readonly_detached_regcache (*regcache);
 
   return inf_state;
 }
@@ -8922,7 +8922,7 @@ discard_infcall_suspend_state (struct infcall_suspend_state *inf_state)
   xfree (inf_state);
 }
 
-struct regcache *
+readonly_detached_regcache *
 get_infcall_suspend_state_regcache (struct infcall_suspend_state *inf_state)
 {
   return inf_state->registers;
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index df7ea4e..9ffab1f 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -45,8 +45,9 @@ struct fork_info
   ptid_t ptid;
   ptid_t parent_ptid;
   int num;			/* Convenient handle (GDB fork id).  */
-  struct regcache *savedregs;	/* Convenient for info fork, saves
+  readonly_detached_regcache *savedregs;	/* Convenient for info fork, saves
 				   having to actually switch contexts.  */
+  CORE_ADDR pc;
   int clobber_regs;		/* True if we should restore saved regs.  */
   off_t *filepos;		/* Set of open file descriptors' offsets.  */
   int maxfd;
@@ -294,7 +295,8 @@ fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
   if (fp->savedregs)
     delete fp->savedregs;
 
-  fp->savedregs = regcache_dup (get_current_regcache ());
+  fp->savedregs = new readonly_detached_regcache (*get_current_regcache ());
+  fp->pc = regcache_read_pc (get_current_regcache ());
   fp->clobber_regs = clobber_regs;
 
   if (clobber_regs)
@@ -590,15 +592,11 @@ info_checkpoints_command (const char *arg, int from_tty)
 
       printed = fp;
       if (ptid_equal (fp->ptid, inferior_ptid))
-	{
-	  printf_filtered ("* ");
-	  pc = regcache_read_pc (get_current_regcache ());
-	}
+	printf_filtered ("* ");
       else
-	{
-	  printf_filtered ("  ");
-	  pc = regcache_read_pc (fp->savedregs);
-	}
+	printf_filtered ("  ");
+
+      pc = fp->pc;
       printf_filtered ("%d %s", fp->num, target_pid_to_str (fp->ptid));
       if (fp->num == 0)
 	printf_filtered (_(" (main process)"));
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 51d33c9..1716a7f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -96,8 +96,8 @@ static void mi_execute_cli_command (const char *cmd, int args_p,
 				    const char *args);
 static void mi_execute_async_cli_command (const char *cli_command,
 					  char **argv, int argc);
-static bool register_changed_p (int regnum, regcache *,
-				regcache *);
+static bool register_changed_p (int regnum, readonly_detached_regcache *,
+			       readonly_detached_regcache *);
 static void output_register (struct frame_info *, int regnum, int format,
 			     int skip_unavailable);
 
@@ -931,9 +931,9 @@ mi_cmd_data_list_register_names (const char *command, char **argv, int argc)
 void
 mi_cmd_data_list_changed_registers (const char *command, char **argv, int argc)
 {
-  static std::unique_ptr<struct regcache> this_regs;
+  static std::unique_ptr<readonly_detached_regcache> this_regs;
   struct ui_out *uiout = current_uiout;
-  std::unique_ptr<struct regcache> prev_regs;
+  std::unique_ptr<readonly_detached_regcache> prev_regs;
   struct gdbarch *gdbarch;
   int regnum, numregs;
   int i;
@@ -995,8 +995,8 @@ mi_cmd_data_list_changed_registers (const char *command, char **argv, int argc)
 }
 
 static bool
-register_changed_p (int regnum, struct regcache *prev_regs,
-		    struct regcache *this_regs)
+register_changed_p (int regnum, readonly_detached_regcache *prev_regs,
+		    readonly_detached_regcache *this_regs)
 {
   struct gdbarch *gdbarch = this_regs->arch ();
   struct value *prev_value, *this_value;
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 13a50d6..d6a98fc 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1262,7 +1262,7 @@ ppc_linux_spe_context (int wordsize, enum bfd_endian byte_order,
 struct ppu2spu_cache
 {
   struct frame_id frame_id;
-  struct regcache *regcache;
+  readonly_detached_regcache *regcache;
 };
 
 static struct gdbarch *
@@ -1369,10 +1369,10 @@ ppu2spu_sniffer (const struct frame_unwind *self,
 	{
 	  struct ppu2spu_cache *cache
 	    = FRAME_OBSTACK_CALLOC (1, struct ppu2spu_cache);
-	  std::unique_ptr<struct regcache> regcache
-	    (new struct regcache (data.gdbarch));
-
-	  regcache->save (ppu2spu_unwind_register, &data);
+	  std::unique_ptr<readonly_detached_regcache> regcache
+	    (new readonly_detached_regcache (data.gdbarch,
+					     ppu2spu_unwind_register,
+					     &data));
 
 	  cache->frame_id = frame_id_build (base, func);
 	  cache->regcache = regcache.release ();
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 0df6a88..ebe3c7b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -226,6 +226,11 @@ regcache::regcache (readonly_t, const regcache &src)
   save (do_cooked_read, (void *) &src);
 }
 
+readonly_detached_regcache::readonly_detached_regcache (const regcache &src)
+  : readonly_detached_regcache (src.arch (), do_cooked_read, (void *) &src)
+{
+}
+
 gdbarch *
 reg_buffer::arch () const
 {
@@ -282,16 +287,14 @@ reg_buffer::register_buffer (int regnum) const
 }
 
 void
-regcache::save (regcache_cooked_read_ftype *cooked_read,
-		void *src)
+reg_buffer::save (regcache_cooked_read_ftype *cooked_read,
+		  void *src)
 {
   struct gdbarch *gdbarch = m_descr->gdbarch;
   int regnum;
 
-  /* The DST should be `read-only', if it wasn't then the save would
-     end up trying to write the register values back out to the
-     target.  */
-  gdb_assert (m_readonly_p);
+  /* It should have pseudo registers.  */
+  gdb_assert (m_has_pseudo);
   /* Clear the dest.  */
   memset (m_registers, 0, m_descr->sizeof_cooked_registers);
   memset (m_register_status, 0, m_descr->nr_cooked_registers);
@@ -317,16 +320,14 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,
 }
 
 void
-regcache::restore (struct regcache *src)
+regcache::restore (readonly_detached_regcache *src)
 {
   struct gdbarch *gdbarch = m_descr->gdbarch;
   int regnum;
 
   gdb_assert (src != NULL);
-  /* The dst had better not be read-only.  If it is, the `restore'
-     doesn't make much sense.  */
   gdb_assert (!m_readonly_p);
-  gdb_assert (src->m_readonly_p);
+  gdb_assert (src->m_has_pseudo);
 
   gdb_assert (gdbarch == src->arch ());
 
@@ -344,12 +345,6 @@ regcache::restore (struct regcache *src)
     }
 }
 
-struct regcache *
-regcache_dup (struct regcache *src)
-{
-  return new regcache (regcache::readonly, *src);
-}
-
 enum register_status
 regcache_register_status (const struct regcache *regcache, int regnum)
 {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index e1ab2e7..d4a9d9b 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -243,6 +243,11 @@ protected:
 
   gdb_byte *register_buffer (int regnum) const;
 
+  /* Save a register cache.  The set of registers saved into the
+     regcache determined by the save_reggroup.  COOKED_READ returns
+     zero iff the register's value can't be returned.  */
+  void save (regcache_cooked_read_ftype *cooked_read, void *src);
+
   struct regcache_descr *m_descr;
 
   bool m_has_pseudo;
@@ -250,6 +255,8 @@ protected:
   gdb_byte *m_registers;
   /* Register cache status.  */
   signed char *m_register_status;
+
+  friend class regcache;
 };
 
 /* An abstract class which only has methods doing read.  */
@@ -284,6 +291,8 @@ protected:
 				  bool is_raw);
 };
 
+class readonly_detached_regcache;
+
 /* The register cache for storing raw register values.  */
 
 class regcache : public readable_regcache
@@ -307,14 +316,11 @@ public:
     return m_aspace;
   }
 
-/* Save/restore a register cache.  The set of registers saved /
-   restored into the regcache determined by the save_reggroup /
-   restore_reggroup respectively.  COOKED_READ returns zero iff the
-   register's value can't be returned.  */
-  void save (regcache_cooked_read_ftype *cooked_read, void *src);
-  /* Writes to regcache will go through to the target.  SRC is a
+  /* Restore a register cache.  The set of registers restored into
+     the regcache determined by the restore_reggroup.
+     Writes to regcache will go through to the target.  SRC is a
      read-only register cache.  */
-  void restore (struct regcache *src);
+  void restore (readonly_detached_regcache *src);
 
   void cooked_write (int regnum, const gdb_byte *buf);
 
@@ -412,9 +418,26 @@ private:
   registers_changed_ptid (ptid_t ptid);
 };
 
-/* Duplicate the contents of a register cache to a read-only register
-   cache.  The operation is pass-through.  */
-extern struct regcache *regcache_dup (struct regcache *regcache);
+class readonly_detached_regcache : public readable_regcache
+{
+public:
+  readonly_detached_regcache (const regcache &src);
+
+  /* Create a readonly regcache by getting contents from COOKED_READ.  */
+
+  readonly_detached_regcache (gdbarch *gdbarch,
+			      regcache_cooked_read_ftype *cooked_read,
+			      void *src)
+    : readable_regcache (gdbarch, true)
+  {
+    save (cooked_read, src);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (readonly_detached_regcache);
+
+  void raw_update (int regnum) override
+  {}
+};
 
 extern void registers_changed (void);
 extern void registers_changed_ptid (ptid_t);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index da7b937..a632c06 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1202,7 +1202,7 @@ spu_write_pc (struct regcache *regcache, CORE_ADDR pc)
 struct spu2ppu_cache
 {
   struct frame_id frame_id;
-  struct regcache *regcache;
+  readonly_detached_regcache *regcache;
 };
 
 static struct gdbarch *
@@ -1229,7 +1229,7 @@ spu2ppu_prev_register (struct frame_info *this_frame,
   gdb_byte *buf;
 
   buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));
-  regcache_cooked_read (cache->regcache, regnum, buf);
+  cache->regcache->cooked_read (regnum, buf);
   return frame_unwind_got_bytes (this_frame, regnum, buf);
 }
 
@@ -1274,7 +1274,7 @@ spu2ppu_sniffer (const struct frame_unwind *self,
 	{
 	  struct regcache *regcache;
 	  regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
-	  cache->regcache = regcache_dup (regcache);
+	  cache->regcache = new readonly_detached_regcache (*regcache);
 	  *this_prologue_cache = cache;
 	  return 1;
 	}
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 00/10 v2] Remove regcache::m_readonly_p
@ 2018-02-07 10:33 Yao Qi
  2018-02-07 10:33 ` [PATCH 10/10] Pass readable_regcache to gdbarch method read_pc Yao Qi
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

regcache is used in many places in gdb in different ways, so regcache
becomes a flat and fat object.  That exposes some unnecessary APIs to
different part, and some APIs are misused or abused:

 1) gdbarch methods pseudo_register_read, pseudo_register_read_value,
 read_pc have a parameter 'regcache *', but these two gdbarch methods
 only need raw_read* and cooked_read* methods.  So it is better to
 pass a class which only has raw_read* and cooked_read* methods, and
 other regcache methods are invisible to each gdbarch implementation.

 2) target_ops methods to_fetch_registers and to_store_registers have a
 parameter 'regcache *', but these two target_ops methods only need
 raw_supply and raw_collect methods, because raw registers are from target
 layer, pseudo registers are "composed" or "created" by gdbarch.

 3) jit.c uses regcache in an odd way, and record-full.c should use
 a simple version regcache instead of an array (see patch 11)

Beside these api issues, one issue in regcache is that there is no
type or class for readonly regcache.  We use a flag field m_readonly_p
to indicate the regcache is readonly or not, so some regcache apis have
assert that this regcache is or is not readonly.  The better way to do
this is to create a new class for readonly regcache which doesn't have
write methods at all.

This patch series fixes all of the problems above except 2) (I had a
patch to fix 2 in my tree, but still need more time to polish it.) by
designing a class hierarchy about regcache, like this,

                      reg_buffer
                         ^
                         |
                   ------+-----
                   ^
                   |
            readable_regcache
                 ^
                 |
           ------+------
           ^           ^
           |           |
    detached_regcache readonly_detached_regcache
          ^
          |
      regcache

Class reg_buffer is a simple class, having register contents and status
(in patch 1).  readable_regcache is an abstract class only having raw_read*
and cooked_read* methods (in patch 2).  detached_regcache is a class which
has read and write methods, but it detaches from target, IOW, the
write doesn't go through.  Class readonly_detached_regcache is the readonly
regcache, created from regcache::save method.

This is the v2 of this patch series, v1 can be found
https://sourceware.org/ml/gdb-patches/2017-12/msg00014.html Some
changes compared with v1,

 - Some of the preparatory patches in v1 are already committed,
 - Rename some classes,
 - Pass readable_regcache to gdbarch read_pc.  We can pass readable_regcache
   to gdbarch breakpoint_kind_from_current_state as well, because this
   gdbarch method doesn't need to write regcache.  The reason I don't
   that is arm_breakpoint_kind_from_current_state uses a function
   arm_get_next_pcs_ctor shared between GDB and GDBserver, and I don't
   propagate the regcache changes to GDBserver at this moment,

This patch series is pushed to users/qiyao/regcache-split-4-2.  Regression
tested on {x86_64,aarch64}-linux.

*** BLURB HERE ***

Yao Qi (10):
  Class reg_buffer
  class readable_regcache and pass readable_regcache to gdbarch
    pseudo_register_read and pseudo_register_read_value
  Remove regcache_save and regcache_cpy
  Class readonly_detached_regcache
  Class detached_regcache
  Replace regcache::dump with class register_dump
  No longer create readonly regcache
  Remove regcache::m_readonly_p
  Move register_dump to regcache-dump.c
  Pass readable_regcache to gdbarch method read_pc

 gdb/Makefile.in      |   1 +
 gdb/aarch64-tdep.c   |   2 +-
 gdb/amd64-tdep.c     |   2 +-
 gdb/arm-tdep.c       |   6 +-
 gdb/avr-tdep.c       |   7 +-
 gdb/bfin-tdep.c      |   2 +-
 gdb/dummy-frame.c    |   6 +-
 gdb/frame.c          |  15 +-
 gdb/frame.h          |   3 +-
 gdb/frv-tdep.c       |   2 +-
 gdb/gdbarch.c        |   6 +-
 gdb/gdbarch.h        |  12 +-
 gdb/gdbarch.sh       |   6 +-
 gdb/h8300-tdep.c     |   4 +-
 gdb/hppa-tdep.c      |   8 +-
 gdb/i386-tdep.c      |   6 +-
 gdb/i386-tdep.h      |   2 +-
 gdb/ia64-tdep.c      |   8 +-
 gdb/infcmd.c         |   6 +-
 gdb/inferior.h       |   2 +-
 gdb/infrun.c         |   8 +-
 gdb/jit.c            |  10 +-
 gdb/linux-fork.c     |  20 ++-
 gdb/m32c-tdep.c      |  20 +--
 gdb/m68hc11-tdep.c   |   2 +-
 gdb/mep-tdep.c       |   6 +-
 gdb/mi/mi-main.c     |  12 +-
 gdb/mips-tdep.c      |   6 +-
 gdb/msp430-tdep.c    |   2 +-
 gdb/nds32-tdep.c     |   2 +-
 gdb/ppc-linux-tdep.c |  10 +-
 gdb/record-full.c    |  21 ++-
 gdb/regcache-dump.c  | 335 ++++++++++++++++++++++++++++++++++++++
 gdb/regcache.c       | 445 +++++++++++++--------------------------------------
 gdb/regcache.h       | 244 ++++++++++++++++------------
 gdb/rl78-tdep.c      |   2 +-
 gdb/rs6000-tdep.c    |  52 ++++--
 gdb/s390-tdep.c      |   2 +-
 gdb/sh-tdep.c        |   4 +-
 gdb/sh64-tdep.c      |   4 +-
 gdb/sparc-tdep.c     |   2 +-
 gdb/sparc64-tdep.c   |   2 +-
 gdb/spu-tdep.c       |  15 +-
 gdb/xtensa-tdep.c    |   4 +-
 44 files changed, 757 insertions(+), 579 deletions(-)
 create mode 100644 gdb/regcache-dump.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/10] Class reg_buffer
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (2 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 07/10] No longer create readonly regcache Yao Qi
@ 2018-02-07 10:33 ` Yao Qi
  2018-02-07 10:33 ` [PATCH 02/10] class readable_regcache and pass readable_regcache to gdbarch pseudo_register_read and pseudo_register_read_value Yao Qi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-07 10:33 UTC (permalink / raw)
  To: gdb-patches

This patch adds a new class reg_buffer, and regcache inherits it.  Class
reg_buffer is a very simple class, which has the buffer for register
contents and status only.  It doesn't have any methods to set contents and
status, and it is expected that its children classes can inherit it and
add different access methods.

Another reason I keep class reg_buffer so simple is that I think
reg_buffer can be even reused in other classes which need to record the
registers contents and status, like frame cache for example.

gdb:

2017-11-27  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (regcache::regcache): Call reg_buffer ctor.
	(regcache::arch): Move it to reg_buffer::arch.
	(regcache::register_buffer): Likewise.
	(regcache::assert_regnum): Likewise.
	(regcache::num_raw_registers): Likewise.
	* regcache.h (reg_buffer): New class.
	(regcache): Inherit reg_buffer.
---
 gdb/regcache.c | 31 ++++++++++++++++++++---------
 gdb/regcache.h | 62 ++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 58 insertions(+), 35 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index ab750c6..8addc82 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -181,14 +181,13 @@ regcache_register_size (const struct regcache *regcache, int n)
   return register_size (regcache->arch (), n);
 }
 
-regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
-		    bool readonly_p_)
-  : m_aspace (aspace_), m_readonly_p (readonly_p_)
+reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo)
+  : m_has_pseudo (has_pseudo)
 {
   gdb_assert (gdbarch != NULL);
   m_descr = regcache_descr (gdbarch);
 
-  if (m_readonly_p)
+  if (has_pseudo)
     {
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_cooked_registers);
       m_register_status = XCNEWVEC (signed char,
@@ -199,6 +198,16 @@ regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_raw_registers);
       m_register_status = XCNEWVEC (signed char, gdbarch_num_regs (gdbarch));
     }
+}
+
+regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
+		    bool readonly_p_)
+/* The register buffers.  A read-only register cache can hold the
+   full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a
+   read/write register cache can only hold [0 .. gdbarch_num_regs).  */
+  : reg_buffer (gdbarch, readonly_p_),
+    m_aspace (aspace_), m_readonly_p (readonly_p_)
+{
   m_ptid = minus_one_ptid;
 }
 
@@ -218,7 +227,7 @@ regcache::regcache (readonly_t, const regcache &src)
 }
 
 gdbarch *
-regcache::arch () const
+reg_buffer::arch () const
 {
   return m_descr->gdbarch;
 }
@@ -267,7 +276,7 @@ private:
 /* Return  a pointer to register REGNUM's buffer cache.  */
 
 gdb_byte *
-regcache::register_buffer (int regnum) const
+reg_buffer::register_buffer (int regnum) const
 {
   return m_registers + m_descr->register_offset[regnum];
 }
@@ -390,9 +399,13 @@ regcache::invalidate (int regnum)
 }
 
 void
-regcache::assert_regnum (int regnum) const
+reg_buffer::assert_regnum (int regnum) const
 {
-  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch ()));
+  gdb_assert (regnum >= 0);
+  if (m_has_pseudo)
+    gdb_assert (regnum < m_descr->nr_cooked_registers);
+  else
+    gdb_assert (regnum < gdbarch_num_regs (arch ()));
 }
 
 /* Global structure containing the current regcache.  */
@@ -1272,7 +1285,7 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc)
 }
 
 int
-regcache::num_raw_registers () const
+reg_buffer::num_raw_registers () const
 {
   return gdbarch_num_regs (arch ());
 }
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 9e3da8c..e2c7621 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -227,9 +227,44 @@ typedef struct cached_reg
   gdb_byte *data;
 } cached_reg_t;
 
+/* Buffer of registers.  */
+
+class reg_buffer
+{
+public:
+  reg_buffer (gdbarch *gdbarch, bool has_pseudo);
+
+  DISABLE_COPY_AND_ASSIGN (reg_buffer);
+
+  /* Return regcache's architecture.  */
+  gdbarch *arch () const;
+
+  virtual ~reg_buffer ()
+  {
+    xfree (m_registers);
+    xfree (m_register_status);
+  }
+
+protected:
+  /* Assert on the range of REGNUM.  */
+  void assert_regnum (int regnum) const;
+
+  int num_raw_registers () const;
+
+  gdb_byte *register_buffer (int regnum) const;
+
+  struct regcache_descr *m_descr;
+
+  bool m_has_pseudo;
+  /* The register buffers.  */
+  gdb_byte *m_registers;
+  /* Register cache status.  */
+  signed char *m_register_status;
+};
+
 /* The register cache for storing raw register values.  */
 
-class regcache
+class regcache : public reg_buffer
 {
 public:
   regcache (gdbarch *gdbarch)
@@ -244,15 +279,6 @@ public:
 
   DISABLE_COPY_AND_ASSIGN (regcache);
 
-  ~regcache ()
-  {
-    xfree (m_registers);
-    xfree (m_register_status);
-  }
-
-  /* Return regcache's architecture.  */
-  gdbarch *arch () const;
-
   /* Return REGCACHE's address space.  */
   const address_space *aspace () const
   {
@@ -339,14 +365,9 @@ public:
   static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
 protected:
   regcache (gdbarch *gdbarch, const address_space *aspace_, bool readonly_p_);
-
-  int num_raw_registers () const;
-
   static std::forward_list<regcache *> current_regcache;
 
 private:
-  gdb_byte *register_buffer (int regnum) const;
-
   void restore (struct regcache *src);
 
   enum register_status xfer_part (int regnum, int offset, int len, void *in,
@@ -357,21 +378,10 @@ private:
 			int regnum, const void *in_buf,
 			void *out_buf, size_t size) const;
 
-  /* Assert on the range of REGNUM.  */
-  void assert_regnum (int regnum) const;
-
-  struct regcache_descr *m_descr;
-
   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
   const address_space * const m_aspace;
 
-  /* The register buffers.  A read-only register cache can hold the
-     full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a read/write
-     register cache can only hold [0 .. gdbarch_num_regs).  */
-  gdb_byte *m_registers;
-  /* Register cache status.  */
-  signed char *m_register_status;
   /* Is this a read-only cache?  A read-only cache is used for saving
      the target's register state (e.g, across an inferior function
      call or just before forcing a function return).  A read-only
-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 03/10] Remove regcache_save and regcache_cpy
  2018-02-07 10:33 ` [PATCH 03/10] Remove regcache_save and regcache_cpy Yao Qi
@ 2018-02-17  2:07   ` Simon Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-02-17  2:07 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 2018-02-07 05:32, Yao Qi wrote:
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -198,20 +198,10 @@ extern struct type *register_type (struct
> gdbarch *gdbarch, int regnum);
> 
>  extern int register_size (struct gdbarch *gdbarch, int regnum);
> 
> -
> -/* Save/restore a register cache.  The set of registers saved /
> -   restored into the DST regcache determined by the save_reggroup /
> -   restore_reggroup respectively.  COOKED_READ returns zero iff the
> -   register's value can't be returned.  */
> -
>  typedef enum register_status (regcache_cooked_read_ftype) (void *src,
>  							   int regnum,
>  							   gdb_byte *buf);
> 
> -extern void regcache_save (struct regcache *dst,
> -			   regcache_cooked_read_ftype *cooked_read,
> -			   void *cooked_read_context);
> -
>  enum regcache_dump_what
>  {
>    regcache_dump_none, regcache_dump_raw,
> @@ -317,7 +307,14 @@ public:
>      return m_aspace;
>    }
> 
> +/* Save/restore a register cache.  The set of registers saved /
> +   restored into the regcache determined by the save_reggroup /
> +   restore_reggroup respectively.  COOKED_READ returns zero iff the
> +   register's value can't be returned.  */
>    void save (regcache_cooked_read_ftype *cooked_read, void *src);
> +  /* Writes to regcache will go through to the target.  SRC is a
> +     read-only register cache.  */
> +  void restore (struct regcache *src);

Nit: add an empty line after the declaration of save.

Also, would it be possible to improve the comment of save?  I read them 
multiple times, and I still haven't figured out what the function does 
exactly.  It saves the content of this regcache in another regcache?  Or 
it saves another regcache in this regcache?  How are the parameters used 
exactly?

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 05/10] Class detached_regcache
  2018-02-07 10:33 ` [PATCH 05/10] Class detached_regcache Yao Qi
@ 2018-02-17  3:15   ` Simon Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-02-17  3:15 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

Just some nits.

On 2018-02-07 05:32, Yao Qi wrote:
> jit.c uses the regcache in a slightly different way, the regcache 
> dosn't

"doesn't"

> write through to target, but it has read and write methods.  If I apply
> regcache in record-full.c, it has the similar use pattern.  This patch
> adds a new class detached_regcache, a register buffer, but can be
> red and written.

"read"

> 
> Since jit.c doesn't want to write registers through to target, it uses
> regcache as a readonly regcache (because only readonly regcache
> disconnects from the target), but it adds a hole in regcache
> (raw_set_cached_value) in order to modify a readonly regcache.  This 
> patch
> fixes this hole completely.
> 
> regcache inherits detached_regcache, and detached_regcache inherits
> readable_regcache.  The ideal design is that both detached_regcache and
> readable_regcache inherit reg_buffer, and regcache inherit
> detached_regcache and regcache_read (virtual inheritance).  I concern
> about the performance overhead of virtual inheritance, so I don't do it 
> in
> the patch.
> 
> gdb:
> 
> 2017-11-28  Yao Qi  <yao.qi@linaro.org>
> 
> 	* jit.c (struct jit_unwind_private) <regcache>: Change its type to
> 	 reg_buffer_rw *.
> 	(jit_unwind_reg_set_impl): Call raw_supply.
> 	(jit_frame_sniffer): Use reg_buffer_rw.
> 	* record-full.c (record_full_core_regbuf): Change its type.
> 	(record_full_core_open_1): Use reg_buffer_rw.
> 	(record_full_close): Likewise.
> 	(record_full_core_fetch_registers): Use regcache->raw_supply.
> 	(record_full_core_store_registers): Likewise.
> 	* regcache.c (regcache::get_register_status): Move it to
> 	reg_buffer.
> 	(regcache_raw_set_cached_value): Remove.
> 	(regcache::raw_set_cached_value): Remove.
> 	(regcache::raw_write): Call raw_supply.
> 	(regcache::raw_supply): Move it to reg_buffer_rw.
> 	* regcache.h (regcache_raw_set_cached_value): Remove.
> 	(reg_buffer_rw): New class.
> ---
>  gdb/jit.c         | 10 ++++------
>  gdb/record-full.c | 21 +++++++++------------
>  gdb/regcache.c    | 32 +++++---------------------------
>  gdb/regcache.h    | 42 ++++++++++++++++++++++++++----------------
>  4 files changed, 44 insertions(+), 61 deletions(-)
> 
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 01ead45..e67e1d2 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -1097,7 +1097,7 @@ struct jit_unwind_private
>  {
>    /* Cached register values.  See jit_frame_sniffer to see how this
>       works.  */
> -  struct regcache *regcache;
> +  detached_regcache *regcache;
> 
>    /* The frame being unwound.  */
>    struct frame_info *this_frame;
> @@ -1126,7 +1126,7 @@ jit_unwind_reg_set_impl (struct
> gdb_unwind_callbacks *cb, int dwarf_regnum,
>        return;
>      }
> 
> -  regcache_raw_set_cached_value (priv->regcache, gdb_reg, 
> value->value);
> +  priv->regcache->raw_supply (gdb_reg, value->value);
>    value->free (value);
>  }
> 
> @@ -1188,7 +1188,6 @@ jit_frame_sniffer (const struct frame_unwind 
> *self,
>    struct jit_unwind_private *priv_data;
>    struct gdb_unwind_callbacks callbacks;
>    struct gdb_reader_funcs *funcs;
> -  struct gdbarch *gdbarch;
> 
>    callbacks.reg_get = jit_unwind_reg_get_impl;
>    callbacks.reg_set = jit_unwind_reg_set_impl;
> @@ -1201,11 +1200,10 @@ jit_frame_sniffer (const struct frame_unwind 
> *self,
> 
>    gdb_assert (!*cache);
> 
> -  gdbarch = get_frame_arch (this_frame);
> -
>    *cache = XCNEW (struct jit_unwind_private);
>    priv_data = (struct jit_unwind_private *) *cache;
> -  priv_data->regcache = new regcache (gdbarch);
> +  /* Take a snapshot of current regcache.  */
> +  priv_data->regcache = new detached_regcache (get_frame_arch
> (this_frame), true);

This line is too long.

> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index d4a9d9b..9a1ac41 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -68,14 +68,6 @@ extern void regcache_raw_write_unsigned (struct
> regcache *regcache,
>  extern LONGEST regcache_raw_get_signed (struct regcache *regcache,
>  					int regnum);
> 
> -/* Set a raw register's value in the regcache's buffer.  Unlike
> -   regcache_raw_write, this is not write-through.  The intention is
> -   allowing to change the buffer contents of a read-only regcache
> -   allocated with new.  */
> -
> -extern void regcache_raw_set_cached_value
> -  (struct regcache *regcache, int regnum, const gdb_byte *buf);
> -
>  /* Partial transfer of raw registers.  These perform read, modify,
>     write style operations.  The read variant returns the status of the
>     register.  */
> @@ -229,12 +221,13 @@ public:
>    /* Return regcache's architecture.  */
>    gdbarch *arch () const;
> 
> +  enum register_status get_register_status (int regnum) const;
> +
>    virtual ~reg_buffer ()
>    {
>      xfree (m_registers);
>      xfree (m_register_status);
>    }
> -
>  protected:
>    /* Assert on the range of REGNUM.  */
>    void assert_regnum (int regnum) const;
> @@ -257,6 +250,7 @@ protected:
>    signed char *m_register_status;
> 
>    friend class regcache;
> +  friend class detached_regcache;
>  };
> 
>  /* An abstract class which only has methods doing read.  */
> @@ -291,11 +285,33 @@ protected:
>  				  bool is_raw);
>  };
> 
> +/* Buffer of registers, can be red and written.  */

"read"

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-07 10:33 ` [PATCH 09/10] Move register_dump to regcache-dump.c Yao Qi
@ 2018-02-17  3:50   ` Simon Marchi
  2018-02-21  8:27     ` Yao Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-02-17  3:50 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Hi Yao,

I think it would make sense to move the remaining dump-related things 
(the register_dump class, the register_dump::dump implementation) to 
regcache-dump.c.

For all empty destructors, you can use "= default" instead of {}.

> diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c
> new file mode 100644
> index 0000000..4780fc4
> --- /dev/null
> +++ b/gdb/regcache-dump.c
> @@ -0,0 +1,335 @@
> +/* Copyright (C) 1986-2017 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or 
> modify
> +   it under the terms of the GNU General Public License as published 
> by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see 
> <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "gdbcmd.h"
> +#include "regcache.h"
> +#include "common/def-vector.h"
> +#include "valprint.h"
> +#include "remote.h"
> +#include "reggroups.h"
> +#include "target.h"
> +
> +/* Dump registers from regcache, used for dump raw registers and
> +   cooked registers.  */

I just saw it now, but you could say either "used for dumping" or "used 
to dump", but "for dump" doesn't sound right.

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/10 v2] Remove regcache::m_readonly_p
  2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
                   ` (9 preceding siblings ...)
  2018-02-07 10:33 ` [PATCH 05/10] Class detached_regcache Yao Qi
@ 2018-02-17  3:53 ` Simon Marchi
  2018-02-21 11:22   ` Yao Qi
  10 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-02-17  3:53 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 2018-02-07 05:32, Yao Qi wrote:
> regcache is used in many places in gdb in different ways, so regcache
> becomes a flat and fat object.  That exposes some unnecessary APIs to
> different part, and some APIs are misused or abused:
> 
>  1) gdbarch methods pseudo_register_read, pseudo_register_read_value,
>  read_pc have a parameter 'regcache *', but these two gdbarch methods
>  only need raw_read* and cooked_read* methods.  So it is better to
>  pass a class which only has raw_read* and cooked_read* methods, and
>  other regcache methods are invisible to each gdbarch implementation.
> 
>  2) target_ops methods to_fetch_registers and to_store_registers have a
>  parameter 'regcache *', but these two target_ops methods only need
>  raw_supply and raw_collect methods, because raw registers are from 
> target
>  layer, pseudo registers are "composed" or "created" by gdbarch.
> 
>  3) jit.c uses regcache in an odd way, and record-full.c should use
>  a simple version regcache instead of an array (see patch 11)
> 
> Beside these api issues, one issue in regcache is that there is no
> type or class for readonly regcache.  We use a flag field m_readonly_p
> to indicate the regcache is readonly or not, so some regcache apis have
> assert that this regcache is or is not readonly.  The better way to do
> this is to create a new class for readonly regcache which doesn't have
> write methods at all.
> 
> This patch series fixes all of the problems above except 2) (I had a
> patch to fix 2 in my tree, but still need more time to polish it.) by
> designing a class hierarchy about regcache, like this,
> 
>                       reg_buffer
>                          ^
>                          |
>                    ------+-----
>                    ^
>                    |
>             readable_regcache
>                  ^
>                  |
>            ------+------
>            ^           ^
>            |           |
>     detached_regcache readonly_detached_regcache
>           ^
>           |
>       regcache
> 
> Class reg_buffer is a simple class, having register contents and status
> (in patch 1).  readable_regcache is an abstract class only having 
> raw_read*
> and cooked_read* methods (in patch 2).  detached_regcache is a class 
> which
> has read and write methods, but it detaches from target, IOW, the
> write doesn't go through.  Class readonly_detached_regcache is the 
> readonly
> regcache, created from regcache::save method.
> 
> This is the v2 of this patch series, v1 can be found
> https://sourceware.org/ml/gdb-patches/2017-12/msg00014.html Some
> changes compared with v1,
> 
>  - Some of the preparatory patches in v1 are already committed,
>  - Rename some classes,
>  - Pass readable_regcache to gdbarch read_pc.  We can pass 
> readable_regcache
>    to gdbarch breakpoint_kind_from_current_state as well, because this
>    gdbarch method doesn't need to write regcache.  The reason I don't
>    that is arm_breakpoint_kind_from_current_state uses a function
>    arm_get_next_pcs_ctor shared between GDB and GDBserver, and I don't
>    propagate the regcache changes to GDBserver at this moment,
> 
> This patch series is pushed to users/qiyao/regcache-split-4-2.  
> Regression
> tested on {x86_64,aarch64}-linux.

Hi Yao,

I did not spot anything fundamentally wrong in the series, though I'm 
not very proficient in this area.  I sent a few minor comments, but 
other than that I'm fine with the series.  It's hard to tell whether 
it's the right design, but I think it improves things by splitting the 
concerns in different classes rather than having one do-it-all class.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-17  3:50   ` Simon Marchi
@ 2018-02-21  8:27     ` Yao Qi
  2018-02-21 12:57       ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2018-02-21  8:27 UTC (permalink / raw)
  To: Simon Marchi; +Cc: GDB Patches

On Sat, Feb 17, 2018 at 3:50 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> I think it would make sense to move the remaining dump-related things (the
> register_dump class, the register_dump::dump implementation) to
> regcache-dump.c.
>

register_dump::dump accesses 'struct regcache_desc', which is declared in
regcache.c, so that register_dump::dump can't be moved to  regcache-dump.c.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/10 v2] Remove regcache::m_readonly_p
  2018-02-17  3:53 ` [PATCH 00/10 v2] Remove regcache::m_readonly_p Simon Marchi
@ 2018-02-21 11:22   ` Yao Qi
  0 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2018-02-21 11:22 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@polymtl.ca> writes:

> I did not spot anything fundamentally wrong in the series, though I'm
> not very proficient in this area.  I sent a few minor comments, but
> other than that I'm fine with the series.  It's hard to tell whether
> it's the right design, but I think it improves things by splitting the
> concerns in different classes rather than having one do-it-all class.

Hi Simon,
Thanks for the review.  I update these patches as you commented.  I
pushed them in.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-21  8:27     ` Yao Qi
@ 2018-02-21 12:57       ` Simon Marchi
  2018-02-21 13:59         ` Yao Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-02-21 12:57 UTC (permalink / raw)
  To: Yao Qi; +Cc: GDB Patches

On 2018-02-21 03:27, Yao Qi wrote:
> On Sat, Feb 17, 2018 at 3:50 AM, Simon Marchi <simon.marchi@polymtl.ca> 
> wrote:
>> I think it would make sense to move the remaining dump-related things 
>> (the
>> register_dump class, the register_dump::dump implementation) to
>> regcache-dump.c.
>> 
> 
> register_dump::dump accesses 'struct regcache_desc', which is declared 
> in
> regcache.c, so that register_dump::dump can't be moved to  
> regcache-dump.c.

Is there any reason we can't move struct regcache_descr to regcache.h?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-21 12:57       ` Simon Marchi
@ 2018-02-21 13:59         ` Yao Qi
  2018-02-21 14:49           ` Simon Marchi
  0 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2018-02-21 13:59 UTC (permalink / raw)
  To: Simon Marchi; +Cc: GDB Patches

On Wed, Feb 21, 2018 at 12:57 PM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> register_dump::dump accesses 'struct regcache_desc', which is declared in
>> regcache.c, so that register_dump::dump can't be moved to
>> regcache-dump.c.
>
>
> Is there any reason we can't move struct regcache_descr to regcache.h?

The reason I didn't move is that I want to keep regcache_descr opaque.
To be clear, I don't mind moving regcache_descr to regcache.h and move
all regcache dump stuff into regcache-dump.{c,h}.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/10] Move register_dump to regcache-dump.c
  2018-02-21 13:59         ` Yao Qi
@ 2018-02-21 14:49           ` Simon Marchi
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-02-21 14:49 UTC (permalink / raw)
  To: Yao Qi; +Cc: GDB Patches

On 2018-02-21 08:58, Yao Qi wrote:
> On Wed, Feb 21, 2018 at 12:57 PM, Simon Marchi 
> <simon.marchi@polymtl.ca> wrote:
>>> 
>>> register_dump::dump accesses 'struct regcache_desc', which is 
>>> declared in
>>> regcache.c, so that register_dump::dump can't be moved to
>>> regcache-dump.c.
>> 
>> 
>> Is there any reason we can't move struct regcache_descr to regcache.h?
> 
> The reason I didn't move is that I want to keep regcache_descr opaque.
> To be clear, I don't mind moving regcache_descr to regcache.h and move
> all regcache dump stuff into regcache-dump.{c,h}.

I think it would make a more logical split, but I'll let you decide :)

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-02-21 14:49 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 10:33 [PATCH 00/10 v2] Remove regcache::m_readonly_p Yao Qi
2018-02-07 10:33 ` [PATCH 10/10] Pass readable_regcache to gdbarch method read_pc Yao Qi
2018-02-07 10:33 ` [PATCH 03/10] Remove regcache_save and regcache_cpy Yao Qi
2018-02-17  2:07   ` Simon Marchi
2018-02-07 10:33 ` [PATCH 07/10] No longer create readonly regcache Yao Qi
2018-02-07 10:33 ` [PATCH 01/10] Class reg_buffer Yao Qi
2018-02-07 10:33 ` [PATCH 02/10] class readable_regcache and pass readable_regcache to gdbarch pseudo_register_read and pseudo_register_read_value Yao Qi
2018-02-07 10:33 ` [PATCH 08/10] Remove regcache::m_readonly_p Yao Qi
2018-02-07 10:33 ` [PATCH 04/10] Class readonly_detached_regcache Yao Qi
2018-02-07 10:33 ` [PATCH 06/10] Replace regcache::dump with class register_dump Yao Qi
2018-02-07 10:33 ` [PATCH 09/10] Move register_dump to regcache-dump.c Yao Qi
2018-02-17  3:50   ` Simon Marchi
2018-02-21  8:27     ` Yao Qi
2018-02-21 12:57       ` Simon Marchi
2018-02-21 13:59         ` Yao Qi
2018-02-21 14:49           ` Simon Marchi
2018-02-07 10:33 ` [PATCH 05/10] Class detached_regcache Yao Qi
2018-02-17  3:15   ` Simon Marchi
2018-02-17  3:53 ` [PATCH 00/10 v2] Remove regcache::m_readonly_p Simon Marchi
2018-02-21 11:22   ` Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).