[8/8] Construct readonly regcache without address space
Commit Message
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
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
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.
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
@@ -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;
@@ -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;
@@ -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);
@@ -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);
}
}
@@ -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 {};