* [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache
@ 2017-08-17 8:47 Alan Hayward
2017-08-23 10:33 ` Yao Qi
0 siblings, 1 reply; 3+ messages in thread
From: Alan Hayward @ 2017-08-17 8:47 UTC (permalink / raw)
To: gdb-patches; +Cc: nd
This patch creates the target_regcache, subclassed from regcache.
All ptid related functions are moved to target_regcache.
A regcache retains the ptid () method, which always returns -1.
This ensures users can always test if a regcache is attached to a
target, without needing to know if it is a target_regcache.
The get_current_regcache and get_thread_* functions now all return
a target_regcache. It is perfectly safe for the existing code to
treat the result as a regcache.
A target_regcache cannot be read only, because it does not make sense
that a regcache connected to a target would be read only.
Tested on a --enable-targets=all build with board files unix,
native-gdbserver and unittest.exp.
2017-08-16 Alan Hayward <alan.hayward@arm.com>
* gdbarch-selftests.c: Use target_regcache.
* regcache.c (target_regcache::target_regcache): New constructor.
(get_thread_arch_aspace_regcache): Use target_regcache.
(get_thread_regcache_for_ptid): Likewise.
(target_regcache::regcache_thread_ptid_changed): Likewise.
(registers_changed_ptid): Likewise.
(current_regcache_test): Likewise.
(_initialize_regcache): Likewise.
* regcache.h: New class target_regcache.
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 096ba975f75edf6e2de81a767f2b645ff2405fd7..a6251d2a9f87ccff310f90eb5c27bc4199844f22 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -27,11 +27,11 @@ namespace selftests {
/* A read-write regcache which doesn't write the target. */
-class regcache_test : public regcache
+class regcache_test : public target_regcache
{
public:
explicit regcache_test (struct gdbarch *gdbarch)
- : regcache (gdbarch, NULL, false)
+ : target_regcache (gdbarch, NULL)
{
set_ptid (inferior_ptid);
diff --git a/gdb/regcache.h b/gdb/regcache.h
index aa64a000acf06b90dc7f2f519b07f8ff7e3903b4..bc3f24cffe1413d521998ab3c8081833a2735754 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -24,14 +24,15 @@
#include <forward_list>
struct regcache;
+class target_regcache;
struct regset;
struct gdbarch;
struct address_space;
-extern struct regcache *get_current_regcache (void);
-extern struct regcache *get_thread_regcache (ptid_t ptid);
-extern struct regcache *get_thread_arch_regcache (ptid_t, struct gdbarch *);
-extern struct regcache *get_thread_arch_aspace_regcache (ptid_t,
+extern target_regcache *get_current_regcache (void);
+extern target_regcache *get_thread_regcache (ptid_t ptid);
+extern target_regcache *get_thread_arch_regcache (ptid_t, struct gdbarch *);
+extern target_regcache *get_thread_arch_aspace_regcache (ptid_t,
struct gdbarch *,
struct address_space *);
@@ -240,7 +241,8 @@ typedef struct cached_reg
gdb_byte *data;
} cached_reg_t;
-/* The register cache for storing raw register values. */
+
+/* A register cache. This is not connected to a target and ptid is unset. */
class regcache
{
@@ -344,41 +346,23 @@ public:
void dump (ui_file *file, enum regcache_dump_what what_to_dump);
- ptid_t ptid () const
- {
- return m_ptid;
- }
-
- void set_ptid (const ptid_t ptid)
+ virtual ptid_t ptid () const
{
- this->m_ptid = ptid;
+ /* Ptid of a non-target regcache is always -1. */
+ return (ptid_t) -1;
}
/* Dump the contents of a register from the register cache to the target
debug. */
void debug_print_register (const char *func, int regno);
- static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
protected:
regcache (gdbarch *gdbarch, address_space *aspace_, bool readonly_p_);
- static std::forward_list<regcache *> current_regcache;
-
-private:
gdb_byte *register_buffer (int regnum) const;
void restore (struct regcache *src);
- enum register_status xfer_part (int regnum, int offset, int len, void *in,
- const void *out,
- decltype (regcache_raw_read) read,
- decltype (regcache_raw_write) write);
-
- void transfer_regset (const struct regset *regset,
- struct regcache *out_regcache,
- int regnum, const void *in_buf,
- void *out_buf, size_t size) const;
-
struct regcache_descr *m_descr;
/* The address space of this register cache (for registers where it
@@ -389,6 +373,7 @@ private:
full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a read/write
register cache can only hold [0 .. gdbarch_num_regs). */
gdb_byte *m_registers;
+
/* Register cache status. */
signed char *m_register_status;
/* Is this a read-only cache? A read-only cache is used for saving
@@ -398,21 +383,63 @@ private:
regcache_cpy(). The actual contents are determined by the
reggroup_save and reggroup_restore methods. */
bool m_readonly_p;
- /* If this is a read-write cache, which thread's registers is
- it connected to? */
- ptid_t m_ptid;
- friend struct regcache *
- get_thread_arch_aspace_regcache (ptid_t ptid, struct gdbarch *gdbarch,
- struct address_space *aspace);
+private:
- friend void
- registers_changed_ptid (ptid_t ptid);
+ enum register_status xfer_part (int regnum, int offset, int len, void *in,
+ const void *out,
+ decltype (regcache_raw_read) read,
+ decltype (regcache_raw_write) write);
+
+ void transfer_regset (const struct regset *regset,
+ struct regcache *out_regcache,
+ int regnum, const void *in_buf,
+ void *out_buf, size_t size) const;
+
+private:
friend void
regcache_cpy (struct regcache *dst, struct regcache *src);
};
+
+/* A register cache that can be attached to a target. ptid can be set. */
+
+class target_regcache : public regcache
+{
+public:
+
+ ptid_t ptid () const
+ {
+ return m_ptid;
+ }
+
+ static void regcache_thread_ptid_changed (ptid_t old_ptid,
+ ptid_t new_ptid);
+
+protected:
+
+ /* Constructor is only called via get_thread_arch_aspace_regcache. */
+ target_regcache (gdbarch *gdbarch, address_space *aspace_);
+
+ void set_ptid (const ptid_t ptid)
+ {
+ this->m_ptid = ptid;
+ }
+
+ static std::forward_list<target_regcache *> current_regcache;
+
+private:
+
+ /* The thread the cache is connected to, or -1 if not attached. */
+ ptid_t m_ptid;
+
+ friend struct target_regcache * get_thread_arch_aspace_regcache
+ (ptid_t ptid, struct gdbarch *gdbarch, struct address_space *aspace);
+
+ friend void registers_changed_ptid (ptid_t ptid);
+};
+
/* Duplicate the contents of a register cache to a read-only register
cache. The operation is pass-through. */
extern struct regcache *regcache_dup (struct regcache *regcache);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index e8f92d684dd80a197e055aa1b1bbe2caaab2392a..3d6ca86006fa59463069808f6e5b355028c4d3cc 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -207,7 +207,15 @@ regcache::regcache (gdbarch *gdbarch, address_space *aspace_,
m_register_status = XCNEWVEC (signed char,
m_descr->sizeof_raw_register_status);
}
+}
+
+target_regcache::target_regcache (gdbarch *gdbarch, address_space *aspace_)
+ : regcache (gdbarch, aspace_, false)
+{
m_ptid = minus_one_ptid;
+
+ /* A target_regcache should never be readonly. */
+ gdb_assert (!m_readonly_p);
}
static enum register_status
@@ -440,25 +448,25 @@ regcache::invalidate (int regnum)
recording if the register values have been changed (eg. by the
user). Therefore all registers must be written back to the
target when appropriate. */
-std::forward_list<regcache *> regcache::current_regcache;
+std::forward_list<target_regcache *> target_regcache::current_regcache;
-struct regcache *
+target_regcache *
get_thread_arch_aspace_regcache (ptid_t ptid, struct gdbarch *gdbarch,
struct address_space *aspace)
{
- for (const auto ®cache : regcache::current_regcache)
+ for (const auto ®cache : target_regcache::current_regcache)
if (ptid_equal (regcache->ptid (), ptid) && regcache->arch () == gdbarch)
return regcache;
- regcache *new_regcache = new regcache (gdbarch, aspace, false);
+ target_regcache *new_regcache = new target_regcache (gdbarch, aspace);
- regcache::current_regcache.push_front (new_regcache);
+ target_regcache::current_regcache.push_front (new_regcache);
new_regcache->set_ptid (ptid);
return new_regcache;
}
-struct regcache *
+target_regcache *
get_thread_arch_regcache (ptid_t ptid, struct gdbarch *gdbarch)
{
struct address_space *aspace;
@@ -479,7 +487,7 @@ get_thread_arch_regcache (ptid_t ptid, struct gdbarch *gdbarch)
static ptid_t current_thread_ptid;
static struct gdbarch *current_thread_arch;
-struct regcache *
+target_regcache *
get_thread_regcache (ptid_t ptid)
{
if (!current_thread_arch || !ptid_equal (current_thread_ptid, ptid))
@@ -491,7 +499,7 @@ get_thread_regcache (ptid_t ptid)
return get_thread_arch_regcache (ptid, current_thread_arch);
}
-struct regcache *
+target_regcache *
get_current_regcache (void)
{
return get_thread_regcache (inferior_ptid);
@@ -502,7 +510,7 @@ get_current_regcache (void)
struct regcache *
get_thread_regcache_for_ptid (ptid_t ptid)
{
- return get_thread_regcache (ptid);
+ return (struct regcache*) get_thread_regcache (ptid);
}
/* Observer for the target_changed event. */
@@ -516,9 +524,9 @@ regcache_observer_target_changed (struct target_ops *target)
/* Update global variables old ptids to hold NEW_PTID if they were
holding OLD_PTID. */
void
-regcache::regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
+target_regcache::regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
{
- for (auto ®cache : regcache::current_regcache)
+ for (auto ®cache : target_regcache::current_regcache)
{
if (ptid_equal (regcache->ptid (), old_ptid))
regcache->set_ptid (new_ptid);
@@ -539,15 +547,15 @@ regcache::regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
void
registers_changed_ptid (ptid_t ptid)
{
- for (auto oit = regcache::current_regcache.before_begin (),
+ for (auto oit = target_regcache::current_regcache.before_begin (),
it = std::next (oit);
- it != regcache::current_regcache.end ();
+ it != target_regcache::current_regcache.end ();
)
{
if (ptid_match ((*it)->ptid (), ptid))
{
delete *it;
- it = regcache::current_regcache.erase_after (oit);
+ it = target_regcache::current_regcache.erase_after (oit);
}
else
oit = it++;
@@ -1668,7 +1676,7 @@ maintenance_print_remote_registers (char *args, int from_tty)
namespace selftests {
-class regcache_access : public regcache
+class regcache_access : public target_regcache
{
public:
@@ -1677,8 +1685,8 @@ public:
static size_t
current_regcache_size ()
{
- return std::distance (regcache::current_regcache.begin (),
- regcache::current_regcache.end ());
+ return std::distance (target_regcache::current_regcache.begin (),
+ target_regcache::current_regcache.end ());
}
};
@@ -1692,7 +1700,7 @@ current_regcache_test (void)
/* Get regcache from ptid1, a new regcache is added to
current_regcache. */
- regcache *regcache = get_thread_arch_aspace_regcache (ptid1,
+ target_regcache *regcache = get_thread_arch_aspace_regcache (ptid1,
target_gdbarch (),
NULL);
@@ -1745,7 +1753,8 @@ _initialize_regcache (void)
= gdbarch_data_register_post_init (init_regcache_descr);
observer_attach_target_changed (regcache_observer_target_changed);
- observer_attach_thread_ptid_changed (regcache::regcache_thread_ptid_changed);
+ observer_attach_thread_ptid_changed
+ (target_regcache::regcache_thread_ptid_changed);
add_com ("flushregs", class_maintenance, reg_flush_command,
_("Force gdb to flush its register cache (maintainer command)"));
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache
2017-08-17 8:47 [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache Alan Hayward
@ 2017-08-23 10:33 ` Yao Qi
2017-08-23 13:24 ` Alan Hayward
0 siblings, 1 reply; 3+ messages in thread
From: Yao Qi @ 2017-08-23 10:33 UTC (permalink / raw)
To: Alan Hayward; +Cc: gdb-patches, nd
Alan Hayward <Alan.Hayward@arm.com> writes:
> All ptid related functions are moved to target_regcache.
>
What is the rationale of this change? The regcache is per-thread, even
it is disconnected from target.
> A regcache retains the ptid () method, which always returns -1.
ptid_t (-1) is minus_one_ptid, which has a special meaning.
> This ensures users can always test if a regcache is attached to a
> target, without needing to know if it is a target_regcache.
When do we need such test ("if a regcache is attached to a target")?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache
2017-08-23 10:33 ` Yao Qi
@ 2017-08-23 13:24 ` Alan Hayward
0 siblings, 0 replies; 3+ messages in thread
From: Alan Hayward @ 2017-08-23 13:24 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, nd
> On 23 Aug 2017, at 11:33, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Alan Hayward <Alan.Hayward@arm.com> writes:
>
>> All ptid related functions are moved to target_regcache.
>>
>
> What is the rationale of this change? The regcache is per-thread, even
> it is disconnected from target.
In the existing code, when calling regcache_dup / copy constructor,
ptid of the new recache is always set to -1.
In the save / restore functions the ptid is not updated.
The regcache.c functions which read/write the ptid only do that using the
target connected regcaches.
Calling regcache_get_ptid on a readonly regcache will result in an assert
firing.
Therefore, in existing code, a readonly recache will always have a ptid
of -1. In the new code this property now becomes part of a detached
regcache.
However.....
>
>> A regcache retains the ptid () method, which always returns -1.
>
> ptid_t (-1) is minus_one_ptid, which has a special meaning.
Agreed. The existing code already treats -1 tpid to mean different things.
I’ve been thinking about this again.
regcache_get_ptid asserts if ptid is -1. Therefore ptid() should also
assert on a detached recache ?
With this change, a detached recache would never have a ptid.
I think that simplifies the code too.
>
>> This ensures users can always test if a regcache is attached to a
>> target, without needing to know if it is a target_regcache.
>
> When do we need such test ("if a regcache is attached to a target”)?
We don’t. I’m happy to drop this statement.
If we do need to add a test, there will probably be a better way of doing it.
>
> --
> Yao (齐尧)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-23 13:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17 8:47 [PATCH 1/7] Regcache: Subclass ptid functionality to target_regcache Alan Hayward
2017-08-23 10:33 ` Yao Qi
2017-08-23 13:24 ` Alan Hayward
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).