This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/8] C++-ify ravenscar_arch_ops
On 02/07/2019 09:40 AM, Tom Tromey wrote:
> From: Tom Tromey <tromey@adacore.com>
>
> This turns ravenscar_arch_ops into an abstract base class and updates
> all the places where it is used. This is an improvement because it
> avoids any possibility of forgetting to set one of the function
> pointers. It also makes clear that these functions aren't intended to
> be changed dynamically.
LGTM, though I'd probably eliminate the prepare_to_store method or
make it not-abstract -- seems that no architecture does anything with it.
Thanks,
Pedro Alves
>
> gdb/ChangeLog
> 2019-02-07 Tom Tromey <tromey@adacore.com>
>
> * sparc-ravenscar-thread.c (struct sparc_ravenscar_ops): Derive
> from ravenscar_arch_ops.
> (sparc_ravenscar_ops::fetch_registers)
> (sparc_ravenscar_ops::prepare_to_store)
> (sparc_ravenscar_ops::store_registers): Now methods.
> (sparc_ravenscar_ops): Redefine.
> * ravenscar-thread.h (struct ravenscar_arch_ops): Add virtual
> methods and destructor. Remove members.
> * ravenscar-thread.c (ravenscar_thread_target::fetch_registers)
> (ravenscar_thread_target::store_registers)
> (ravenscar_thread_target::prepare_to_store): Update.
> * ppc-ravenscar-thread.c (ppc_ravenscar_generic_prepare_to_store):
> Remove.
> (struct ppc_ravenscar_powerpc_ops): Derive from
> ravenscar_arch_ops.
> (ppc_ravenscar_powerpc_ops::fetch_registers)
> (ppc_ravenscar_powerpc_ops::store_registers): Now methods.
> (ppc_ravenscar_powerpc_ops): Redefine.
> (struct ppc_ravenscar_e500_ops): Derive from ravenscar_arch_ops.
> (ppc_ravenscar_e500_ops::fetch_registers)
> (ppc_ravenscar_e500_ops::store_registers): Now methods.
> (ppc_ravenscar_e500_ops): Redefine.
> * aarch64-ravenscar-thread.c
> (aarch64_ravenscar_generic_prepare_to_store): Remove.
> (struct aarch64_ravenscar_ops): Derive from ravenscar_arch_ops.
> (aarch64_ravenscar_fetch_registers)
> (aarch64_ravenscar_store_registers): Now methods.
> (aarch64_ravenscar_ops): Redefine.
> ---
> gdb/ChangeLog | 31 ++++++++++++++++
> gdb/aarch64-ravenscar-thread.c | 51 ++++++++++-----------------
> gdb/ppc-ravenscar-thread.c | 64 +++++++++++++++-------------------
> gdb/ravenscar-thread.c | 6 ++--
> gdb/ravenscar-thread.h | 10 ++++--
> gdb/sparc-ravenscar-thread.c | 30 +++++++---------
> 6 files changed, 100 insertions(+), 92 deletions(-)
>
> diff --git a/gdb/aarch64-ravenscar-thread.c b/gdb/aarch64-ravenscar-thread.c
> index 1234650a2b7..fa99d896ffe 100644
> --- a/gdb/aarch64-ravenscar-thread.c
> +++ b/gdb/aarch64-ravenscar-thread.c
> @@ -133,15 +133,6 @@ aarch64_ravenscar_generic_fetch_registers
> }
> }
>
> -/* to_prepare_to_store when inferior_ptid is different from the running
> - thread. */
> -
> -static void
> -aarch64_ravenscar_generic_prepare_to_store (struct regcache *regcache)
> -{
> - /* Nothing to do. */
> -}
> -
> /* to_store_registers when inferior_ptid is different from the running
> thread. */
>
> @@ -175,34 +166,28 @@ static const struct ravenscar_reg_info aarch64_reg_info =
> ARRAY_SIZE (aarch64_context_offsets),
> };
>
> -/* Implement the to_fetch_registers ravenscar_arch_ops method
> - for most Aarch64 targets. */
> -
> -static void
> -aarch64_ravenscar_fetch_registers (struct regcache *regcache, int regnum)
> +struct aarch64_ravenscar_ops : public ravenscar_arch_ops
> {
> - aarch64_ravenscar_generic_fetch_registers
> - (&aarch64_reg_info, regcache, regnum);
> -}
> -
> -/* Implement the to_store_registers ravenscar_arch_ops method
> - for most Aarch64 targets. */
> -
> -static void
> -aarch64_ravenscar_store_registers (struct regcache *regcache, int regnum)
> -{
> - aarch64_ravenscar_generic_store_registers
> - (&aarch64_reg_info, regcache, regnum);
> -}
> + void fetch_registers (struct regcache *regcache, int regnum) override
> + {
> + aarch64_ravenscar_generic_fetch_registers
> + (&aarch64_reg_info, regcache, regnum);
> + }
> +
> + void store_registers (struct regcache *regcache, int regnum) override
> + {
> + aarch64_ravenscar_generic_store_registers
> + (&aarch64_reg_info, regcache, regnum);
> + }
> +
> + void prepare_to_store (struct regcache *) override
> + {
> + }
> +};
>
> /* The ravenscar_arch_ops vector for most Aarch64 targets. */
>
> -static struct ravenscar_arch_ops aarch64_ravenscar_ops =
> -{
> - aarch64_ravenscar_fetch_registers,
> - aarch64_ravenscar_store_registers,
> - aarch64_ravenscar_generic_prepare_to_store
> -};
> +static struct aarch64_ravenscar_ops aarch64_ravenscar_ops;
>
> /* Register aarch64_ravenscar_ops in GDBARCH. */
>
> diff --git a/gdb/ppc-ravenscar-thread.c b/gdb/ppc-ravenscar-thread.c
> index eca80c534a5..919b32c94f4 100644
> --- a/gdb/ppc-ravenscar-thread.c
> +++ b/gdb/ppc-ravenscar-thread.c
> @@ -169,15 +169,6 @@ ppc_ravenscar_generic_fetch_registers
> }
> }
>
> -/* to_prepare_to_store when inferior_ptid is different from the running
> - thread. */
> -
> -static void
> -ppc_ravenscar_generic_prepare_to_store (struct regcache *regcache)
> -{
> - /* Nothing to do. */
> -}
> -
> /* to_store_registers when inferior_ptid is different from the running
> thread. */
>
> @@ -211,32 +202,31 @@ static const struct ravenscar_reg_info ppc_reg_info =
> ARRAY_SIZE (powerpc_context_offsets),
> };
>
> -/* Implement the to_fetch_registers ravenscar_arch_ops method
> - for most PowerPC targets. */
> +struct ppc_ravenscar_powerpc_ops : public ravenscar_arch_ops
> +{
> + void fetch_registers (struct regcache *, int) override;
> + void store_registers (struct regcache *, int) override;
> +
> + void prepare_to_store (struct regcache *) override
> + {
> + }
> +};
>
> -static void
> -ppc_ravenscar_powerpc_fetch_registers (struct regcache *regcache, int regnum)
> +void
> +ppc_ravenscar_powerpc_ops::fetch_registers (struct regcache *regcache, int regnum)
> {
> ppc_ravenscar_generic_fetch_registers (&ppc_reg_info, regcache, regnum);
> }
>
> -/* Implement the to_store_registers ravenscar_arch_ops method
> - for most PowerPC targets. */
> -
> -static void
> -ppc_ravenscar_powerpc_store_registers (struct regcache *regcache, int regnum)
> +void
> +ppc_ravenscar_powerpc_ops::store_registers (struct regcache *regcache, int regnum)
> {
> ppc_ravenscar_generic_store_registers (&ppc_reg_info, regcache, regnum);
> }
>
> /* The ravenscar_arch_ops vector for most PowerPC targets. */
>
> -static struct ravenscar_arch_ops ppc_ravenscar_powerpc_ops =
> -{
> - ppc_ravenscar_powerpc_fetch_registers,
> - ppc_ravenscar_powerpc_store_registers,
> - ppc_ravenscar_generic_prepare_to_store
> -};
> +static struct ppc_ravenscar_powerpc_ops ppc_ravenscar_powerpc_ops;
>
> /* Register ppc_ravenscar_powerpc_ops in GDBARCH. */
>
> @@ -254,11 +244,18 @@ static const struct ravenscar_reg_info e500_reg_info =
> ARRAY_SIZE (e500_context_offsets),
> };
>
> -/* Implement the to_fetch_registers ravenscar_arch_ops method
> - for E500 targets. */
> +struct ppc_ravenscar_e500_ops : public ravenscar_arch_ops
> +{
> + void fetch_registers (struct regcache *, int) override;
> + void store_registers (struct regcache *, int) override;
>
> -static void
> -ppc_ravenscar_e500_fetch_registers (struct regcache *regcache, int regnum)
> + void prepare_to_store (struct regcache *) override
> + {
> + }
> +};
> +
> +void
> +ppc_ravenscar_e500_ops::fetch_registers (struct regcache *regcache, int regnum)
> {
> ppc_ravenscar_generic_fetch_registers (&e500_reg_info, regcache, regnum);
> }
> @@ -266,20 +263,15 @@ ppc_ravenscar_e500_fetch_registers (struct regcache *regcache, int regnum)
> /* Implement the to_store_registers ravenscar_arch_ops method
> for E500 targets. */
>
> -static void
> -ppc_ravenscar_e500_store_registers (struct regcache *regcache, int regnum)
> +void
> +ppc_ravenscar_e500_ops::store_registers (struct regcache *regcache, int regnum)
> {
> ppc_ravenscar_generic_store_registers (&e500_reg_info, regcache, regnum);
> }
>
> /* The ravenscar_arch_ops vector for E500 targets. */
>
> -static struct ravenscar_arch_ops ppc_ravenscar_e500_ops =
> -{
> - ppc_ravenscar_e500_fetch_registers,
> - ppc_ravenscar_e500_store_registers,
> - ppc_ravenscar_generic_prepare_to_store
> -};
> +static struct ppc_ravenscar_e500_ops ppc_ravenscar_e500_ops;
>
> /* Register ppc_ravenscar_e500_ops in GDBARCH. */
>
> diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
> index 9d708fd8581..0dc50a41429 100644
> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -414,7 +414,7 @@ ravenscar_thread_target::fetch_registers (struct regcache *regcache, int regnum)
> struct ravenscar_arch_ops *arch_ops
> = gdbarch_ravenscar_ops (gdbarch);
>
> - arch_ops->to_fetch_registers (regcache, regnum);
> + arch_ops->fetch_registers (regcache, regnum);
> }
> else
> beneath ()->fetch_registers (regcache, regnum);
> @@ -434,7 +434,7 @@ ravenscar_thread_target::store_registers (struct regcache *regcache,
> struct ravenscar_arch_ops *arch_ops
> = gdbarch_ravenscar_ops (gdbarch);
>
> - arch_ops->to_store_registers (regcache, regnum);
> + arch_ops->store_registers (regcache, regnum);
> }
> else
> beneath ()->store_registers (regcache, regnum);
> @@ -453,7 +453,7 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
> struct ravenscar_arch_ops *arch_ops
> = gdbarch_ravenscar_ops (gdbarch);
>
> - arch_ops->to_prepare_to_store (regcache);
> + arch_ops->prepare_to_store (regcache);
> }
> else
> beneath ()->prepare_to_store (regcache);
> diff --git a/gdb/ravenscar-thread.h b/gdb/ravenscar-thread.h
> index 8aab0a124f2..f0c163c5f0b 100644
> --- a/gdb/ravenscar-thread.h
> +++ b/gdb/ravenscar-thread.h
> @@ -24,9 +24,13 @@
>
> struct ravenscar_arch_ops
> {
> - void (*to_fetch_registers) (struct regcache *, int);
> - void (*to_store_registers) (struct regcache *, int);
> - void (*to_prepare_to_store) (struct regcache *);
> + virtual ~ravenscar_arch_ops ()
> + {
> + }
> +
> + virtual void fetch_registers (struct regcache *, int) = 0;
> + virtual void store_registers (struct regcache *, int) = 0;
> + virtual void prepare_to_store (struct regcache *) = 0;
> };
>
> #endif /* !defined (RAVENSCAR_THREAD_H) */
> diff --git a/gdb/sparc-ravenscar-thread.c b/gdb/sparc-ravenscar-thread.c
> index e09e453eabf..4b26f55490c 100644
> --- a/gdb/sparc-ravenscar-thread.c
> +++ b/gdb/sparc-ravenscar-thread.c
> @@ -25,11 +25,12 @@
> #include "ravenscar-thread.h"
> #include "sparc-ravenscar-thread.h"
>
> -static void sparc_ravenscar_fetch_registers (struct regcache *regcache,
> - int regnum);
> -static void sparc_ravenscar_store_registers (struct regcache *regcache,
> - int regnum);
> -static void sparc_ravenscar_prepare_to_store (struct regcache *regcache);
> +struct sparc_ravenscar_ops : public ravenscar_arch_ops
> +{
> + void fetch_registers (struct regcache *, int) override;
> + void store_registers (struct regcache *, int) override;
> + void prepare_to_store (struct regcache *) override;
> +};
>
> /* Register offsets from a referenced address (exempli gratia the
> Thread_Descriptor). The referenced address depends on the register
> @@ -100,8 +101,8 @@ register_in_thread_descriptor_p (int regnum)
> /* to_fetch_registers when inferior_ptid is different from the running
> thread. */
>
> -static void
> -sparc_ravenscar_fetch_registers (struct regcache *regcache, int regnum)
> +void
> +sparc_ravenscar_ops::fetch_registers (struct regcache *regcache, int regnum)
> {
> struct gdbarch *gdbarch = regcache->arch ();
> const int sp_regnum = gdbarch_sp_regnum (gdbarch);
> @@ -143,8 +144,8 @@ sparc_ravenscar_fetch_registers (struct regcache *regcache, int regnum)
> /* to_prepare_to_store when inferior_ptid is different from the running
> thread. */
>
> -static void
> -sparc_ravenscar_prepare_to_store (struct regcache *regcache)
> +void
> +sparc_ravenscar_ops::prepare_to_store (struct regcache *regcache)
> {
> /* Nothing to do. */
> }
> @@ -152,8 +153,8 @@ sparc_ravenscar_prepare_to_store (struct regcache *regcache)
> /* to_store_registers when inferior_ptid is different from the running
> thread. */
>
> -static void
> -sparc_ravenscar_store_registers (struct regcache *regcache, int regnum)
> +void
> +sparc_ravenscar_ops::store_registers (struct regcache *regcache, int regnum)
> {
> struct gdbarch *gdbarch = regcache->arch ();
> int buf_size = register_size (gdbarch, regnum);
> @@ -178,12 +179,7 @@ sparc_ravenscar_store_registers (struct regcache *regcache, int regnum)
> buf_size);
> }
>
> -static struct ravenscar_arch_ops sparc_ravenscar_ops =
> -{
> - sparc_ravenscar_fetch_registers,
> - sparc_ravenscar_store_registers,
> - sparc_ravenscar_prepare_to_store
> -};
> +static struct sparc_ravenscar_ops sparc_ravenscar_ops;
>
> /* Register ravenscar_arch_ops in GDBARCH. */
>
>