Patchwork [11/15] Class reg_buffer_rw

login
register
mail settings
Submitter Yao Qi
Date Dec. 1, 2017, 10:48 a.m.
Message ID <1512125286-29788-12-git-send-email-yao.qi@linaro.org>
Download mbox | patch
Permalink /patch/24661/
State New
Headers show

Comments

Yao Qi - Dec. 1, 2017, 10:48 a.m.
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 reg_buffer_rw, which is 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 reg_buffer_rw, and reg_buffer_rw inherits regcache_read.
The ideal design is that both reg_buffer_rw and regcache_read inherit
reg_buffer, and regcache inherit reg_buffer_rw 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(-)

Patch

diff --git a/gdb/jit.c b/gdb/jit.c
index 36aaefc..602aec9 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;
+  reg_buffer_rw *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 reg_buffer_rw (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 ec6fc69..65a2bfb 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 reg_buffer_rw *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 reg_buffer_rw (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 ()));
@@ -871,7 +871,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;
     }
 
@@ -2032,12 +2032,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.  */
@@ -2056,8 +2054,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 4174a8c..9b5b02e 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).  */
-  : regcache_read (gdbarch, readonly_p_),
+  : reg_buffer_rw (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)
+reg_buffer_rw::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 bd91bc6..3de5d80 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 reg_buffer_rw;
 };
 
 class regcache_read : public reg_buffer
@@ -289,11 +283,33 @@  protected:
 				  bool is_raw);
 };
 
+/* Buffer of registers, can be red and written.  */
+
+class reg_buffer_rw : public regcache_read
+{
+public:
+  reg_buffer_rw (gdbarch *gdbarch, bool has_pseudo)
+    : regcache_read (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 (reg_buffer_rw);
+};
+
 class regcache_readonly;
 
 /* The register cache for storing raw register values.  */
 
-class regcache : public regcache_read
+class regcache : public reg_buffer_rw
 {
 public:
   regcache (gdbarch *gdbarch)
@@ -337,17 +353,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);