[7/7] : Regcache: Refactor raw_set_cached_value

Message ID 280BC89C-5892-4097-9FEC-F5003D1BBE3C@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Aug. 17, 2017, 8:49 a.m. UTC
  I was going to remove raw_set_cached_value. However, I think it can be
used to make the code clearer.

I'm renaming it to _reg instead of _value, because it doesn't use value
structs. I'm making it a internal method, and adding a _get version.

In regcache, wherever the code directly reads or writes to the internal
cache, this is replaced with a call to the new functions. This makes it
obvious where regcache is writing to the cache. On the downside, this
adds extra function calls.

The alternative to this patch would be to remove raw_set_cached_value,
and keep the rest of the code unchanged.

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>

	* gdbarch-selftests.c (regcache_test::raw_write): Call super
	method.
	* regcache.c (regcache::raw_read): Use raw_get_cached_reg.
	(target_regcache::raw_read): Move code to raw_get_cached_reg.
	(regcache::raw_set_cached_reg): Rename function.
	(regcache::raw_get_cached_reg): New function.
	(regcache::raw_write): Use raw_set_cached_reg.
	(target_regcache::raw_write): Use raw_set_cached_reg.
	(regcache::raw_supply): Likewise
	(regcache::raw_collect): Use raw_get_cached_reg.
	* regcache.h: (regcache_raw_set_cached_value): Remove.
  

Patch

diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index a6251d2a9f87ccff310f90eb5c27bc4199844f22..dbb0f810f951a87a56115d7fb1c392fc7eb0f9c1 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -40,7 +40,8 @@  public:

  void raw_write (int regnum, const gdb_byte *buf) override
  {
-    raw_set_cached_value (regnum, buf);
+    /* Bypass writing to the target.  */
+    regcache::raw_write (regnum, buf);
  }
};

diff --git a/gdb/regcache.h b/gdb/regcache.h
index f4408a562f84bb2d0dcd2792c646c0f98deee716..f45ab71d92018087e3fe94e3a7a69a87ecaf2e70 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -79,14 +79,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 regcache_xmalloc.  */
-
-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.  */
@@ -301,8 +293,6 @@  public:

  virtual enum register_status get_register_status (int regnum) const;

-  void raw_set_cached_value (int regnum, const gdb_byte *buf);
-
  void invalidate (int regnum);

  enum register_status raw_read_part (int regnum, int offset, int len,
@@ -339,6 +329,10 @@  protected:

  gdb_byte *register_buffer (int regnum) const;

+  /* Get/Set the cached contents of the regcache.  */
+  void raw_set_cached_reg (int regnum, const gdb_byte *buf);
+  enum register_status raw_get_cached_reg (int regnum, gdb_byte *buf) const;
+
  struct regcache_descr *m_descr;

  /* The address space of this register cache (for registers where it
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 9d04c7be904cfb17ac8fd871e9d7678df1694dfd..1fcb4d21332385ba094c23963a3b9db078a7ae3d 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -604,8 +604,7 @@  regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf)
enum register_status
regcache::raw_read (int regnum, gdb_byte *buf)
{
-  raw_collect (regnum, buf);
-  return (enum register_status) m_register_status[regnum];
+  return raw_get_cached_reg (regnum, buf);
}

enum register_status
@@ -616,13 +615,7 @@  target_regcache::raw_read (int regnum, gdb_byte *buf)
  /* Read register value from the target into the regcache.  */
  raw_update (regnum);

-  if (m_register_status[regnum] != REG_VALID)
-    memset (buf, 0, m_descr->sizeof_register[regnum]);
-  else
-    memcpy (buf, register_buffer (regnum),
-	    m_descr->sizeof_register[regnum]);
-
-  return (enum register_status) m_register_status[regnum];
+  return raw_get_cached_reg (regnum, buf);
}

enum register_status
@@ -852,21 +845,42 @@  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_reg (int regnum, const gdb_byte *buf)
{
-  regcache->raw_set_cached_value (regnum, buf);
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (!m_readonly_p);
+
+  if (buf)
+    {
+      memcpy (register_buffer (regnum), buf, m_descr->sizeof_register[regnum]);
+      m_register_status[regnum] = REG_VALID;
+    }
+  else
+    {
+      /* This memset not strictly necessary, but better than garbage
+	 in case the register value manages to escape somewhere (due
+	 to a bug, no less).  */
+      memset (register_buffer (regnum), 0, m_descr->sizeof_register[regnum]);
+      m_register_status[regnum] = REG_UNAVAILABLE;
+    }
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
}

-void
-regcache::raw_set_cached_value (int regnum, const gdb_byte *buf)
+enum register_status
+regcache::raw_get_cached_reg (int regnum, gdb_byte *buf) const
{
-  memcpy (register_buffer (regnum), buf,
-	  m_descr->sizeof_register[regnum]);
-  m_register_status[regnum] = REG_VALID;
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (buf != NULL);
+
+  if (m_register_status[regnum] != REG_VALID)
+    memset (buf, 0, m_descr->sizeof_register[regnum]);
+  else
+    memcpy (buf, register_buffer (regnum),
+	    m_descr->sizeof_register[regnum]);
+
+  return (enum register_status) m_register_status[regnum];
}

void
@@ -880,16 +894,14 @@  regcache_raw_write (struct regcache *regcache, int regnum,
void
regcache::raw_write (int regnum, const gdb_byte *buf)
{
-  gdb_assert (buf != NULL);
  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
-  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.  */
  if (gdbarch_cannot_store_register (arch (), regnum))
    return;

-  raw_set_cached_value (regnum, buf);
+  raw_set_cached_reg (regnum, buf);
}

void
@@ -913,7 +925,7 @@  target_regcache::raw_write (int regnum, const gdb_byte *buf)
    return;

  target_prepare_to_store (this);
-  raw_set_cached_value (regnum, buf);
+  raw_set_cached_reg (regnum, buf);

  /* Register a cleanup function for invalidating the register after it is
     written, in case of a failure.  */
@@ -1072,28 +1084,7 @@  regcache_raw_supply (struct regcache *regcache, int regnum, const void *buf)
void
regcache::raw_supply (int regnum, const void *buf)
{
-  void *regbuf;
-  size_t size;
-
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
-  gdb_assert (!m_readonly_p);
-
-  regbuf = register_buffer (regnum);
-  size = m_descr->sizeof_register[regnum];
-
-  if (buf)
-    {
-      memcpy (regbuf, buf, size);
-      m_register_status[regnum] = REG_VALID;
-    }
-  else
-    {
-      /* This memset not strictly necessary, but better than garbage
-	 in case the register value manages to escape somewhere (due
-	 to a bug, no less).  */
-      memset (regbuf, 0, size);
-      m_register_status[regnum] = REG_UNAVAILABLE;
-    }
+  raw_set_cached_reg (regnum, (const gdb_byte*) buf);
}

/* Supply register REGNUM to REGCACHE.  Value to supply is an integer stored at
@@ -1153,15 +1144,7 @@  regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf)
void
regcache::raw_collect (int regnum, void *buf) const
{
-  const void *regbuf;
-  size_t size;
-
-  gdb_assert (buf != NULL);
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
-
-  regbuf = register_buffer (regnum);
-  size = m_descr->sizeof_register[regnum];
-  memcpy (buf, regbuf, size);
+  raw_get_cached_reg (regnum, (gdb_byte*) buf);
}

/* Transfer a single or all registers belonging to a certain register