[2/7] Regcache: Only target_regcache reads/writes go to the target

Message ID 9C3CF11A-EBAE-432D-ABB6-37E88A10CBB4@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Aug. 17, 2017, 8:47 a.m. UTC
  This patch ensures that only a target_regcache can call down to
the target. This is done by virtualising raw_read, raw_write and
raw_update.

In a target_regcache, these functions continue to work as they have
always done. A target_regcache cannot be readonly, so does not need
to check this.

In a regcache, these functions now do not call down to the target.
Effectively, raw_read becomes raw_collect, raw_write becomes
raw_set_cached_value and raw_update becomes a no op.


There are no other functions in regcache that call down to the target.
Functions such as raw_read_unsigned or cooked_write all use the above
three functions.


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>

	* regcache.h: (regcache::raw_update): No op function.
	* regcache.c (target_regcache::raw_update): Retain existing
	functionality.
	(regcache::raw_read): Do not read from target.
	(target_regcache::raw_read):  Retain existing functionality.
	(regcache::raw_write): Do not write to target.
	(target_regcache::raw_write):  Retain existing functionality.
  

Patch

diff --git a/gdb/regcache.h b/gdb/regcache.h
index bc3f24cffe1413d521998ab3c8081833a2735754..aeac54e8d7b91e2bcf6a3e1ce8e781c3f994306b 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -242,7 +242,8 @@  typedef struct cached_reg
} cached_reg_t;


-/* A register cache.  This is not connected to a target and ptid is unset.  */
+/* A register cache.  This is not connected to a target - reads and writes will
+   not be passed through to a target and ptid is unset.  */

class regcache
{
@@ -283,14 +284,8 @@  public:
  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);
-
-  /* class regcache is only extended in unit test, so only mark it
-     virtual when selftest is enabled.  */
-#if GDB_SELF_TEST
-  virtual
-#endif
-  void raw_write (int regnum, const gdb_byte *buf);
+  virtual enum register_status raw_read (int regnum, gdb_byte *buf);
+  virtual void raw_write (int regnum, const gdb_byte *buf);

  template<typename T, typename = RequireLongest<T>>
  enum register_status raw_read (int regnum, T *val);
@@ -306,7 +301,7 @@  public:
  template<typename T, typename = RequireLongest<T>>
  void cooked_write (int regnum, T val);

-  void raw_update (int regnum);
+  virtual void raw_update (int regnum) {}

  void raw_collect (int regnum, void *buf) const;

@@ -403,12 +398,19 @@  private:
};


-/* A register cache that can be attached to a target. ptid can be set.  */
+/* A register cache attached to a target.  Reads and writes of register values
+   will be passed through to the target and ptid can be set.  */

class target_regcache : public regcache
{
public:

+  /* Overridden regcache methods.  These versions will pass the read/write
+     through to the target.  */
+  enum register_status raw_read (int regnum, gdb_byte *buf);
+  virtual void raw_write (int regnum, const gdb_byte *buf);
+  void raw_update (int regnum);
+
  ptid_t ptid () const
  {
    return m_ptid;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 3d6ca86006fa59463069808f6e5b355028c4d3cc..524ac5754f6d2c1b00097b15a40b928845e45a59 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -597,16 +597,14 @@  regcache_raw_update (struct regcache *regcache, int regnum)
}

void
-regcache::raw_update (int regnum)
+target_regcache::raw_update (int regnum)
{
  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);

  /* Make certain that the register cache is up-to-date with respect
-     to the current thread.  This switching shouldn't be necessary
-     only there is still only one target side register cache.  Sigh!
-     On the bright side, at least there is a regcache object.  */
+     to the current thread.  */

-  if (!m_readonly_p && get_register_status (regnum) == REG_UNKNOWN)
+  if (get_register_status (regnum) == REG_UNKNOWN)
    {
      target_fetch_registers (this, regnum);

@@ -627,7 +625,16 @@  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];
+}
+
+enum register_status
+target_regcache::raw_read (int regnum, gdb_byte *buf)
+{
  gdb_assert (buf != NULL);
+
+  /* Read register value from the target into the regcache.  */
  raw_update (regnum);

  if (m_register_status[regnum] != REG_VALID)
@@ -895,11 +902,25 @@  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);
+}
+
+void
+target_regcache::raw_write (int regnum, const gdb_byte *buf)
+{
  struct cleanup *old_chain;

  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.  */