This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[PATCH 4/7] Regcache: Refactor dup/cpy/save/restore
- From: Alan Hayward <Alan dot Hayward at arm dot com>
- To: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Cc: nd <nd at arm dot com>
- Date: Thu, 17 Aug 2017 08:48:10 +0000
- Subject: [PATCH 4/7] Regcache: Refactor dup/cpy/save/restore
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan dot Hayward at arm dot com;
- Nodisclaimer: True
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
This patch refactors and simplifies the methods for copying a regcache.
In the existing code, save/restore/dup/cpy differed on whether the dest and
source were readonly or not readonly.
These changes change the dependency to whether the dest and source are a
target_regcache or regcache.
The functions are now:
save () : This is deleted from target_regcache, ensuring the destination
is always a detached regcache. The code does not care if the source is a
target_regache or not.
restore_to () : Replacement for restore. This is deleted from target_regcache.
It ensures that the target is a regcache and the source is a target_regcache.
dup () : There is a different regcache and target_regcache version of this.
Both versions return a detached regcache.
regcache_cpy () : Removed. The above methods give everything required.
regcache(readonly_t, regcache*) : Removed. Use dup() instead.
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>
* frame.c (frame_save_as_regcache): Use save.
(frame_pop): Use restore_to.
* infcmd.c (get_return_value): Use dup.
* infrun.c (save_infcall_suspend_state): Likewise.
(restore_infcall_suspend_state): Use restore_to.
* linux-fork.c (fork_load_infrun_state): Likewise.
(fork_save_infrun_state): Use dup.
* ppc-linux-tdep.c (ppu2spu_sniffer): Use save.
* regcache.c (regcache::regcache): Remove function.
(regcache_save): Remove function.
(regcache::save): Remove checks.
(regcache::restore_to): New version of...
(regcache::restore): ...this.
(regcache::cpy): Remove function.
(regcache::dup): Copy from regcache.
(target_regcache::dup): Likewise.
(regcache_dup): Remove function.
* spu-tdep.c (spu2ppu_sniffer): Use dup.
diff --git a/gdb/frame.c b/gdb/frame.c
index 7fd4b07a2e95f28b2eb6a18dea4d2071f0ece4e2..30cd8532e6dcb91510c527f0c3c524558a7a6058 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1024,7 +1024,7 @@ frame_save_as_regcache (struct frame_info *this_frame)
regcache *backup = new regcache (get_frame_arch (this_frame), aspace);
struct cleanup *cleanups = make_cleanup_regcache_delete (backup);
- regcache_save (backup, do_frame_register_read, this_frame);
+ backup->save (do_frame_register_read, this_frame);
discard_cleanups (cleanups);
return backup;
}
@@ -1072,9 +1072,8 @@ frame_pop (struct frame_info *this_frame)
Unfortunately, they don't implement it. Their lack of a formal
definition can lead to targets writing back bogus values
(arguably a bug in the target code mind). */
- /* Now copy those saved registers into the current regcache.
- Here, regcache_cpy() calls regcache_restore(). */
- regcache_cpy (get_current_regcache (), scratch);
+ /* Now copy those saved registers into the current regcache. */
+ scratch->restore_to (get_current_regcache ());
do_cleanups (cleanups);
/* We've made right mess of GDB's local state, just discard
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index defa7b0c48cd3a1a90786ba3d883a986247a3b5d..52da4ba0e68417b7b515d1382f3f107e9d262130 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1602,8 +1602,8 @@ advance_command (char *arg, int from_tty)
struct value *
get_return_value (struct value *function, struct type *value_type)
{
- regcache stop_regs (regcache::readonly, *get_current_regcache ());
- struct gdbarch *gdbarch = stop_regs.arch ();
+ regcache *stop_regs = get_current_regcache ()->dup ();
+ struct gdbarch *gdbarch = stop_regs->arch ();
struct value *value;
value_type = check_typedef (value_type);
@@ -1623,7 +1623,7 @@ get_return_value (struct value *function, struct type *value_type)
case RETURN_VALUE_ABI_RETURNS_ADDRESS:
case RETURN_VALUE_ABI_PRESERVES_ADDRESS:
value = allocate_value (value_type);
- gdbarch_return_value (gdbarch, function, value_type, &stop_regs,
+ gdbarch_return_value (gdbarch, function, value_type, stop_regs,
value_contents_raw (value), NULL);
break;
case RETURN_VALUE_STRUCT_CONVENTION:
@@ -1633,6 +1633,7 @@ get_return_value (struct value *function, struct type *value_type)
internal_error (__FILE__, __LINE__, _("bad switch"));
}
+ delete stop_regs;
return value;
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6510aec548f6495ca64f5b566ff3628734698113..485cdccf133a055382b569c9d20a6f2f76c4c0ba 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8911,7 +8911,7 @@ save_infcall_suspend_state (void)
inf_state->stop_pc = stop_pc;
- inf_state->registers = regcache_dup (regcache);
+ inf_state->registers = regcache->dup ();
return inf_state;
}
@@ -8922,7 +8922,7 @@ void
restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
{
struct thread_info *tp = inferior_thread ();
- struct regcache *regcache = get_current_regcache ();
+ target_regcache *regcache = get_current_regcache ();
struct gdbarch *gdbarch = get_regcache_arch (regcache);
tp->suspend = inf_state->thread_suspend;
@@ -8942,7 +8942,7 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
(and perhaps other times). */
if (target_has_execution)
/* NB: The register write goes through to the target. */
- regcache_cpy (regcache, inf_state->registers);
+ inf_state->registers->restore_to (regcache);
discard_infcall_suspend_state (inf_state);
}
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 032ff62807b728be336bf0e5daf59e47615d4799..d307aa7162cdd2b8ac1536e4eb1a1936ed6766f5 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -264,7 +264,7 @@ fork_load_infrun_state (struct fork_info *fp)
linux_nat_switch_fork (fp->ptid);
if (fp->savedregs && fp->clobber_regs)
- regcache_cpy (get_current_regcache (), fp->savedregs);
+ fp->savedregs->restore_to (get_current_regcache ());
registers_changed ();
reinit_frame_cache ();
@@ -297,7 +297,7 @@ fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
if (fp->savedregs)
delete fp->savedregs;
- fp->savedregs = regcache_dup (get_current_regcache ());
+ fp->savedregs = get_current_regcache ()->dup ();
fp->clobber_regs = clobber_regs;
if (clobber_regs)
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 42aff2cd1bf3834268b0b58dcf00dac1c52add96..285ab74cbf656f501805663b4e1063f28d5269d0 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1366,7 +1366,7 @@ ppu2spu_sniffer (const struct frame_unwind *self,
struct address_space *aspace = get_frame_address_space (this_frame);
regcache *backup = new regcache (data.gdbarch, aspace);
struct cleanup *cleanups = make_cleanup_regcache_delete (backup);
- regcache_save (backup, ppu2spu_unwind_register, &data);
+ backup->save (ppu2spu_unwind_register, &data);
discard_cleanups (cleanups);
cache->frame_id = frame_id_build (base, func);
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 00b87db7d145205b74859c08ca5a9d852441a4aa..1437dac220e0364557e3c568f1c5223cec598dfd 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -209,20 +209,10 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum);
extern int register_size (struct gdbarch *gdbarch, int regnum);
-
-/* Save/restore a register cache. The set of registers saved /
- restored into the DST regcache determined by the save_reggroup /
- restore_reggroup respectively. COOKED_READ returns zero iff the
- register's value can't be returned. */
-
typedef enum register_status (regcache_cooked_read_ftype) (void *src,
int regnum,
gdb_byte *buf);
-extern void regcache_save (struct regcache *dst,
- regcache_cooked_read_ftype *cooked_read,
- void *cooked_read_context);
-
enum regcache_dump_what
{
regcache_dump_none, regcache_dump_raw,
@@ -249,12 +239,6 @@ public:
: regcache (gdbarch, aspace_, true)
{}
- struct readonly_t {};
- static constexpr readonly_t readonly {};
-
- /* Create a readonly regcache from a non-readonly regcache. */
- regcache (readonly_t, const regcache &src);
-
regcache (const regcache &) = delete;
void operator= (const regcache &) = delete;
@@ -271,8 +255,17 @@ public:
return m_aspace;
}
+ /* Duplicate self into a new regcache. */
+ virtual regcache* dup ();
+
+ /* Copy the register contents from a target_regcache to self.
+ All cooked registers are read and cached. */
void save (regcache_cooked_read_ftype *cooked_read, void *src);
+ /* Copy register contents to a target_regcache. All cached cooked registers
+ are also restored. */
+ void restore_to (target_regcache *dst);
+
enum register_status cooked_read (int regnum, gdb_byte *buf);
void cooked_write (int regnum, const gdb_byte *buf);
@@ -348,8 +341,6 @@ protected:
gdb_byte *register_buffer (int regnum) const;
- void restore (struct regcache *src);
-
struct regcache_descr *m_descr;
/* The address space of this register cache (for registers where it
@@ -363,12 +354,10 @@ protected:
/* Register cache status. */
signed char *m_register_status;
- /* Is this a read-only cache? A read-only cache is used for saving
- the target's register state (e.g, across an inferior function
- call or just before forcing a function return). A read-only
- 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. */
+
+ /* A read-only cache can not change it's register contents, except from
+ an target_regcache via the save () method.
+ A target_regcache cache can never be read-only. */
bool m_readonly_p;
private:
@@ -385,8 +374,6 @@ private:
private:
- friend void
- regcache_cpy (struct regcache *dst, struct regcache *src);
};
@@ -397,6 +384,16 @@ class target_regcache : public regcache
{
public:
+ target_regcache (const target_regcache &) = delete;
+ void operator= (const target_regcache &) = delete;
+
+ /* Cannot be called on a target_regcache. */
+ void save (regcache_cooked_read_ftype *cooked_read, void *src) = delete;
+ void restore_to (target_regcache *dst) = delete;
+
+ /* Duplicate self into a new regcache. Result is not a target_regcache. */
+ regcache* dup ();
+
/* Overridden regcache methods. These versions will pass the read/write
through to the target. */
enum register_status raw_read (int regnum, gdb_byte *buf);
@@ -434,14 +431,6 @@ private:
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);
-
-/* Writes to DEST will go through to the target. SRC is a read-only
- register cache. */
-extern void regcache_cpy (struct regcache *dest, struct regcache *src);
-
extern void registers_changed (void);
extern void registers_changed_ptid (ptid_t);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index e7da5a622e861e1e6a1938f91a0dd9a39942a050..5405dba7a706910f0b6d20c77eef657e38695b34 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -226,13 +226,6 @@ do_cooked_read (void *src, int regnum, gdb_byte *buf)
return regcache_cooked_read (regcache, regnum, buf);
}
-regcache::regcache (readonly_t, const regcache &src)
- : regcache (src.arch (), src.aspace (), true)
-{
- gdb_assert (!src.m_readonly_p);
- save (do_cooked_read, (void *) &src);
-}
-
gdbarch *
regcache::arch () const
{
@@ -312,23 +305,11 @@ regcache::register_buffer (int regnum) const
}
void
-regcache_save (struct regcache *regcache,
- regcache_cooked_read_ftype *cooked_read, void *src)
-{
- regcache->save (cooked_read, src);
-}
-
-void
-regcache::save (regcache_cooked_read_ftype *cooked_read,
- void *src)
+regcache::save (regcache_cooked_read_ftype *cooked_read, void *src)
{
struct gdbarch *gdbarch = m_descr->gdbarch;
int regnum;
- /* The DST should be `read-only', if it wasn't then the save would
- end up trying to write the register values back out to the
- target. */
- 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);
@@ -354,15 +335,14 @@ regcache::save (regcache_cooked_read_ftype *cooked_read,
}
void
-regcache::restore (struct regcache *src)
+regcache::restore_to (target_regcache *dst)
{
struct gdbarch *gdbarch = m_descr->gdbarch;
int regnum;
+ gdb_assert (dst != NULL);
+ gdb_assert (dst->m_descr->gdbarch == m_descr->gdbarch);
+ gdb_assert (dst != this);
- /* The dst had better not be read-only. If it is, the `restore'
- doesn't make much sense. */
- gdb_assert (!m_readonly_p);
- gdb_assert (src->m_readonly_p);
/* Copy over any registers, being careful to only restore those that
were both saved and need to be restored. The full [0 .. gdbarch_num_regs
+ gdbarch_num_pseudo_regs) range is checked since some architectures need
@@ -371,27 +351,33 @@ regcache::restore (struct regcache *src)
{
if (gdbarch_register_reggroup_p (gdbarch, regnum, restore_reggroup))
{
- if (src->m_register_status[regnum] == REG_VALID)
- cooked_write (regnum, src->register_buffer (regnum));
+ if (m_register_status[regnum] == REG_VALID)
+ dst->cooked_write (regnum, register_buffer (regnum));
}
}
}
-void
-regcache_cpy (struct regcache *dst, struct regcache *src)
+/* Duplicate detached regcache to a detached regcache. */
+regcache*
+regcache::dup ()
{
- gdb_assert (src != NULL && dst != NULL);
- gdb_assert (src->m_descr->gdbarch == dst->m_descr->gdbarch);
- gdb_assert (src != dst);
- gdb_assert (src->m_readonly_p && !dst->m_readonly_p);
+ regcache *new_regcache = new regcache (arch (), aspace ());
+
+ memcpy (new_regcache->m_registers, m_registers,
+ m_descr->sizeof_cooked_registers);
+ memcpy (new_regcache->m_register_status, m_register_status,
+ m_descr->sizeof_cooked_register_status);
- dst->restore (src);
+ return new_regcache;
}
-struct regcache *
-regcache_dup (struct regcache *src)
+/* Duplicate a target_regcache to a detached regcache. */
+regcache*
+target_regcache::dup ()
{
- return new regcache (regcache::readonly, *src);
+ regcache *new_regcache = new regcache (arch (), aspace ());
+ new_regcache->save (do_cooked_read, (void *) this);
+ return new_regcache;
}
enum register_status
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 1ac763da7ede9d4b9f4bff1e8d71551b0c776fa1..59ed3851a62df324a61ca3c604f22548a1630f7d 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1275,7 +1275,7 @@ spu2ppu_sniffer (const struct frame_unwind *self,
{
struct regcache *regcache;
regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
- cache->regcache = regcache_dup (regcache);
+ cache->regcache = regcache->dup ();
*this_prologue_cache = cache;
return 1;
}