Patchwork [03/10] Remove regcache_save and regcache_cpy

login
register
mail settings
Submitter Yao Qi
Date Feb. 7, 2018, 10:32 a.m.
Message ID <1517999572-14987-4-git-send-email-yao.qi@linaro.org>
Download mbox | patch
Permalink /patch/25844/
State New
Headers show

Comments

Yao Qi - Feb. 7, 2018, 10:32 a.m.
... 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(-)
Simon Marchi - Feb. 17, 2018, 2:07 a.m.
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

Patch

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);