public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/8] Remove regcache_descr fields sizeof_raw_register_status and sizeof_cooked_register_status
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
  2017-10-27  9:31 ` [PATCH 4/8] Remove regcache_descr::nr_raw_registers Yao Qi
@ 2017-10-27  9:31 ` Yao Qi
  2017-10-27  9:31 ` [PATCH 3/8] New method regcache::assert_regnum Yao Qi
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:31 UTC (permalink / raw)
  To: gdb-patches

struct regcache_descr has two fields sizeof_raw_register_status
and sizeof_cooked_register_status, but they equal to nr_cooked_registers
and nr_raw_registers respectively, so this patch removes them.

gdb:

2017-10-18  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (struct regcache_descr) <sizeof_raw_register_status>:
	Remove.
	<sizeof_cooked_register_status>: Remove.
	(init_regcache_descr): Update.
	(regcache::regcache): Use nr_cooked_registers and nr_raw_registers.
	(regcache::save): Likewise.
	(regcache::dump): Likewise.
---
 gdb/regcache.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 1ba49ad..5a1152e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -53,7 +53,6 @@ struct regcache_descr
      cache.  */
   int nr_raw_registers;
   long sizeof_raw_registers;
-  long sizeof_raw_register_status;
 
   /* The cooked register space.  Each cooked register in the range
      [0..NR_RAW_REGISTERS) is direct-mapped onto the corresponding raw
@@ -63,7 +62,6 @@ struct regcache_descr
      gdbarch_pseudo_register_read and gdbarch_pseudo_register_write.  */
   int nr_cooked_registers;
   long sizeof_cooked_registers;
-  long sizeof_cooked_register_status;
 
   /* Offset and size (in 8 bit bytes), of each register in the
      register cache.  All registers (including those in the range
@@ -92,8 +90,6 @@ init_regcache_descr (struct gdbarch *gdbarch)
      either mapped onto raw-registers or memory.  */
   descr->nr_cooked_registers = gdbarch_num_regs (gdbarch)
 			       + gdbarch_num_pseudo_regs (gdbarch);
-  descr->sizeof_cooked_register_status
-    = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch);
 
   /* Fill in a table of register types.  */
   descr->register_type
@@ -105,7 +101,6 @@ init_regcache_descr (struct gdbarch *gdbarch)
   /* Construct a strictly RAW register cache.  Don't allow pseudo's
      into the register cache.  */
   descr->nr_raw_registers = gdbarch_num_regs (gdbarch);
-  descr->sizeof_raw_register_status = gdbarch_num_regs (gdbarch);
 
   /* Lay out the register cache.
 
@@ -199,13 +194,13 @@ regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
     {
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_cooked_registers);
       m_register_status = XCNEWVEC (signed char,
-				    m_descr->sizeof_cooked_register_status);
+				    m_descr->nr_cooked_registers);
     }
   else
     {
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_raw_registers);
       m_register_status = XCNEWVEC (signed char,
-				    m_descr->sizeof_raw_register_status);
+				    m_descr->nr_raw_registers);
     }
   m_ptid = minus_one_ptid;
 }
@@ -306,7 +301,7 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,
   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);
+  memset (m_register_status, 0, m_descr->nr_cooked_registers);
   /* Copy over any registers (identified by their membership in the
      save_reggroup) and mark them as valid.  The full [0 .. gdbarch_num_regs +
      gdbarch_num_pseudo_regs) range is checked since some architectures need
@@ -1343,7 +1338,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
   fprintf_unfiltered (file, "sizeof_raw_registers %ld\n",
 		      m_descr->sizeof_raw_registers);
   fprintf_unfiltered (file, "sizeof_raw_register_status %ld\n",
-		      m_descr->sizeof_raw_register_status);
+		      m_descr->nr_raw_registers);
   fprintf_unfiltered (file, "gdbarch_num_regs %d\n", 
 		      gdbarch_num_regs (gdbarch));
   fprintf_unfiltered (file, "gdbarch_num_pseudo_regs %d\n",
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/8] Remove regcache_descr::nr_raw_registers
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
@ 2017-10-27  9:31 ` Yao Qi
  2017-10-31 14:27   ` Simon Marchi
  2017-10-27  9:31 ` [PATCH 1/8] Remove regcache_descr fields sizeof_raw_register_status and sizeof_cooked_register_status Yao Qi
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:31 UTC (permalink / raw)
  To: gdb-patches

struct regcache_descr has fields nr_raw_registers and gdbarch, and
nr_raw_registers can be got via gdbarch_num_regs (gdbarch), so it looks
nr_raw_registers is redundant.  This patch removes it.

gdb:

2017-10-19  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (struct regcache_descr) <nr_raw_registers>: Remove.
	(init_regcache_descr): Use gdbarch_num_regs.
	(regcache::regcache): Likewise.
	(regcache::get_register_status): Likewise.
	(regcache::assert_raw_regnum): Likewise.
	(regcache::cooked_read): Likewise.
	(regcache::cooked_read_value): Likewise.
	(regcache::cooked_write): Likewise.
	(regcache::dump): Likewise.
---
 gdb/regcache.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 0aee934..fa1ed48 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -51,7 +51,6 @@ struct regcache_descr
      redundant information - if the PC is constructed from two
      registers then those registers and not the PC lives in the raw
      cache.  */
-  int nr_raw_registers;
   long sizeof_raw_registers;
 
   /* The cooked register space.  Each cooked register in the range
@@ -100,7 +99,6 @@ init_regcache_descr (struct gdbarch *gdbarch)
 
   /* Construct a strictly RAW register cache.  Don't allow pseudo's
      into the register cache.  */
-  descr->nr_raw_registers = gdbarch_num_regs (gdbarch);
 
   /* Lay out the register cache.
 
@@ -116,7 +114,7 @@ init_regcache_descr (struct gdbarch *gdbarch)
       = GDBARCH_OBSTACK_CALLOC (gdbarch, descr->nr_cooked_registers, long);
     descr->register_offset
       = GDBARCH_OBSTACK_CALLOC (gdbarch, descr->nr_cooked_registers, long);
-    for (i = 0; i < descr->nr_raw_registers; i++)
+    for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
       {
 	descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]);
 	descr->register_offset[i] = offset;
@@ -199,8 +197,7 @@ regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
   else
     {
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_raw_registers);
-      m_register_status = XCNEWVEC (signed char,
-				    m_descr->nr_raw_registers);
+      m_register_status = XCNEWVEC (signed char, gdbarch_num_regs (gdbarch));
     }
   m_ptid = minus_one_ptid;
 }
@@ -378,7 +375,7 @@ regcache::get_register_status (int regnum) const
   if (m_readonly_p)
     gdb_assert (regnum < m_descr->nr_cooked_registers);
   else
-    gdb_assert (regnum < m_descr->nr_raw_registers);
+    gdb_assert (regnum < gdbarch_num_regs (arch ()));
 
   return (enum register_status) m_register_status[regnum];
 }
@@ -401,7 +398,7 @@ regcache::invalidate (int regnum)
 void
 regcache::assert_regnum (int regnum) const
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch ()));
 }
 
 /* Global structure containing the current regcache.  */
@@ -677,7 +674,7 @@ regcache::cooked_read (int regnum, gdb_byte *buf)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
-  if (regnum < m_descr->nr_raw_registers)
+  if (regnum < gdbarch_num_regs (arch ()))
     return raw_read (regnum, buf);
   else if (m_readonly_p
 	   && m_register_status[regnum] != REG_UNKNOWN)
@@ -731,7 +728,7 @@ regcache::cooked_read_value (int regnum)
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
 
-  if (regnum < m_descr->nr_raw_registers
+  if (regnum < gdbarch_num_regs (arch ())
       || (m_readonly_p && m_register_status[regnum] != REG_UNKNOWN)
       || !gdbarch_pseudo_register_read_value_p (m_descr->gdbarch))
     {
@@ -890,7 +887,7 @@ regcache::cooked_write (int regnum, const gdb_byte *buf)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
-  if (regnum < m_descr->nr_raw_registers)
+  if (regnum < gdbarch_num_regs (arch ()))
     raw_write (regnum, buf);
   else
     gdbarch_pseudo_register_write (m_descr->gdbarch, this,
@@ -1435,7 +1432,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	{
 	  if (regnum < 0)
 	    fprintf_unfiltered (file, "Raw value");
-	  else if (regnum >= m_descr->nr_raw_registers)
+	  else if (regnum >= gdbarch_num_regs (arch ()))
 	    fprintf_unfiltered (file, "<cooked>");
 	  else if (get_register_status (regnum) == REG_UNKNOWN)
 	    fprintf_unfiltered (file, "<invalid>");
@@ -1461,7 +1458,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	      enum register_status status;
 	      struct value *value = NULL;
 
-	      if (regnum < m_descr->nr_raw_registers)
+	      if (regnum < gdbarch_num_regs (arch ()))
 		{
 		  raw_update (regnum);
 		  status = get_register_status (regnum);
@@ -1529,7 +1526,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	    {
 	      fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
 	    }
-	  else if (regnum < m_descr->nr_raw_registers)
+	  else if (regnum < gdbarch_num_regs (arch ()))
 	    {
 	      int pnum, poffset;
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/8] regcache misc cleanup and refactor
@ 2017-10-27  9:31 Yao Qi
  2017-10-27  9:31 ` [PATCH 4/8] Remove regcache_descr::nr_raw_registers Yao Qi
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:31 UTC (permalink / raw)
  To: gdb-patches

During the work of regcache rewriting, I find there are some minor
cleanups and refactors can go upstream first.  Each patch is quite
self-contained, but they may be dependent to previous patches in
this series, so I put all of them in one patch set.

Rebuild GDB with every patch applied accumulatively, and regress
tested on x86_64-linux.

*** BLURB HERE ***

Yao Qi (8):
  Remove regcache_descr fields sizeof_raw_register_status and
    sizeof_cooked_register_status
  Remove code wrapped by "#if 0"
  New method regcache::assert_regnum
  Remove regcache_descr::nr_raw_registers
  s/get_regcache_aspace (regcache)/regcache->aspace ()/g
  const-fy regcache::m_aspace
  const-fy regcache::m_readonly_p
  Construct readonly regcache without address space

 gdb/darwin-nat.c     |  2 +-
 gdb/frame.c          |  5 ++-
 gdb/infrun.c         | 42 ++++++++++++-------------
 gdb/jit.c            |  4 +--
 gdb/linux-nat.c      |  8 ++---
 gdb/ppc-linux-tdep.c |  5 ++-
 gdb/record-full.c    |  7 ++---
 gdb/regcache.c       | 88 +++++++++++++++++++---------------------------------
 gdb/regcache.h       | 20 ++++++------
 9 files changed, 75 insertions(+), 106 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/8] New method regcache::assert_regnum
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
  2017-10-27  9:31 ` [PATCH 4/8] Remove regcache_descr::nr_raw_registers Yao Qi
  2017-10-27  9:31 ` [PATCH 1/8] Remove regcache_descr fields sizeof_raw_register_status and sizeof_cooked_register_status Yao Qi
@ 2017-10-27  9:31 ` Yao Qi
  2017-10-27  9:32 ` [PATCH 2/8] Remove code wrapped by "#if 0" Yao Qi
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:31 UTC (permalink / raw)
  To: gdb-patches

class regcache has some methods checking the range of register number,
this patch is to move it in a new method assert_regnum.

gdb:

2017-10-19  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (regcache::assert_regnum): New method.
	(regcache::invalidate): Call assert_regnum.
	(regcache::raw_update): Likewise.
	(regcache::raw_write): Likewise.
	(regcache::raw_read_part): Likewise.
	(regcache::raw_write_part): Likewise.
	(regcache::raw_supply): Likewise.
	(regcache::raw_supply_integer): Likewise.
	(regcache::raw_supply_zeroed): Likewise.
	(regcache::raw_collect): Likewise.
	(regcache::raw_collect_integer): Likewise.
	* regcache.h (regcache::assert_regnum): Declare.
---
 gdb/regcache.c | 31 ++++++++++++++++++-------------
 gdb/regcache.h |  3 +++
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 0d3fe3d..0aee934 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -393,12 +393,17 @@ regcache_invalidate (struct regcache *regcache, int regnum)
 void
 regcache::invalidate (int regnum)
 {
-  gdb_assert (regnum >= 0);
   gdb_assert (!m_readonly_p);
-  gdb_assert (regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   m_register_status[regnum] = REG_UNKNOWN;
 }
 
+void
+regcache::assert_regnum (int regnum) const
+{
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+}
+
 /* Global structure containing the current regcache.  */
 
 /* NOTE: this is a write-through cache.  There is no "dirty" bit for
@@ -546,7 +551,7 @@ regcache_raw_update (struct regcache *regcache, int regnum)
 void
 regcache::raw_update (int regnum)
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
 
   /* Make certain that the register cache is up-to-date with respect
      to the current thread.  This switching shouldn't be necessary
@@ -600,7 +605,7 @@ regcache::raw_read (int regnum, T *val)
   gdb_byte *buf;
   enum register_status status;
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
   status = raw_read (regnum, buf);
   if (status == REG_VALID)
@@ -633,7 +638,7 @@ regcache::raw_write (int regnum, T val)
 {
   gdb_byte *buf;
 
-  gdb_assert (regnum >=0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
   store_integer (buf, m_descr->sizeof_register[regnum],
 		 gdbarch_byte_order (m_descr->gdbarch), val);
@@ -844,7 +849,7 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
 {
 
   gdb_assert (buf != NULL);
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   gdb_assert (!m_readonly_p);
 
   /* On the sparc, writing %g0 is a no-op, so we don't even want to
@@ -953,7 +958,7 @@ regcache_raw_read_part (struct regcache *regcache, int regnum,
 enum register_status
 regcache::raw_read_part (int regnum, int offset, int len, gdb_byte *buf)
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   return xfer_part (regnum, offset, len, buf, NULL, true);
 }
 
@@ -968,7 +973,7 @@ void
 regcache::raw_write_part (int regnum, int offset, int len,
 			  const gdb_byte *buf)
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   xfer_part (regnum, offset, len, NULL, buf, true);
 }
 
@@ -1017,7 +1022,7 @@ regcache::raw_supply (int regnum, const void *buf)
   void *regbuf;
   size_t size;
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
@@ -1052,7 +1057,7 @@ regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
   gdb_byte *regbuf;
   size_t regsize;
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
@@ -1073,7 +1078,7 @@ regcache::raw_supply_zeroed (int regnum)
   void *regbuf;
   size_t size;
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
   gdb_assert (!m_readonly_p);
 
   regbuf = register_buffer (regnum);
@@ -1099,7 +1104,7 @@ regcache::raw_collect (int regnum, void *buf) const
   size_t size;
 
   gdb_assert (buf != NULL);
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
 
   regbuf = register_buffer (regnum);
   size = m_descr->sizeof_register[regnum];
@@ -1124,7 +1129,7 @@ regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
   const gdb_byte *regbuf;
   size_t regsize;
 
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  assert_regnum (regnum);
 
   regbuf = register_buffer (regnum);
   regsize = m_descr->sizeof_register[regnum];
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 0eea042..6fb790d 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -358,6 +358,9 @@ private:
 			int regnum, const void *in_buf,
 			void *out_buf, size_t size) const;
 
+  /* Assert on the range of REGNUM.  */
+  void assert_regnum (int regnum) const;
+
   struct regcache_descr *m_descr;
 
   /* The address space of this register cache (for registers where it
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/8] Remove code wrapped by "#if 0"
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
                   ` (2 preceding siblings ...)
  2017-10-27  9:31 ` [PATCH 3/8] New method regcache::assert_regnum Yao Qi
@ 2017-10-27  9:32 ` Yao Qi
  2017-10-27  9:32 ` [PATCH 6/8] const-fy regcache::m_aspace Yao Qi
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:32 UTC (permalink / raw)
  To: gdb-patches

These code wrapped by "#if 0" was added by af030b9a, which added the new
command to dump registers in 2002.  The email didn't mention this either
https://sourceware.org/ml/gdb-patches/2002-08/msg00227.html  It was there
for 15 years, and nobody needs it, so we can remove it.

gdb:

2017-10-18  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (regcache::dump): Remove code.
---
 gdb/regcache.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 5a1152e..0d3fe3d 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1330,21 +1330,6 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
   int footnote_register_type_name_null = 0;
   long register_offset = 0;
 
-#if 0
-  fprintf_unfiltered (file, "nr_raw_registers %d\n",
-		      m_descr->nr_raw_registers);
-  fprintf_unfiltered (file, "nr_cooked_registers %d\n",
-		      m_descr->nr_cooked_registers);
-  fprintf_unfiltered (file, "sizeof_raw_registers %ld\n",
-		      m_descr->sizeof_raw_registers);
-  fprintf_unfiltered (file, "sizeof_raw_register_status %ld\n",
-		      m_descr->nr_raw_registers);
-  fprintf_unfiltered (file, "gdbarch_num_regs %d\n", 
-		      gdbarch_num_regs (gdbarch));
-  fprintf_unfiltered (file, "gdbarch_num_pseudo_regs %d\n",
-		      gdbarch_num_pseudo_regs (gdbarch));
-#endif
-
   gdb_assert (m_descr->nr_cooked_registers
 	      == (gdbarch_num_regs (gdbarch)
 		  + gdbarch_num_pseudo_regs (gdbarch)));
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 8/8] Construct readonly regcache without address space
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
                   ` (4 preceding siblings ...)
  2017-10-27  9:32 ` [PATCH 6/8] const-fy regcache::m_aspace Yao Qi
@ 2017-10-27  9:32 ` Yao Qi
  2017-10-31 14:35   ` Simon Marchi
  2017-10-27  9:32 ` [PATCH 7/8] const-fy regcache::m_readonly_p Yao Qi
  2017-10-27  9:32 ` [PATCH 5/8] s/get_regcache_aspace (regcache)/regcache->aspace ()/g Yao Qi
  7 siblings, 1 reply; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:32 UTC (permalink / raw)
  To: gdb-patches

The address space is useless to readonly regcache, so this patch removes
the parameter to construct readonly regcache.

gdb:

2017-10-23  Yao Qi  <yao.qi@linaro.org>

	* frame.c (do_frame_register_read): Remove aspace.
	* jit.c (jit_frame_sniffer): Likwise.
	* ppc-linux-tdep.c (ppu2spu_sniffer): Likewise.
	* regcache.c (regcache::regcache): Pass nullptr.
	(regcache_print): Caller updated.
	* regcache.h (regcache::regcache): Remove one constructor
	parameter aspace.
---
 gdb/frame.c          | 3 +--
 gdb/jit.c            | 4 +---
 gdb/ppc-linux-tdep.c | 5 ++---
 gdb/regcache.c       | 4 ++--
 gdb/regcache.h       | 4 ++--
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index bf308ba..0f5d846 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1020,9 +1020,8 @@ do_frame_register_read (void *src, int regnum, gdb_byte *buf)
 std::unique_ptr<struct regcache>
 frame_save_as_regcache (struct frame_info *this_frame)
 {
-  struct address_space *aspace = get_frame_address_space (this_frame);
   std::unique_ptr<struct regcache> regcache
-    (new struct regcache (get_frame_arch (this_frame), aspace));
+    (new struct regcache (get_frame_arch (this_frame)));
 
   regcache_save (regcache.get (), do_frame_register_read, this_frame);
   return regcache;
diff --git a/gdb/jit.c b/gdb/jit.c
index a2d1f6d..7538684 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1186,7 +1186,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 address_space *aspace;
   struct gdbarch *gdbarch;
 
   callbacks.reg_get = jit_unwind_reg_get_impl;
@@ -1200,12 +1199,11 @@ jit_frame_sniffer (const struct frame_unwind *self,
 
   gdb_assert (!*cache);
 
-  aspace = get_frame_address_space (this_frame);
   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, aspace);
+  priv_data->regcache = new regcache (gdbarch);
   priv_data->this_frame = this_frame;
 
   callbacks.priv_data = priv_data;
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 67d8305..847908a 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1361,10 +1361,9 @@ ppu2spu_sniffer (const struct frame_unwind *self,
 	{
 	  struct ppu2spu_cache *cache
 	    = FRAME_OBSTACK_CALLOC (1, struct ppu2spu_cache);
-
-	  struct address_space *aspace = get_frame_address_space (this_frame);
 	  std::unique_ptr<struct regcache> regcache
-	    (new struct regcache (data.gdbarch, aspace));
+	    (new struct regcache (data.gdbarch));
+
 	  regcache_save (regcache.get (), ppu2spu_unwind_register, &data);
 
 	  cache->frame_id = frame_id_build (base, func);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 2cffb70..8ec099c 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -211,7 +211,7 @@ do_cooked_read (void *src, int regnum, gdb_byte *buf)
 }
 
 regcache::regcache (readonly_t, const regcache &src)
-  : regcache (src.arch (), src.aspace (), true)
+  : regcache (src.arch (), nullptr, true)
 {
   gdb_assert (!src.m_readonly_p);
   save (do_cooked_read, (void *) &src);
@@ -1568,7 +1568,7 @@ regcache_print (const char *args, enum regcache_dump_what what_to_dump)
       /* For the benefit of "maint print registers" & co when
 	 debugging an executable, allow dumping a regcache even when
 	 there is no thread selected / no registers.  */
-      regcache dummy_regs (target_gdbarch (), nullptr);
+      regcache dummy_regs (target_gdbarch ());
       dummy_regs.dump (out, what_to_dump);
     }
 }
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 98cb095..8c3cbc9 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -232,8 +232,8 @@ typedef struct cached_reg
 class regcache
 {
 public:
-  regcache (gdbarch *gdbarch, const address_space *aspace_)
-    : regcache (gdbarch, aspace_, true)
+  regcache (gdbarch *gdbarch)
+    : regcache (gdbarch, nullptr, true)
   {}
 
   struct readonly_t {};
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/8] s/get_regcache_aspace (regcache)/regcache->aspace ()/g
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
                   ` (6 preceding siblings ...)
  2017-10-27  9:32 ` [PATCH 7/8] const-fy regcache::m_readonly_p Yao Qi
@ 2017-10-27  9:32 ` Yao Qi
  7 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:32 UTC (permalink / raw)
  To: gdb-patches

and remove get_regcache_aspace.

gdb:

2017-10-22  Yao Qi  <yao.qi@linaro.org>

	* darwin-nat.c (cancel_breakpoint): Use regcache->aspace ().
	* frame.c (create_sentinel_frame): Likewise.
	* infrun.c (displaced_step_prepare_throw): Likewise.
	(resume): Likewise.
	(thread_still_needs_step_over_bp): Likewise.
	(proceed): Likewise.
	(do_target_wait): Likewise.
	(adjust_pc_after_break): Likewise.
	(handle_syscall_event): Likewise.
	(save_waitstatus): Likewise.
	(handle_inferior_event_1): Likewise.
	(handle_signal_stop): Likewise.
	(keep_going_pass_signal): Likewise.
	* linux-nat.c (status_callback): Likewise.
	(save_stop_reason): Likewise.
	(resume_stopped_resumed_lwps): Likewise.
	* record-full.c (record_full_exec_insn): Likewise.
	(record_full_wait_1): Likewise.
	* regcache.c (get_regcache_aspace): Remove.
	* regcache.h (get_regcache_aspace): Remove.
---
 gdb/darwin-nat.c  |  2 +-
 gdb/frame.c       |  2 +-
 gdb/infrun.c      | 34 +++++++++++++++++-----------------
 gdb/linux-nat.c   |  8 ++++----
 gdb/record-full.c |  6 +++---
 gdb/regcache.c    |  6 ------
 gdb/regcache.h    |  5 +----
 7 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index dc47c73..ab15913 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1195,7 +1195,7 @@ cancel_breakpoint (ptid_t ptid)
   CORE_ADDR pc;
 
   pc = regcache_read_pc (regcache) - gdbarch_decr_pc_after_break (gdbarch);
-  if (breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+  if (breakpoint_inserted_here_p (regcache->aspace (), pc))
     {
       inferior_debug (4, "cancel_breakpoint for thread 0x%lx\n",
 		      (unsigned long) ptid_get_tid (ptid));
diff --git a/gdb/frame.c b/gdb/frame.c
index 8d48169..5380c7d 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1521,7 +1521,7 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
 
   frame->level = -1;
   frame->pspace = pspace;
-  frame->aspace = get_regcache_aspace (regcache);
+  frame->aspace = regcache->aspace ();
   /* Explicitly initialize the sentinel frame's cache.  Provide it
      with the underlying regcache.  In the future additional
      information, such as the frame's thread will be added.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 858ffa1..0e31dbc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1760,7 +1760,7 @@ displaced_step_prepare_throw (ptid_t ptid)
   struct thread_info *tp = find_thread_ptid (ptid);
   struct regcache *regcache = get_thread_regcache (ptid);
   struct gdbarch *gdbarch = regcache->arch ();
-  struct address_space *aspace = get_regcache_aspace (regcache);
+  struct address_space *aspace = regcache->aspace ();
   CORE_ADDR original, copy;
   ULONGEST len;
   struct displaced_step_closure *closure;
@@ -2388,7 +2388,7 @@ resume (enum gdb_signal sig)
   struct gdbarch *gdbarch = regcache->arch ();
   struct thread_info *tp = inferior_thread ();
   CORE_ADDR pc = regcache_read_pc (regcache);
-  struct address_space *aspace = get_regcache_aspace (regcache);
+  struct address_space *aspace = regcache->aspace ();
   ptid_t resume_ptid;
   /* This represents the user's step vs continue request.  When
      deciding whether "set scheduler-locking step" applies, it's the
@@ -2591,7 +2591,7 @@ resume (enum gdb_signal sig)
 	  if (target_is_non_stop_p ())
 	    stop_all_threads ();
 
-	  set_step_over_info (get_regcache_aspace (regcache),
+	  set_step_over_info (regcache->aspace (),
 			      regcache_read_pc (regcache), 0, tp->global_num);
 
 	  step = maybe_software_singlestep (gdbarch, pc);
@@ -2920,7 +2920,7 @@ thread_still_needs_step_over_bp (struct thread_info *tp)
     {
       struct regcache *regcache = get_thread_regcache (tp->ptid);
 
-      if (breakpoint_here_p (get_regcache_aspace (regcache),
+      if (breakpoint_here_p (regcache->aspace (),
 			     regcache_read_pc (regcache))
 	  == ordinary_breakpoint_here)
 	return 1;
@@ -3007,7 +3007,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   regcache = get_current_regcache ();
   gdbarch = regcache->arch ();
-  aspace = get_regcache_aspace (regcache);
+  aspace = regcache->aspace ();
   pc = regcache_read_pc (regcache);
   tp = inferior_thread ();
 
@@ -3535,7 +3535,7 @@ do_target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
 				paddress (gdbarch, pc));
 	  discard = 1;
 	}
-      else if (!breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+      else if (!breakpoint_inserted_here_p (regcache->aspace (), pc))
 	{
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -4150,7 +4150,7 @@ adjust_pc_after_break (struct thread_info *thread,
   if (decr_pc == 0)
     return;
 
-  aspace = get_regcache_aspace (regcache);
+  aspace = regcache->aspace ();
 
   /* Find the location where (if we've hit a breakpoint) the
      breakpoint would be.  */
@@ -4266,7 +4266,7 @@ handle_syscall_event (struct execution_control_state *ecs)
                             syscall_number);
 
       ecs->event_thread->control.stop_bpstat
-	= bpstat_stop_status (get_regcache_aspace (regcache),
+	= bpstat_stop_status (regcache->aspace (),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
       if (handle_stop_requested (ecs))
@@ -4404,7 +4404,7 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
   tp->suspend.waitstatus_pending_p = 1;
 
   regcache = get_thread_regcache (tp->ptid);
-  aspace = get_regcache_aspace (regcache);
+  aspace = regcache->aspace ();
 
   if (ws->kind == TARGET_WAITKIND_STOPPED
       && ws->value.sig == GDB_SIGNAL_TRAP)
@@ -4888,7 +4888,7 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
     {
       struct regcache *regcache = get_thread_regcache (ecs->ptid);
 
-      if (breakpoint_inserted_here_p (get_regcache_aspace (regcache),
+      if (breakpoint_inserted_here_p (regcache->aspace (),
 				      regcache_read_pc (regcache)))
 	{
 	  if (debug_infrun)
@@ -4958,7 +4958,7 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
 	  handle_solib_event ();
 
 	  ecs->event_thread->control.stop_bpstat
-	    = bpstat_stop_status (get_regcache_aspace (regcache),
+	    = bpstat_stop_status (regcache->aspace (),
 				  stop_pc, ecs->ptid, &ecs->ws);
 
 	  if (handle_stop_requested (ecs))
@@ -5212,7 +5212,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
 
       ecs->event_thread->control.stop_bpstat
-	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
+	= bpstat_stop_status (get_current_regcache ()->aspace (),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
       if (handle_stop_requested (ecs))
@@ -5327,7 +5327,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
       ecs->event_thread = inferior_thread ();
 
       ecs->event_thread->control.stop_bpstat
-	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
+	= bpstat_stop_status (get_current_regcache ()->aspace (),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
       /* Note that this may be referenced from inside
@@ -5780,7 +5780,7 @@ handle_signal_stop (struct execution_control_state *ecs)
       CORE_ADDR pc;
 
       regcache = get_thread_regcache (ecs->ptid);
-      aspace = get_regcache_aspace (regcache);
+      aspace = regcache->aspace ();
       pc = regcache_read_pc (regcache);
 
       /* However, before doing so, if this single-step breakpoint was
@@ -5872,7 +5872,7 @@ handle_signal_stop (struct execution_control_state *ecs)
   if (ecs->event_thread->control.step_range_end != 1)
     {
       struct address_space *aspace = 
-	get_regcache_aspace (get_thread_regcache (ecs->ptid));
+	get_thread_regcache (ecs->ptid)->aspace ();
 
       /* skip_inline_frames is expensive, so we avoid it if we can
 	 determine that the address is one where functions cannot have
@@ -5944,7 +5944,7 @@ handle_signal_stop (struct execution_control_state *ecs)
   /* See if there is a breakpoint/watchpoint/catchpoint/etc. that
      handles this event.  */
   ecs->event_thread->control.stop_bpstat
-    = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
+    = bpstat_stop_status (get_current_regcache ()->aspace (),
 			  stop_pc, ecs->ptid, &ecs->ws);
 
   /* Following in case break condition called a
@@ -7758,7 +7758,7 @@ keep_going_pass_signal (struct execution_control_state *ecs)
       if (remove_bp
 	  && (remove_wps || !use_displaced_stepping (ecs->event_thread)))
 	{
-	  set_step_over_info (get_regcache_aspace (regcache),
+	  set_step_over_info (regcache->aspace (),
 			      regcache_read_pc (regcache), remove_wps,
 			      ecs->event_thread->global_num);
 	}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 5d8f9f3..a6395f4 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2642,7 +2642,7 @@ status_callback (struct lwp_info *lp, void *data)
 	}
 
 #if !USE_SIGTRAP_SIGINFO
-      else if (!breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+      else if (!breakpoint_inserted_here_p (regcache->aspace (), pc))
 	{
 	  if (debug_linux_nat)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -2802,7 +2802,7 @@ save_stop_reason (struct lwp_info *lp)
     }
 #else
   if ((!lp->step || lp->stop_pc == sw_bp_pc)
-      && software_breakpoint_inserted_here_p (get_regcache_aspace (regcache),
+      && software_breakpoint_inserted_here_p (regcache->aspace (),
 					      sw_bp_pc))
     {
       /* The LWP was either continued, or stepped a software
@@ -2810,7 +2810,7 @@ save_stop_reason (struct lwp_info *lp)
       lp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
     }
 
-  if (hardware_breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+  if (hardware_breakpoint_inserted_here_p (regcache->aspace (), pc))
     lp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
 
   if (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON)
@@ -3574,7 +3574,7 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, void *data)
 	     immediately, and we're not waiting for this LWP.  */
 	  if (!ptid_match (lp->ptid, *wait_ptid_p))
 	    {
-	      if (breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+	      if (breakpoint_inserted_here_p (regcache->aspace (), pc))
 		leave_stopped = 1;
 	    }
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 8c5e36b..0488b71 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -743,7 +743,7 @@ record_full_exec_insn (struct regcache *regcache,
 		       not doing the change at all if the watchpoint
 		       traps.  */
 		    if (hardware_watchpoint_inserted_in_range
-			(get_regcache_aspace (regcache),
+			(regcache->aspace (),
 			 entry->u.mem.addr, entry->u.mem.len))
 		      record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
 		  }
@@ -1109,7 +1109,7 @@ record_full_wait_1 (struct target_ops *ops,
 		  registers_changed ();
 		  regcache = get_current_regcache ();
 		  tmp_pc = regcache_read_pc (regcache);
-		  aspace = get_regcache_aspace (regcache);
+		  aspace = regcache->aspace ();
 
 		  if (target_stopped_by_watchpoint ())
 		    {
@@ -1172,7 +1172,7 @@ record_full_wait_1 (struct target_ops *ops,
     {
       struct regcache *regcache = get_current_regcache ();
       struct gdbarch *gdbarch = regcache->arch ();
-      struct address_space *aspace = get_regcache_aspace (regcache);
+      struct address_space *aspace = regcache->aspace ();
       int continue_flag = 1;
       int first_record_full_end = 1;
       struct cleanup *old_cleanups
diff --git a/gdb/regcache.c b/gdb/regcache.c
index fa1ed48..6985dc9 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -264,12 +264,6 @@ private:
   int m_regnum;
 };
 
-struct address_space *
-get_regcache_aspace (const struct regcache *regcache)
-{
-  return regcache->aspace ();
-}
-
 /* Return  a pointer to register REGNUM's buffer cache.  */
 
 gdb_byte *
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 6fb790d..2360e27 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -39,10 +39,6 @@ extern struct regcache *get_thread_arch_aspace_regcache (ptid_t,
 
 extern ptid_t regcache_get_ptid (const struct regcache *regcache);
 
-/* Return REGCACHE's address space.  */
-
-extern struct address_space *get_regcache_aspace (const struct regcache *);
-
 enum register_status regcache_register_status (const struct regcache *regcache,
 					       int regnum);
 
@@ -257,6 +253,7 @@ public:
   /* Return regcache's architecture.  */
   gdbarch *arch () const;
 
+  /* Return REGCACHE's address space.  */
   address_space *aspace () const
   {
     return m_aspace;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 7/8] const-fy regcache::m_readonly_p
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
                   ` (5 preceding siblings ...)
  2017-10-27  9:32 ` [PATCH 8/8] Construct readonly regcache without address space Yao Qi
@ 2017-10-27  9:32 ` Yao Qi
  2017-10-27  9:32 ` [PATCH 5/8] s/get_regcache_aspace (regcache)/regcache->aspace ()/g Yao Qi
  7 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:32 UTC (permalink / raw)
  To: gdb-patches

gdb:

2017-10-23  Yao Qi  <yao.qi@linaro.org>

	* regcache.h (regcache) <m_readonly_p>: Change it to const bool.
---
 gdb/regcache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/regcache.h b/gdb/regcache.h
index be87af5..98cb095 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -376,7 +376,7 @@ private:
      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.  */
-  bool m_readonly_p;
+  const bool m_readonly_p;
   /* If this is a read-write cache, which thread's registers is
      it connected to?  */
   ptid_t m_ptid;
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6/8] const-fy regcache::m_aspace
  2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
                   ` (3 preceding siblings ...)
  2017-10-27  9:32 ` [PATCH 2/8] Remove code wrapped by "#if 0" Yao Qi
@ 2017-10-27  9:32 ` Yao Qi
  2017-10-31 14:19   ` Simon Marchi
  2017-10-27  9:32 ` [PATCH 8/8] Construct readonly regcache without address space Yao Qi
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Yao Qi @ 2017-10-27  9:32 UTC (permalink / raw)
  To: gdb-patches

regcache::m_aspace is a const, never changed during the lifetime of
regcache object.

gdb:

2017-10-23  Yao Qi  <yao.qi@linaro.org>

	* frame.c (create_sentinel_frame): Cast.
	* infrun.c (displaced_step_prepare_throw): Change variable const.
	(resume): Likewise.
	(proceed): Likewise.
	(adjust_pc_after_break): Likewise.
	(save_waitstatus): Likewise.
	(handle_signal_stop): Likewise.
	(keep_going_pass_signal): Likewise.
	* record-full.c (record_full_wait_1): Likewise.
	(record_full_wait_1): Likewise.
	* regcache.c (regcache::regcache): Change parameter to const.
	* regcache.h (regcache::regcache): Likewise.
	(regcache::aspace): Return const address_space *.
	(regcache) <m_aspace>: Add const.
---
 gdb/frame.c       |  2 +-
 gdb/infrun.c      | 24 +++++++++++-------------
 gdb/record-full.c |  5 ++---
 gdb/regcache.c    |  2 +-
 gdb/regcache.h    |  8 ++++----
 5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 5380c7d..bf308ba 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1521,7 +1521,7 @@ create_sentinel_frame (struct program_space *pspace, struct regcache *regcache)
 
   frame->level = -1;
   frame->pspace = pspace;
-  frame->aspace = regcache->aspace ();
+  frame->aspace = const_cast<address_space *> (regcache->aspace ());
   /* Explicitly initialize the sentinel frame's cache.  Provide it
      with the underlying regcache.  In the future additional
      information, such as the frame's thread will be added.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0e31dbc..3e0cc86 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1760,7 +1760,7 @@ displaced_step_prepare_throw (ptid_t ptid)
   struct thread_info *tp = find_thread_ptid (ptid);
   struct regcache *regcache = get_thread_regcache (ptid);
   struct gdbarch *gdbarch = regcache->arch ();
-  struct address_space *aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
   CORE_ADDR original, copy;
   ULONGEST len;
   struct displaced_step_closure *closure;
@@ -2388,7 +2388,7 @@ resume (enum gdb_signal sig)
   struct gdbarch *gdbarch = regcache->arch ();
   struct thread_info *tp = inferior_thread ();
   CORE_ADDR pc = regcache_read_pc (regcache);
-  struct address_space *aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
   ptid_t resume_ptid;
   /* This represents the user's step vs continue request.  When
      deciding whether "set scheduler-locking step" applies, it's the
@@ -2591,7 +2591,7 @@ resume (enum gdb_signal sig)
 	  if (target_is_non_stop_p ())
 	    stop_all_threads ();
 
-	  set_step_over_info (regcache->aspace (),
+	  set_step_over_info (const_cast<address_space *> (regcache->aspace ()),
 			      regcache_read_pc (regcache), 0, tp->global_num);
 
 	  step = maybe_software_singlestep (gdbarch, pc);
@@ -2983,7 +2983,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   struct gdbarch *gdbarch;
   struct thread_info *tp;
   CORE_ADDR pc;
-  struct address_space *aspace;
   ptid_t resume_ptid;
   struct execution_control_state ecss;
   struct execution_control_state *ecs = &ecss;
@@ -3007,7 +3006,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   regcache = get_current_regcache ();
   gdbarch = regcache->arch ();
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
+
   pc = regcache_read_pc (regcache);
   tp = inferior_thread ();
 
@@ -4070,7 +4070,6 @@ adjust_pc_after_break (struct thread_info *thread,
 {
   struct regcache *regcache;
   struct gdbarch *gdbarch;
-  struct address_space *aspace;
   CORE_ADDR breakpoint_pc, decr_pc;
 
   /* If we've hit a breakpoint, we'll normally be stopped with SIGTRAP.  If
@@ -4150,7 +4149,7 @@ adjust_pc_after_break (struct thread_info *thread,
   if (decr_pc == 0)
     return;
 
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
 
   /* Find the location where (if we've hit a breakpoint) the
      breakpoint would be.  */
@@ -4385,7 +4384,6 @@ static void
 save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
 {
   struct regcache *regcache;
-  struct address_space *aspace;
 
   if (debug_infrun)
     {
@@ -4404,7 +4402,7 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
   tp->suspend.waitstatus_pending_p = 1;
 
   regcache = get_thread_regcache (tp->ptid);
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
 
   if (ws->kind == TARGET_WAITKIND_STOPPED
       && ws->value.sig == GDB_SIGNAL_TRAP)
@@ -5776,11 +5774,11 @@ handle_signal_stop (struct execution_control_state *ecs)
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
     {
       struct regcache *regcache;
-      struct address_space *aspace;
       CORE_ADDR pc;
 
       regcache = get_thread_regcache (ecs->ptid);
-      aspace = regcache->aspace ();
+      const address_space *aspace = regcache->aspace ();
+
       pc = regcache_read_pc (regcache);
 
       /* However, before doing so, if this single-step breakpoint was
@@ -5871,7 +5869,7 @@ handle_signal_stop (struct execution_control_state *ecs)
      inline function call sites).  */
   if (ecs->event_thread->control.step_range_end != 1)
     {
-      struct address_space *aspace = 
+      const address_space *aspace =
 	get_thread_regcache (ecs->ptid)->aspace ();
 
       /* skip_inline_frames is expensive, so we avoid it if we can
@@ -7758,7 +7756,7 @@ keep_going_pass_signal (struct execution_control_state *ecs)
       if (remove_bp
 	  && (remove_wps || !use_displaced_stepping (ecs->event_thread)))
 	{
-	  set_step_over_info (regcache->aspace (),
+	  set_step_over_info (const_cast<address_space *> (regcache->aspace ()),
 			      regcache_read_pc (regcache), remove_wps,
 			      ecs->event_thread->global_num);
 	}
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 0488b71..6fc29d6 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1098,7 +1098,6 @@ record_full_wait_1 (struct target_ops *ops,
 		  && status->value.sig == GDB_SIGNAL_TRAP)
 		{
 		  struct regcache *regcache;
-		  struct address_space *aspace;
 		  enum target_stop_reason *stop_reason_p
 		    = &record_full_stop_reason;
 
@@ -1109,7 +1108,7 @@ record_full_wait_1 (struct target_ops *ops,
 		  registers_changed ();
 		  regcache = get_current_regcache ();
 		  tmp_pc = regcache_read_pc (regcache);
-		  aspace = regcache->aspace ();
+		  const struct address_space *aspace = regcache->aspace ();
 
 		  if (target_stopped_by_watchpoint ())
 		    {
@@ -1172,7 +1171,7 @@ record_full_wait_1 (struct target_ops *ops,
     {
       struct regcache *regcache = get_current_regcache ();
       struct gdbarch *gdbarch = regcache->arch ();
-      struct address_space *aspace = regcache->aspace ();
+      const struct address_space *aspace = regcache->aspace ();
       int continue_flag = 1;
       int first_record_full_end = 1;
       struct cleanup *old_cleanups
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6985dc9..2cffb70 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -181,7 +181,7 @@ regcache_register_size (const struct regcache *regcache, int n)
   return register_size (regcache->arch (), n);
 }
 
-regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
+regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
 		    bool readonly_p_)
   : m_aspace (aspace_), m_readonly_p (readonly_p_)
 {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 2360e27..be87af5 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -232,7 +232,7 @@ typedef struct cached_reg
 class regcache
 {
 public:
-  regcache (gdbarch *gdbarch, address_space *aspace_)
+  regcache (gdbarch *gdbarch, const address_space *aspace_)
     : regcache (gdbarch, aspace_, true)
   {}
 
@@ -254,7 +254,7 @@ public:
   gdbarch *arch () const;
 
   /* Return REGCACHE's address space.  */
-  address_space *aspace () const
+  const address_space *aspace () const
   {
     return m_aspace;
   }
@@ -338,7 +338,7 @@ public:
 
   static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
 protected:
-  regcache (gdbarch *gdbarch, address_space *aspace_, bool readonly_p_);
+  regcache (gdbarch *gdbarch, const address_space *aspace_, bool readonly_p_);
 
   static std::forward_list<regcache *> current_regcache;
 
@@ -362,7 +362,7 @@ private:
 
   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
-  struct address_space *m_aspace;
+  const address_space *m_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
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/8] const-fy regcache::m_aspace
  2017-10-27  9:32 ` [PATCH 6/8] const-fy regcache::m_aspace Yao Qi
@ 2017-10-31 14:19   ` Simon Marchi
  2017-11-01  9:43     ` Yao Qi
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-10-31 14:19 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 2017-10-27 05:31 AM, Yao Qi wrote:
> regcache::m_aspace is a const, never changed during the lifetime of
> regcache object.

I don't really understand what this patch tries to achieve.  From the
description above, I thought you wanted to make the m_aspace field const,
not the pointed object.

If constifying the pointed address_space object is really what you meant to
do, I find having the const_cast more confusing than anything else.  I think
we should constify all the way (removing const_casts, putting more consts
where needed) or not at all.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/8] Remove regcache_descr::nr_raw_registers
  2017-10-27  9:31 ` [PATCH 4/8] Remove regcache_descr::nr_raw_registers Yao Qi
@ 2017-10-31 14:27   ` Simon Marchi
  2017-11-02 15:20     ` Yao Qi
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-10-31 14:27 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 2017-10-27 05:31 AM, Yao Qi wrote:
> struct regcache_descr has fields nr_raw_registers and gdbarch, and
> nr_raw_registers can be got via gdbarch_num_regs (gdbarch), so it looks
> nr_raw_registers is redundant.  This patch removes it.

I would suggest adding a num_regs method to regcache to wrap the call to
gdbarch_num_regs, so that if we need to change that call, we have only one
place to change.  Otherwise, LGTM.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] Construct readonly regcache without address space
  2017-10-27  9:32 ` [PATCH 8/8] Construct readonly regcache without address space Yao Qi
@ 2017-10-31 14:35   ` Simon Marchi
  2017-10-31 17:44     ` Yao Qi
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-10-31 14:35 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 2017-10-27 05:31 AM, Yao Qi wrote:
> The address space is useless to readonly regcache, so this patch removes
> the parameter to construct readonly regcache.

Can you expand on why the aspace is useless for readonly regcaches?  The
comment of m_aspace says:

  /* The address space of this register cache (for registers where it
     makes sense, like PC or SP).  */

Registers like PC or SP are present even in a readonly regcache, so I
would think that it makes sense to have the address space there as well.
So, is it that it's really useless (as in it doesn't make sense to have
it there) or that we just don't happen to use the address space right now
with readonly regcaches?

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] Construct readonly regcache without address space
  2017-10-31 14:35   ` Simon Marchi
@ 2017-10-31 17:44     ` Yao Qi
  2017-10-31 18:04       ` Simon Marchi
  0 siblings, 1 reply; 18+ messages in thread
From: Yao Qi @ 2017-10-31 17:44 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Tue, Oct 31, 2017 at 2:35 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
> On 2017-10-27 05:31 AM, Yao Qi wrote:
>> The address space is useless to readonly regcache, so this patch removes
>> the parameter to construct readonly regcache.
>
> Can you expand on why the aspace is useless for readonly regcaches?  The
> comment of m_aspace says:
>
>   /* The address space of this register cache (for registers where it
>      makes sense, like PC or SP).  */
>

This comment was there because address_space was added for read-write
regcache.  Nowadays, address_space in regcache is used for various
breakpoint/watchpoint checkings, and these regcache are not read-only
regcache.

Additionally, regcache itself doesn't use address_space at all, so various
breakpoint/watchpoint checking code should get address_space from thread
ptid rather than regcache.

> Registers like PC or SP are present even in a readonly regcache, so I
> would think that it makes sense to have the address space there as well.
> So, is it that it's really useless (as in it doesn't make sense to have
> it there) or that we just don't happen to use the address space right now
> with readonly regcaches?

It doesn't make sense to have address_space in read-only regcache, at
least.  Since we don't have a type/class for read-only regcache, we still
have to keep address_space in regcache.  However, I don't see how
address_space can be used by regcache, we can remove it from regcache
completely, but that is a separate thing.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] Construct readonly regcache without address space
  2017-10-31 17:44     ` Yao Qi
@ 2017-10-31 18:04       ` Simon Marchi
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2017-10-31 18:04 UTC (permalink / raw)
  To: Yao Qi; +Cc: Simon Marchi, gdb-patches

On 2017-10-31 13:44, Yao Qi wrote:
> On Tue, Oct 31, 2017 at 2:35 PM, Simon Marchi 
> <simon.marchi@ericsson.com> wrote:
>> On 2017-10-27 05:31 AM, Yao Qi wrote:
>>> The address space is useless to readonly regcache, so this patch 
>>> removes
>>> the parameter to construct readonly regcache.
>> 
>> Can you expand on why the aspace is useless for readonly regcaches?  
>> The
>> comment of m_aspace says:
>> 
>>   /* The address space of this register cache (for registers where it
>>      makes sense, like PC or SP).  */
>> 
> 
> This comment was there because address_space was added for read-write
> regcache.  Nowadays, address_space in regcache is used for various
> breakpoint/watchpoint checkings, and these regcache are not read-only
> regcache.
> 
> Additionally, regcache itself doesn't use address_space at all, so 
> various
> breakpoint/watchpoint checking code should get address_space from 
> thread
> ptid rather than regcache.
> 
>> Registers like PC or SP are present even in a readonly regcache, so I
>> would think that it makes sense to have the address space there as 
>> well.
>> So, is it that it's really useless (as in it doesn't make sense to 
>> have
>> it there) or that we just don't happen to use the address space right 
>> now
>> with readonly regcaches?
> 
> It doesn't make sense to have address_space in read-only regcache, at
> least.  Since we don't have a type/class for read-only regcache, we 
> still
> have to keep address_space in regcache.  However, I don't see how
> address_space can be used by regcache, we can remove it from regcache
> completely, but that is a separate thing.

Ok thanks, that explanation helps to understand.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/8] const-fy regcache::m_aspace
  2017-10-31 14:19   ` Simon Marchi
@ 2017-11-01  9:43     ` Yao Qi
  2017-11-01 14:00       ` Simon Marchi
  0 siblings, 1 reply; 18+ messages in thread
From: Yao Qi @ 2017-11-01  9:43 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@ericsson.com> writes:

> I don't really understand what this patch tries to achieve.  From the
> description above, I thought you wanted to make the m_aspace field const,
> not the pointed object.
>

I want to achieve both, the field m_aspace is a const, and the pointed
object is const too.

   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
-  struct address_space *m_aspace;
+  const address_space * const m_aspace;

> If constifying the pointed address_space object is really what you meant to
> do, I find having the const_cast more confusing than anything else.  I think
> we should constify all the way (removing const_casts, putting more consts
> where needed) or not at all.

OK, const_cast is removed in the updated patch, what do you think?

-- 
Yao (齐尧)

From 9e7e0560bafbb736a7a7fda79fd99fa37bd4c701 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 23 Oct 2017 10:42:54 +0100
Subject: [PATCH] const-fy regcache::m_aspace

regcache::m_aspace is a const, never changed during the lifetime of
regcache object.  The address_space object is a const object too.

gdb:

2017-10-23  Yao Qi  <yao.qi@linaro.org>

	* breakpoint.c (insert_single_step_breakpoints): Update.
	* frame.c (struct frame_info) <aspace>: Add const.
	(frame_save_as_regcache): Add const.
	(get_frame_address_space): Return const address_space *.
	* frame.h (get_frame_address_space): Update declaration.
	* infrun.c (struct step_over_info) <aspace>: Add const.
	(set_step_over_info): Make aspace const.
	(displaced_step_prepare_throw): Change variable const.
	(resume): Likewise.
	(proceed): Likewise.
	(adjust_pc_after_break): Likewise.
	(save_waitstatus): Likewise.
	(handle_signal_stop): Likewise.
	(keep_going_pass_signal): Likewise.
	* jit.c (jit_frame_sniffer): Add const.
	* mips-tdep.c (mips_single_step_through_delay): Likewise.
	* ppc-linux-tdep.c (ppu2spu_sniffer): Likewise.
	* record-full.c (record_full_wait_1): Likewise.
	* regcache.c (regcache::regcache): Change parameter to const.
	* regcache.h (regcache::regcache): Likewise.
	(regcache::aspace): Return const address_space *.
	(regcache) <m_aspace>: Add const.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ada72e0..98cedb4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14572,7 +14572,7 @@ insert_single_step_breakpoints (struct gdbarch *gdbarch)
   if (!next_pcs.empty ())
     {
       struct frame_info *frame = get_current_frame ();
-      struct address_space *aspace = get_frame_address_space (frame);
+      const address_space *aspace = get_frame_address_space (frame);
 
       for (CORE_ADDR pc : next_pcs)
 	insert_single_step_breakpoint (gdbarch, aspace, pc);
diff --git a/gdb/frame.c b/gdb/frame.c
index 5380c7d..e40685f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -99,7 +99,7 @@ struct frame_info
   struct program_space *pspace;
 
   /* The frame's address space.  */
-  struct address_space *aspace;
+  const address_space *aspace;
 
   /* The frame's low-level unwinder and corresponding cache.  The
      low-level unwinder is responsible for unwinding register values
@@ -1020,7 +1020,7 @@ do_frame_register_read (void *src, int regnum, gdb_byte *buf)
 std::unique_ptr<struct regcache>
 frame_save_as_regcache (struct frame_info *this_frame)
 {
-  struct address_space *aspace = get_frame_address_space (this_frame);
+  const address_space *aspace = get_frame_address_space (this_frame);
   std::unique_ptr<struct regcache> regcache
     (new struct regcache (get_frame_arch (this_frame), aspace));
 
@@ -2643,7 +2643,7 @@ frame_unwind_program_space (struct frame_info *this_frame)
   return this_frame->pspace;
 }
 
-struct address_space *
+const address_space *
 get_frame_address_space (struct frame_info *frame)
 {
   return frame->aspace;
diff --git a/gdb/frame.h b/gdb/frame.h
index 190ce76..c751002 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -494,8 +494,10 @@ extern struct program_space *get_frame_program_space (struct frame_info *);
 /* Unwind THIS frame's program space from the NEXT frame.  */
 extern struct program_space *frame_unwind_program_space (struct frame_info *);
 
+class address_space;
+
 /* Return the frame's address space.  */
-extern struct address_space *get_frame_address_space (struct frame_info *);
+extern const address_space *get_frame_address_space (struct frame_info *);
 
 /* For frames where we can not unwind further, describe why.  */
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0e31dbc..ef5a505 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1289,7 +1289,7 @@ struct step_over_info
      and address of the instruction the breakpoint is set at.  We'll
      skip inserting all breakpoints here.  Valid iff ASPACE is
      non-NULL.  */
-  struct address_space *aspace;
+  const address_space *aspace;
   CORE_ADDR address;
 
   /* The instruction being stepped over triggers a nonsteppable
@@ -1332,7 +1332,7 @@ static struct step_over_info step_over_info;
    because when we need the info later the thread may be running.  */
 
 static void
-set_step_over_info (struct address_space *aspace, CORE_ADDR address,
+set_step_over_info (const address_space *aspace, CORE_ADDR address,
 		    int nonsteppable_watchpoint_p,
 		    int thread)
 {
@@ -1760,7 +1760,7 @@ displaced_step_prepare_throw (ptid_t ptid)
   struct thread_info *tp = find_thread_ptid (ptid);
   struct regcache *regcache = get_thread_regcache (ptid);
   struct gdbarch *gdbarch = regcache->arch ();
-  struct address_space *aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
   CORE_ADDR original, copy;
   ULONGEST len;
   struct displaced_step_closure *closure;
@@ -2388,7 +2388,7 @@ resume (enum gdb_signal sig)
   struct gdbarch *gdbarch = regcache->arch ();
   struct thread_info *tp = inferior_thread ();
   CORE_ADDR pc = regcache_read_pc (regcache);
-  struct address_space *aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
   ptid_t resume_ptid;
   /* This represents the user's step vs continue request.  When
      deciding whether "set scheduler-locking step" applies, it's the
@@ -2983,7 +2983,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   struct gdbarch *gdbarch;
   struct thread_info *tp;
   CORE_ADDR pc;
-  struct address_space *aspace;
   ptid_t resume_ptid;
   struct execution_control_state ecss;
   struct execution_control_state *ecs = &ecss;
@@ -3007,7 +3006,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   regcache = get_current_regcache ();
   gdbarch = regcache->arch ();
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
+
   pc = regcache_read_pc (regcache);
   tp = inferior_thread ();
 
@@ -4070,7 +4070,6 @@ adjust_pc_after_break (struct thread_info *thread,
 {
   struct regcache *regcache;
   struct gdbarch *gdbarch;
-  struct address_space *aspace;
   CORE_ADDR breakpoint_pc, decr_pc;
 
   /* If we've hit a breakpoint, we'll normally be stopped with SIGTRAP.  If
@@ -4150,7 +4149,7 @@ adjust_pc_after_break (struct thread_info *thread,
   if (decr_pc == 0)
     return;
 
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
 
   /* Find the location where (if we've hit a breakpoint) the
      breakpoint would be.  */
@@ -4385,7 +4384,6 @@ static void
 save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
 {
   struct regcache *regcache;
-  struct address_space *aspace;
 
   if (debug_infrun)
     {
@@ -4404,7 +4402,7 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
   tp->suspend.waitstatus_pending_p = 1;
 
   regcache = get_thread_regcache (tp->ptid);
-  aspace = regcache->aspace ();
+  const address_space *aspace = regcache->aspace ();
 
   if (ws->kind == TARGET_WAITKIND_STOPPED
       && ws->value.sig == GDB_SIGNAL_TRAP)
@@ -5776,11 +5774,11 @@ handle_signal_stop (struct execution_control_state *ecs)
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
     {
       struct regcache *regcache;
-      struct address_space *aspace;
       CORE_ADDR pc;
 
       regcache = get_thread_regcache (ecs->ptid);
-      aspace = regcache->aspace ();
+      const address_space *aspace = regcache->aspace ();
+
       pc = regcache_read_pc (regcache);
 
       /* However, before doing so, if this single-step breakpoint was
@@ -5871,7 +5869,7 @@ handle_signal_stop (struct execution_control_state *ecs)
      inline function call sites).  */
   if (ecs->event_thread->control.step_range_end != 1)
     {
-      struct address_space *aspace = 
+      const address_space *aspace =
 	get_thread_regcache (ecs->ptid)->aspace ();
 
       /* skip_inline_frames is expensive, so we avoid it if we can
diff --git a/gdb/jit.c b/gdb/jit.c
index a2d1f6d..b70f87c 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1186,7 +1186,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 address_space *aspace;
   struct gdbarch *gdbarch;
 
   callbacks.reg_get = jit_unwind_reg_get_impl;
@@ -1200,7 +1199,8 @@ jit_frame_sniffer (const struct frame_unwind *self,
 
   gdb_assert (!*cache);
 
-  aspace = get_frame_address_space (this_frame);
+  const address_space *aspace = get_frame_address_space (this_frame);
+
   gdbarch = get_frame_arch (this_frame);
 
   *cache = XCNEW (struct jit_unwind_private);
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 4b19896..20bd5d3 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -6598,7 +6598,6 @@ mips_single_step_through_delay (struct gdbarch *gdbarch,
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR pc = get_frame_pc (frame);
-  struct address_space *aspace;
   enum mips_isa isa;
   ULONGEST insn;
   int status;
@@ -6616,7 +6615,9 @@ mips_single_step_through_delay (struct gdbarch *gdbarch,
   /* _has_delay_slot above will have validated the read.  */
   insn = mips_fetch_instruction (gdbarch, isa, pc, NULL);
   size = mips_insn_size (isa, insn);
-  aspace = get_frame_address_space (frame);
+
+  const address_space *aspace = get_frame_address_space (frame);
+
   return breakpoint_here_p (aspace, pc + size) != no_breakpoint_here;
 }
 
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 67d8305..d01cff8 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1362,7 +1362,7 @@ ppu2spu_sniffer (const struct frame_unwind *self,
 	  struct ppu2spu_cache *cache
 	    = FRAME_OBSTACK_CALLOC (1, struct ppu2spu_cache);
 
-	  struct address_space *aspace = get_frame_address_space (this_frame);
+	  const address_space *aspace = get_frame_address_space (this_frame);
 	  std::unique_ptr<struct regcache> regcache
 	    (new struct regcache (data.gdbarch, aspace));
 	  regcache_save (regcache.get (), ppu2spu_unwind_register, &data);
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 0488b71..6fc29d6 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1098,7 +1098,6 @@ record_full_wait_1 (struct target_ops *ops,
 		  && status->value.sig == GDB_SIGNAL_TRAP)
 		{
 		  struct regcache *regcache;
-		  struct address_space *aspace;
 		  enum target_stop_reason *stop_reason_p
 		    = &record_full_stop_reason;
 
@@ -1109,7 +1108,7 @@ record_full_wait_1 (struct target_ops *ops,
 		  registers_changed ();
 		  regcache = get_current_regcache ();
 		  tmp_pc = regcache_read_pc (regcache);
-		  aspace = regcache->aspace ();
+		  const struct address_space *aspace = regcache->aspace ();
 
 		  if (target_stopped_by_watchpoint ())
 		    {
@@ -1172,7 +1171,7 @@ record_full_wait_1 (struct target_ops *ops,
     {
       struct regcache *regcache = get_current_regcache ();
       struct gdbarch *gdbarch = regcache->arch ();
-      struct address_space *aspace = regcache->aspace ();
+      const struct address_space *aspace = regcache->aspace ();
       int continue_flag = 1;
       int first_record_full_end = 1;
       struct cleanup *old_cleanups
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6985dc9..2cffb70 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -181,7 +181,7 @@ regcache_register_size (const struct regcache *regcache, int n)
   return register_size (regcache->arch (), n);
 }
 
-regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
+regcache::regcache (gdbarch *gdbarch, const address_space *aspace_,
 		    bool readonly_p_)
   : m_aspace (aspace_), m_readonly_p (readonly_p_)
 {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 2360e27..cea76fb 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -232,7 +232,7 @@ typedef struct cached_reg
 class regcache
 {
 public:
-  regcache (gdbarch *gdbarch, address_space *aspace_)
+  regcache (gdbarch *gdbarch, const address_space *aspace_)
     : regcache (gdbarch, aspace_, true)
   {}
 
@@ -254,7 +254,7 @@ public:
   gdbarch *arch () const;
 
   /* Return REGCACHE's address space.  */
-  address_space *aspace () const
+  const address_space *aspace () const
   {
     return m_aspace;
   }
@@ -338,7 +338,7 @@ public:
 
   static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
 protected:
-  regcache (gdbarch *gdbarch, address_space *aspace_, bool readonly_p_);
+  regcache (gdbarch *gdbarch, const address_space *aspace_, bool readonly_p_);
 
   static std::forward_list<regcache *> current_regcache;
 
@@ -362,7 +362,7 @@ private:
 
   /* The address space of this register cache (for registers where it
      makes sense, like PC or SP).  */
-  struct address_space *m_aspace;
+  const address_space * const m_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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/8] const-fy regcache::m_aspace
  2017-11-01  9:43     ` Yao Qi
@ 2017-11-01 14:00       ` Simon Marchi
  2017-11-01 14:35         ` Yao Qi
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-11-01 14:00 UTC (permalink / raw)
  To: Yao Qi; +Cc: Simon Marchi, gdb-patches

On 2017-11-01 05:42, Yao Qi wrote:
> Simon Marchi <simon.marchi@ericsson.com> writes:
> 
>> I don't really understand what this patch tries to achieve.  From the
>> description above, I thought you wanted to make the m_aspace field 
>> const,
>> not the pointed object.
>> 
> 
> I want to achieve both, the field m_aspace is a const, and the pointed
> object is const too.
> 
>    /* The address space of this register cache (for registers where it
>       makes sense, like PC or SP).  */
> -  struct address_space *m_aspace;
> +  const address_space * const m_aspace;

Ah ok I had missed that there was two consts added.

>> If constifying the pointed address_space object is really what you 
>> meant to
>> do, I find having the const_cast more confusing than anything else.  I 
>> think
>> we should constify all the way (removing const_casts, putting more 
>> consts
>> where needed) or not at all.
> 
> OK, const_cast is removed in the updated patch, what do you think?

LGTM!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/8] const-fy regcache::m_aspace
  2017-11-01 14:00       ` Simon Marchi
@ 2017-11-01 14:35         ` Yao Qi
  0 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-11-01 14:35 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Simon Marchi, gdb-patches

On Wed, Nov 1, 2017 at 1:59 PM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> I want to achieve both, the field m_aspace is a const, and the pointed
>> object is const too.
>>
>>    /* The address space of this register cache (for registers where it
>>       makes sense, like PC or SP).  */
>> -  struct address_space *m_aspace;
>> +  const address_space * const m_aspace;
>
>
> Ah ok I had missed that there was two consts added.
>

The 2nd const was added in the updated patch.  It was missed in the
the first version.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/8] Remove regcache_descr::nr_raw_registers
  2017-10-31 14:27   ` Simon Marchi
@ 2017-11-02 15:20     ` Yao Qi
  0 siblings, 0 replies; 18+ messages in thread
From: Yao Qi @ 2017-11-02 15:20 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Simon Marchi <simon.marchi@ericsson.com> writes:

> I would suggest adding a num_regs method to regcache to wrap the call to
> gdbarch_num_regs, so that if we need to change that call, we have only one
> place to change.  Otherwise, LGTM.

Patch below is pushed in.

-- 
Yao (齐尧)

From f6f09fce6356c68893ad468d4c8872c4107aebcb Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Thu, 19 Oct 2017 10:41:14 +0100
Subject: [PATCH 4/8] Remove regcache_descr::nr_raw_registers

struct regcache_descr has fields nr_raw_registers and gdbarch, and
nr_raw_registers can be got via gdbarch_num_regs (gdbarch), so it looks
nr_raw_registers is redundant.  This patch removes it and adds a protected
method num_raw_registers.

gdb:

2017-11-02  Yao Qi  <yao.qi@linaro.org>

	* regcache.c (struct regcache_descr) <nr_raw_registers>: Remove.
	(init_regcache_descr): Use gdbarch_num_regs.
	(regcache::regcache): Likewise.
	(regcache::get_register_status): Likewise.
	(regcache::assert_raw_regnum): Likewise.
	(regcache::cooked_read): Likewise.
	(regcache::cooked_read_value): Likewise.
	(regcache::cooked_write): Likewise.
	(regcache::dump): Likewise.
	(regcache::num_raw_registers): New method.
	* regcache.h (class regcache) <num_raw_registers>: New.
---
 gdb/regcache.c | 29 ++++++++++++++++-------------
 gdb/regcache.h |  2 ++
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 0aee934..508f37b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -51,7 +51,6 @@ struct regcache_descr
      redundant information - if the PC is constructed from two
      registers then those registers and not the PC lives in the raw
      cache.  */
-  int nr_raw_registers;
   long sizeof_raw_registers;
 
   /* The cooked register space.  Each cooked register in the range
@@ -100,7 +99,6 @@ init_regcache_descr (struct gdbarch *gdbarch)
 
   /* Construct a strictly RAW register cache.  Don't allow pseudo's
      into the register cache.  */
-  descr->nr_raw_registers = gdbarch_num_regs (gdbarch);
 
   /* Lay out the register cache.
 
@@ -116,7 +114,7 @@ init_regcache_descr (struct gdbarch *gdbarch)
       = GDBARCH_OBSTACK_CALLOC (gdbarch, descr->nr_cooked_registers, long);
     descr->register_offset
       = GDBARCH_OBSTACK_CALLOC (gdbarch, descr->nr_cooked_registers, long);
-    for (i = 0; i < descr->nr_raw_registers; i++)
+    for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
       {
 	descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]);
 	descr->register_offset[i] = offset;
@@ -199,8 +197,7 @@ regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
   else
     {
       m_registers = XCNEWVEC (gdb_byte, m_descr->sizeof_raw_registers);
-      m_register_status = XCNEWVEC (signed char,
-				    m_descr->nr_raw_registers);
+      m_register_status = XCNEWVEC (signed char, gdbarch_num_regs (gdbarch));
     }
   m_ptid = minus_one_ptid;
 }
@@ -378,7 +375,7 @@ regcache::get_register_status (int regnum) const
   if (m_readonly_p)
     gdb_assert (regnum < m_descr->nr_cooked_registers);
   else
-    gdb_assert (regnum < m_descr->nr_raw_registers);
+    gdb_assert (regnum < num_raw_registers ());
 
   return (enum register_status) m_register_status[regnum];
 }
@@ -401,7 +398,7 @@ regcache::invalidate (int regnum)
 void
 regcache::assert_regnum (int regnum) const
 {
-  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (regnum >= 0 && regnum < gdbarch_num_regs (arch ()));
 }
 
 /* Global structure containing the current regcache.  */
@@ -677,7 +674,7 @@ regcache::cooked_read (int regnum, gdb_byte *buf)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
-  if (regnum < m_descr->nr_raw_registers)
+  if (regnum < num_raw_registers ())
     return raw_read (regnum, buf);
   else if (m_readonly_p
 	   && m_register_status[regnum] != REG_UNKNOWN)
@@ -731,7 +728,7 @@ regcache::cooked_read_value (int regnum)
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
 
-  if (regnum < m_descr->nr_raw_registers
+  if (regnum < num_raw_registers ()
       || (m_readonly_p && m_register_status[regnum] != REG_UNKNOWN)
       || !gdbarch_pseudo_register_read_value_p (m_descr->gdbarch))
     {
@@ -890,7 +887,7 @@ regcache::cooked_write (int regnum, const gdb_byte *buf)
 {
   gdb_assert (regnum >= 0);
   gdb_assert (regnum < m_descr->nr_cooked_registers);
-  if (regnum < m_descr->nr_raw_registers)
+  if (regnum < num_raw_registers ())
     raw_write (regnum, buf);
   else
     gdbarch_pseudo_register_write (m_descr->gdbarch, this,
@@ -1280,6 +1277,12 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc)
   reinit_frame_cache ();
 }
 
+int
+regcache::num_raw_registers () const
+{
+  return gdbarch_num_regs (arch ());
+}
+
 void
 regcache::debug_print_register (const char *func,  int regno)
 {
@@ -1435,7 +1438,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	{
 	  if (regnum < 0)
 	    fprintf_unfiltered (file, "Raw value");
-	  else if (regnum >= m_descr->nr_raw_registers)
+	  else if (regnum >= num_raw_registers ())
 	    fprintf_unfiltered (file, "<cooked>");
 	  else if (get_register_status (regnum) == REG_UNKNOWN)
 	    fprintf_unfiltered (file, "<invalid>");
@@ -1461,7 +1464,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	      enum register_status status;
 	      struct value *value = NULL;
 
-	      if (regnum < m_descr->nr_raw_registers)
+	      if (regnum < num_raw_registers ())
 		{
 		  raw_update (regnum);
 		  status = get_register_status (regnum);
@@ -1529,7 +1532,7 @@ regcache::dump (ui_file *file, enum regcache_dump_what what_to_dump)
 	    {
 	      fprintf_unfiltered (file, "Rmt Nr  g/G Offset");
 	    }
-	  else if (regnum < m_descr->nr_raw_registers)
+	  else if (regnum < num_raw_registers ())
 	    {
 	      int pnum, poffset;
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 6fb790d..696b776 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -343,6 +343,8 @@ public:
 protected:
   regcache (gdbarch *gdbarch, address_space *aspace_, bool readonly_p_);
 
+  int num_raw_registers () const;
+
   static std::forward_list<regcache *> current_regcache;
 
 private:
-- 
1.9.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-11-02 15:20 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27  9:31 [PATCH 0/8] regcache misc cleanup and refactor Yao Qi
2017-10-27  9:31 ` [PATCH 4/8] Remove regcache_descr::nr_raw_registers Yao Qi
2017-10-31 14:27   ` Simon Marchi
2017-11-02 15:20     ` Yao Qi
2017-10-27  9:31 ` [PATCH 1/8] Remove regcache_descr fields sizeof_raw_register_status and sizeof_cooked_register_status Yao Qi
2017-10-27  9:31 ` [PATCH 3/8] New method regcache::assert_regnum Yao Qi
2017-10-27  9:32 ` [PATCH 2/8] Remove code wrapped by "#if 0" Yao Qi
2017-10-27  9:32 ` [PATCH 6/8] const-fy regcache::m_aspace Yao Qi
2017-10-31 14:19   ` Simon Marchi
2017-11-01  9:43     ` Yao Qi
2017-11-01 14:00       ` Simon Marchi
2017-11-01 14:35         ` Yao Qi
2017-10-27  9:32 ` [PATCH 8/8] Construct readonly regcache without address space Yao Qi
2017-10-31 14:35   ` Simon Marchi
2017-10-31 17:44     ` Yao Qi
2017-10-31 18:04       ` Simon Marchi
2017-10-27  9:32 ` [PATCH 7/8] const-fy regcache::m_readonly_p Yao Qi
2017-10-27  9:32 ` [PATCH 5/8] s/get_regcache_aspace (regcache)/regcache->aspace ()/g Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).