diff mbox

[8/8] Construct readonly regcache without address space

Message ID 1509096702-12202-9-git-send-email-yao.qi@linaro.org
State New
Headers show

Commit Message

Yao Qi Oct. 27, 2017, 9:31 a.m. UTC
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(-)

Comments

Simon Marchi Oct. 31, 2017, 2:35 p.m. UTC | #1
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
Yao Qi Oct. 31, 2017, 5:44 p.m. UTC | #2
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.
Simon Marchi Oct. 31, 2017, 6:04 p.m. UTC | #3
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
diff mbox

Patch

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 {};