[PATCH 4/7] Regcache: Refactor dup/cpy/save/restore

[Alan Hayward <Alan dot Hayward at arm dot com>]

This patch refactors and simplifies the methods for copying a regcache.

In the existing code, save/restore/dup/cpy differed on whether the dest and
source were readonly or not readonly.
These changes change the dependency to whether the dest and source are a
target_regcache or regcache.

The functions are now:

save () : This is deleted from target_regcache, ensuring the destination
is always a detached regcache. The code does not care if the source is a
target_regache or not.

restore_to () : Replacement for restore. This is deleted from target_regcache.
It ensures that the target is a regcache and the source is a target_regcache.

dup () : There is a different regcache and target_regcache version of this.
Both versions return a detached regcache.

regcache_cpy () : Removed. The above methods give everything required.

regcache(readonly_t, regcache*) : Removed. Use dup() instead.



Tested on a --enable-targets=all build with board files unix,
native-gdbserver and unittest.exp.



2017-08-16  Alan Hayward  <alan.hayward@arm.com>

	* frame.c (frame_save_as_regcache): Use save.
	(frame_pop): Use restore_to.
	* infcmd.c (get_return_value): Use dup.
	* infrun.c (save_infcall_suspend_state): Likewise.
	(restore_infcall_suspend_state): Use restore_to.
	* linux-fork.c (fork_load_infrun_state): Likewise.
	(fork_save_infrun_state): Use dup.
	* ppc-linux-tdep.c (ppu2spu_sniffer): Use save.
	* regcache.c (regcache::regcache): Remove function.
	(regcache_save): Remove function.
	(regcache::save): Remove checks.
	(regcache::restore_to): New version of...
	(regcache::restore): ...this.
	(regcache::cpy): Remove function. 
	(regcache::dup): Copy from regcache.
	(target_regcache::dup): Likewise.
	(regcache_dup): Remove function. 
	* spu-tdep.c (spu2ppu_sniffer): Use dup.


diff --git a/gdb/frame.c b/gdb/frame.c
index 7fd4b07a2e95f28b2eb6a18dea4d2071f0ece4e2..30cd8532e6dcb91510c527f0c3c524558a7a6058 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1024,7 +1024,7 @@ frame_save_as_regcache (struct frame_info *this_frame)
  regcache *backup = new regcache (get_frame_arch (this_frame), aspace);
  struct cleanup *cleanups = make_cleanup_regcache_delete (backup);

-  regcache_save (backup, do_frame_register_read, this_frame);
+  backup->save (do_frame_register_read, this_frame);
  discard_cleanups (cleanups);
  return backup;
}
@@ -1072,9 +1072,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);
+  /* Now copy those saved registers into the current regcache.  */
+  scratch->restore_to (get_current_regcache ());
  do_cleanups (cleanups);

  /* We've made right mess of GDB's local state, just discard
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index defa7b0c48cd3a1a90786ba3d883a986247a3b5d..52da4ba0e68417b7b515d1382f3f107e9d262130 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1602,8 +1602,8 @@ advance_command (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 ()->dup ();
+  struct gdbarch *gdbarch = stop_regs->arch ();
  struct value *value;

  value_type = check_typedef (value_type);
@@ -1623,7 +1623,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:
@@ -1633,6 +1633,7 @@ get_return_value (struct value *function, struct type *value_type)
      internal_error (__FILE__, __LINE__, _("bad switch"));
    }

+  delete stop_regs;
  return value;
}

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6510aec548f6495ca64f5b566ff3628734698113..485cdccf133a055382b569c9d20a6f2f76c4c0ba 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8911,7 +8911,7 @@ save_infcall_suspend_state (void)

  inf_state->stop_pc = stop_pc;

-  inf_state->registers = regcache_dup (regcache);
+  inf_state->registers = regcache->dup ();

  return inf_state;
}
@@ -8922,7 +8922,7 @@ void
restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
{
  struct thread_info *tp = inferior_thread ();
-  struct regcache *regcache = get_current_regcache ();
+  target_regcache *regcache = get_current_regcache ();
  struct gdbarch *gdbarch = get_regcache_arch (regcache);

  tp->suspend = inf_state->thread_suspend;
@@ -8942,7 +8942,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);
+    inf_state->registers->restore_to (regcache);

  discard_infcall_suspend_state (inf_state);
}
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 032ff62807b728be336bf0e5daf59e47615d4799..d307aa7162cdd2b8ac1536e4eb1a1936ed6766f5 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -264,7 +264,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);
+     fp->savedregs->restore_to (get_current_regcache ());

  registers_changed ();
  reinit_frame_cache ();
@@ -297,7 +297,7 @@ 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 = get_current_regcache ()->dup ();
  fp->clobber_regs = clobber_regs;

  if (clobber_regs)
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 42aff2cd1bf3834268b0b58dcf00dac1c52add96..285ab74cbf656f501805663b4e1063f28d5269d0 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1366,7 +1366,7 @@ ppu2spu_sniffer (const struct frame_unwind *self,
	  struct address_space *aspace = get_frame_address_space (this_frame);
	  regcache *backup = new regcache (data.gdbarch, aspace);
	  struct cleanup *cleanups = make_cleanup_regcache_delete (backup);
-	  regcache_save (backup, ppu2spu_unwind_register, &data);
+	  backup->save (ppu2spu_unwind_register, &data);
	  discard_cleanups (cleanups);

	  cache->frame_id = frame_id_build (base, func);
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 00b87db7d145205b74859c08ca5a9d852441a4aa..1437dac220e0364557e3c568f1c5223cec598dfd 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -209,20 +209,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,
@@ -249,12 +239,6 @@ public:
    : regcache (gdbarch, aspace_, true)
  {}

-  struct readonly_t {};
-  static constexpr readonly_t readonly {};
-
-  /* Create a readonly regcache from a non-readonly regcache.  */
-  regcache (readonly_t, const regcache &src);
-
  regcache (const regcache &) = delete;
  void operator= (const regcache &) = delete;

@@ -271,8 +255,17 @@ public:
    return m_aspace;
  }

+  /* Duplicate self into a new regcache.  */
+  virtual regcache* dup ();
+
+  /* Copy the register contents from a target_regcache to self.
+     All cooked registers are read and cached.  */
  void save (regcache_cooked_read_ftype *cooked_read, void *src);

+  /* Copy register contents to a target_regcache. All cached cooked registers
+     are also restored.  */
+  void restore_to (target_regcache *dst);
+
  enum register_status cooked_read (int regnum, gdb_byte *buf);
  void cooked_write (int regnum, const gdb_byte *buf);

@@ -348,8 +341,6 @@ protected:

  gdb_byte *register_buffer (int regnum) const;

-  void restore (struct regcache *src);
-
  struct regcache_descr *m_descr;

  /* The address space of this register cache (for registers where it
@@ -363,12 +354,10 @@ protected:

  /* 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
-     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.  */
+
+  /* A read-only cache can not change it's register contents, except from
+     an target_regcache via the save () method.
+     A target_regcache cache can never be read-only.  */
  bool m_readonly_p;

private:
@@ -385,8 +374,6 @@ private:

private:

-  friend void
-  regcache_cpy (struct regcache *dst, struct regcache *src);
};


@@ -397,6 +384,16 @@ class target_regcache : public regcache
{
public:

+  target_regcache (const target_regcache &) = delete;
+  void operator= (const target_regcache &) = delete;
+
+  /* Cannot be called on a target_regcache.  */
+  void save (regcache_cooked_read_ftype *cooked_read, void *src) = delete;
+  void restore_to (target_regcache *dst) = delete;
+
+  /* Duplicate self into a new regcache.  Result is not a target_regcache.  */
+  regcache* dup ();
+
  /* Overridden regcache methods.  These versions will pass the read/write
     through to the target.  */
  enum register_status raw_read (int regnum, gdb_byte *buf);
@@ -434,14 +431,6 @@ private:
  friend void 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);
-
-/* 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);

diff --git a/gdb/regcache.c b/gdb/regcache.c
index e7da5a622e861e1e6a1938f91a0dd9a39942a050..5405dba7a706910f0b6d20c77eef657e38695b34 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -226,13 +226,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 (), src.aspace (), true)
-{
-  gdb_assert (!src.m_readonly_p);
-  save (do_cooked_read, (void *) &src);
-}
-
gdbarch *
regcache::arch () const
{
@@ -312,23 +305,11 @@ regcache::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)
+regcache::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);
  /* Clear the dest.  */
  memset (m_registers, 0, m_descr->sizeof_cooked_registers);
  memset (m_register_status, 0, m_descr->sizeof_cooked_register_status);
@@ -354,15 +335,14 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,
}

void
-regcache::restore (struct regcache *src)
+regcache::restore_to (target_regcache *dst)
{
  struct gdbarch *gdbarch = m_descr->gdbarch;
  int regnum;
+  gdb_assert (dst != NULL);
+  gdb_assert (dst->m_descr->gdbarch == m_descr->gdbarch);
+  gdb_assert (dst != this);

-  /* 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);
  /* 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
@@ -371,27 +351,33 @@ regcache::restore (struct regcache *src)
    {
      if (gdbarch_register_reggroup_p (gdbarch, regnum, restore_reggroup))
	{
-	  if (src->m_register_status[regnum] == REG_VALID)
-	    cooked_write (regnum, src->register_buffer (regnum));
+	  if (m_register_status[regnum] == REG_VALID)
+	    dst->cooked_write (regnum, register_buffer (regnum));
	}
    }
}

-void
-regcache_cpy (struct regcache *dst, struct regcache *src)
+/* Duplicate detached regcache to a detached regcache.  */
+regcache*
+regcache::dup ()
{
-  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);
+  regcache *new_regcache = new regcache (arch (), aspace ());
+
+  memcpy (new_regcache->m_registers, m_registers,
+	  m_descr->sizeof_cooked_registers);
+  memcpy (new_regcache->m_register_status, m_register_status,
+	  m_descr->sizeof_cooked_register_status);

-  dst->restore (src);
+  return new_regcache;
}

-struct regcache *
-regcache_dup (struct regcache *src)
+/* Duplicate a target_regcache to a detached regcache.  */
+regcache*
+target_regcache::dup ()
{
-  return new regcache (regcache::readonly, *src);
+  regcache *new_regcache = new regcache (arch (), aspace ());
+  new_regcache->save (do_cooked_read, (void *) this);
+  return new_regcache;
}

enum register_status
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 1ac763da7ede9d4b9f4bff1e8d71551b0c776fa1..59ed3851a62df324a61ca3c604f22548a1630f7d 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1275,7 +1275,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 = regcache->dup ();
	  *this_prologue_cache = cache;
	  return 1;
	}