From: Luis Machado <luis.machado@arm.com>
To: Thiago Jung Bauermann <thiago.bauermann@linaro.org>,
gdb-patches@sourceware.org
Subject: Re: [PATCH 8/8] gdb/aarch64: When remote debugging detect changes in the target description
Date: Tue, 20 Sep 2022 09:59:09 +0100 [thread overview]
Message-ID: <36d5d17b-de17-71f9-06b8-46a31c450b4c@arm.com> (raw)
In-Reply-To: <20220908064151.3959930-9-thiago.bauermann@linaro.org>
On 9/8/22 07:41, Thiago Jung Bauermann via Gdb-patches wrote:
> If the remote target provides an expedited VG register, use it to update
> its target description. This allows debugging programs that change their
> SVE vector length during runtime.
>
> This is accomplished by implementing the thread_architecture method in
> remote_target, which returns the gdbarch corresponding to the expedited
> registers provided by the last stop reply.
>
> To allow changing the architecture based on the expedited registers, add a
> new gdbarch method to allow arch-specific code to make the adjustment.
> ---
> gdb/aarch64-tdep.c | 28 +++++++++++++++++++++++++-
> gdb/arch-utils.c | 9 +++++++++
> gdb/arch-utils.h | 5 +++++
> gdb/gdbarch-components.py | 16 +++++++++++++++
> gdb/gdbarch-gen.h | 11 ++++++++++
> gdb/gdbarch.c | 23 +++++++++++++++++++++
> gdb/remote.c | 42 +++++++++++++++++++++++++++++++++++++++
> 7 files changed, 133 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 18c2b1ec1523..acfd84ea3f33 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -3420,7 +3420,8 @@ aarch64_cannot_store_register (struct gdbarch *gdbarch, int regnum)
> || regnum == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base));
> }
>
> -/* Helper function for the "thread_architecture" target_ops method.
> +/* Helper function for both the "update_architecture" gdbarch method and the
> + "thread_architecture" target_ops method.
>
> Returns a new gdbarch that is equivalent to the given gdbarch, but with SVE
> registers reflecting the given vq value. */
> @@ -3454,6 +3455,30 @@ aarch64_update_gdbarch (struct gdbarch *gdbarch, uint64_t vq)
> return gdbarch_find_by_info (info);
> }
>
> +/* Implement the "update_architecture" gdbarch method. */
> +
> +static struct gdbarch *
> +aarch64_update_architecture (struct gdbarch *gdbarch,
> + const std::vector<cached_reg_t> ®cache)
> +{
> + gdb_assert (gdbarch != NULL);
> +
> + /* Look for the VG register. */
> + auto it = find_if (regcache.cbegin (), regcache.cend (),
> + [] (const cached_reg_t ®) {
> + return reg.num == AARCH64_SVE_VG_REGNUM;
> + });
> +
> + /* No VG register was provided. Don't change gdbarch. */
> + if (it == regcache.cend ())
> + return gdbarch;
> +
> + ULONGEST vg = extract_unsigned_integer (it->data, 8,
> + gdbarch_byte_order (gdbarch));
> +
> + return aarch64_update_gdbarch (gdbarch, sve_vq_from_vg (vg));
> +}
> +
> /* Implement the stack_frame_destroyed_p gdbarch method. */
>
> static int
> @@ -3667,6 +3692,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> set_tdesc_pseudo_register_reggroup_p (gdbarch,
> aarch64_pseudo_register_reggroup_p);
> set_gdbarch_cannot_store_register (gdbarch, aarch64_cannot_store_register);
> + set_gdbarch_update_architecture (gdbarch, aarch64_update_architecture);
>
> /* ABI */
> set_gdbarch_short_bit (gdbarch, 16);
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 9bd4f0ddae66..1c75d100b55b 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1092,6 +1092,15 @@ default_read_core_file_mappings
> {
> }
>
> +/* See arch-utils.h. */
> +
> +struct gdbarch *
> +default_update_architecture (struct gdbarch *gdbarch,
> + const std::vector<cached_reg_t> ®cache)
> +{
> + return gdbarch;
> +}
> +
> /* Non-zero if we want to trace architecture code. */
>
> #ifndef GDBARCH_DEBUG
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index f850e5fd6e78..f81e0ad78fa3 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -300,4 +300,9 @@ extern void default_read_core_file_mappings
> struct bfd *cbfd,
> read_core_file_mappings_pre_loop_ftype pre_loop_cb,
> read_core_file_mappings_loop_ftype loop_cb);
> +
> +/* Default implementation of gdbarch update_architecture method. */
> +extern struct gdbarch *
> +default_update_architecture (struct gdbarch *gdbarch,
> + const std::vector<cached_reg_t> ®cache);
> #endif /* ARCH_UTILS_H */
> diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
> index 71aa5991fbe0..5a23b2ac48b7 100644
> --- a/gdb/gdbarch-components.py
> +++ b/gdb/gdbarch-components.py
> @@ -2652,3 +2652,19 @@ Read core file mappings
> predefault="default_read_core_file_mappings",
> invalid=False,
> )
> +
> +Method(
> + comment="""
> +An architecture may change based on the current contents of its registers.
> +For instance, the length of the vector registers in AArch64's Scalable Vector
> +Extension is given by the contents of the VL pseudo-register.
> +
> +This method allows an architecture to provide a new gdbarch corresponding to
> +the registers in the given regcache.
> +""",
> + type="struct gdbarch *",
> + name="update_architecture",
> + params=[("const std::vector<cached_reg_t> &", "regcache")],
> + predefault="default_update_architecture",
> + invalid=False,
> +)
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 0504962e50d2..b543a9878930 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -1627,3 +1627,14 @@ extern void set_gdbarch_get_pc_address_flags (struct gdbarch *gdbarch, gdbarch_g
> typedef void (gdbarch_read_core_file_mappings_ftype) (struct gdbarch *gdbarch, struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb);
> extern void gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb);
> extern void set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch, gdbarch_read_core_file_mappings_ftype *read_core_file_mappings);
> +
> +/* An architecture may change based on the current contents of its registers.
> + For instance, the length of the vector registers in AArch64's Scalable Vector
> + Extension is given by the contents of the VL pseudo-register.
> +
> + This method allows an architecture to provide a new gdbarch corresponding to
> + the registers in the given regcache. */
> +
> +typedef struct gdbarch * (gdbarch_update_architecture_ftype) (struct gdbarch *gdbarch, const std::vector<cached_reg_t> ®cache);
> +extern struct gdbarch * gdbarch_update_architecture (struct gdbarch *gdbarch, const std::vector<cached_reg_t> ®cache);
> +extern void set_gdbarch_update_architecture (struct gdbarch *gdbarch, gdbarch_update_architecture_ftype *update_architecture);
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 0edae7f6f0a2..4c9482f8fbf7 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -253,6 +253,7 @@ struct gdbarch
> gdbarch_type_align_ftype *type_align = nullptr;
> gdbarch_get_pc_address_flags_ftype *get_pc_address_flags = nullptr;
> gdbarch_read_core_file_mappings_ftype *read_core_file_mappings = nullptr;
> + gdbarch_update_architecture_ftype *update_architecture = nullptr;
> };
>
> /* Create a new ``struct gdbarch'' based on information provided by
> @@ -368,6 +369,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
> gdbarch->type_align = default_type_align;
> gdbarch->get_pc_address_flags = default_get_pc_address_flags;
> gdbarch->read_core_file_mappings = default_read_core_file_mappings;
> + gdbarch->update_architecture = default_update_architecture;
> /* gdbarch_alloc() */
>
> return gdbarch;
> @@ -605,6 +607,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
> /* Skip verify of type_align, invalid_p == 0 */
> /* Skip verify of get_pc_address_flags, invalid_p == 0 */
> /* Skip verify of read_core_file_mappings, invalid_p == 0 */
> + /* Skip verify of update_architecture, invalid_p == 0 */
> if (!log.empty ())
> internal_error (__FILE__, __LINE__,
> _("verify_gdbarch: the following are invalid ...%s"),
> @@ -1438,6 +1441,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> gdb_printf (file,
> "gdbarch_dump: read_core_file_mappings = <%s>\n",
> host_address_to_string (gdbarch->read_core_file_mappings));
> + gdb_printf (file,
> + "gdbarch_dump: update_architecture = <%s>\n",
> + host_address_to_string (gdbarch->update_architecture));
> if (gdbarch->dump_tdep != NULL)
> gdbarch->dump_tdep (gdbarch, file);
> }
> @@ -5363,3 +5369,20 @@ set_gdbarch_read_core_file_mappings (struct gdbarch *gdbarch,
> {
> gdbarch->read_core_file_mappings = read_core_file_mappings;
> }
> +
> +struct gdbarch *
> +gdbarch_update_architecture (struct gdbarch *gdbarch, const std::vector<cached_reg_t> ®cache)
> +{
> + gdb_assert (gdbarch != NULL);
> + gdb_assert (gdbarch->update_architecture != NULL);
> + if (gdbarch_debug >= 2)
> + gdb_printf (gdb_stdlog, "gdbarch_update_architecture called\n");
> + return gdbarch->update_architecture (gdbarch, regcache);
> +}
> +
> +void
> +set_gdbarch_update_architecture (struct gdbarch *gdbarch,
> + gdbarch_update_architecture_ftype update_architecture)
> +{
> + gdbarch->update_architecture = update_architecture;
> +}
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 70f918a7362c..4e6a33dfb686 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -491,6 +491,8 @@ class remote_target : public process_stratum_target
> gdb::byte_vector thread_info_to_thread_handle (struct thread_info *tp)
> override;
>
> + struct gdbarch *thread_architecture (ptid_t ptid) override;
> +
> void stop (ptid_t) override;
>
> void interrupt () override;
> @@ -1150,6 +1152,10 @@ struct remote_thread_info : public private_thread_info
> to stop for a watchpoint. */
> CORE_ADDR watch_data_address = 0;
>
> + /* The architecture corresponding to the expedited registers returned
> + in the last stop reply. */
> + struct gdbarch *expedited_arch = nullptr;
> +
> /* Get the thread's resume state. */
> enum resume_state get_resume_state () const
> {
> @@ -1168,6 +1174,8 @@ struct remote_thread_info : public private_thread_info
> m_resume_state = resume_state::RESUMED_PENDING_VCONT;
> m_resumed_pending_vcont_info.step = step;
> m_resumed_pending_vcont_info.sig = sig;
> +
> + expedited_arch = nullptr;
> }
>
> /* Get the information this thread's pending vCont-resumption.
> @@ -1185,6 +1193,8 @@ struct remote_thread_info : public private_thread_info
> void set_resumed ()
> {
> m_resume_state = resume_state::RESUMED;
> +
> + expedited_arch = nullptr;
> }
>
> private:
> @@ -8072,6 +8082,21 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply,
> /* Expedited registers. */
> if (!stop_reply->regcache.empty ())
> {
> + /* If GDB already knows about this thread, we can give the
> + architecture-specific code a chance to update the gdbarch based on
> + the expedited registers. */
> + if (find_thread_ptid (this, ptid) != nullptr)
> + {
> + stop_reply->arch = gdbarch_update_architecture (stop_reply->arch,
> + stop_reply->regcache);
> +
> + /* Save stop_reply->arch so that it can be returned by the
> + thread_architecture method. */
> + remote_thread_info *remote_thr = get_remote_thread_info (this,
> + ptid);
> + remote_thr->expedited_arch = stop_reply->arch;
> + }
> +
> struct regcache *regcache
> = get_thread_arch_regcache (this, ptid, stop_reply->arch);
>
> @@ -14448,6 +14473,23 @@ remote_target::thread_info_to_thread_handle (struct thread_info *tp)
> return priv->thread_handle;
> }
>
> +struct gdbarch *
> +remote_target::thread_architecture (ptid_t ptid)
> +{
> + thread_info *thr = find_thread_ptid (this, ptid);
> + remote_thread_info *remote_thr = nullptr;
> +
> + if (thr != nullptr)
> + remote_thr = get_remote_thread_info (thr);
> +
> + if (remote_thr == nullptr || remote_thr->expedited_arch == nullptr)
> + /* The default thread_architecture implementation is the one from
> + process_stratum_target. */
> + return process_stratum_target::thread_architecture(ptid);
> +
> + return remote_thr->expedited_arch;
> +}
> +
> bool
> remote_target::can_async_p ()
> {
LGTM for the aarch64 code. Someone else must approve the generic parts.
next prev parent reply other threads:[~2022-09-20 8:59 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-08 6:41 [PATCH 0/8] gdbserver improvements for AArch64 SVE support Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 1/8] gdbserver: Add asserts in register_size and register_data functions Thiago Jung Bauermann
2022-09-20 7:36 ` Luis Machado
2022-11-26 1:36 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 2/8] gdbserver: Add thread parameter to linux_get_auxv and linux_get_hwcap Thiago Jung Bauermann
2022-09-20 7:43 ` Luis Machado
2022-11-26 1:37 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 3/8] gdb, gdbserver/aarch64-linux: Allow aarch64_sve_get_vq to return error Thiago Jung Bauermann
2022-09-20 8:07 ` Luis Machado
2022-11-26 1:42 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 4/8] gdbserver/linux-aarch64: Factor out function to get aarch64_features Thiago Jung Bauermann
2022-09-20 8:08 ` Luis Machado
2022-11-26 1:44 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 5/8] gdbserver: Move target description from being per-process to being per-thread Thiago Jung Bauermann
2022-09-20 11:21 ` Luis Machado
2022-11-26 1:47 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 6/8] gdbserver/linux-aarch64: When inferior stops, update its target description Thiago Jung Bauermann
2022-09-20 8:48 ` Luis Machado
2022-11-26 1:49 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 7/8] gdb/aarch64: Factor out most of the thread_architecture method Thiago Jung Bauermann
2022-09-20 8:52 ` Luis Machado
2022-11-26 1:49 ` Thiago Jung Bauermann
2022-09-08 6:41 ` [PATCH 8/8] gdb/aarch64: When remote debugging detect changes in the target description Thiago Jung Bauermann
2022-09-20 8:59 ` Luis Machado [this message]
2022-11-26 1:50 ` Thiago Jung Bauermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=36d5d17b-de17-71f9-06b8-46a31c450b4c@arm.com \
--to=luis.machado@arm.com \
--cc=gdb-patches@sourceware.org \
--cc=thiago.bauermann@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).