public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Thiago Jung Bauermann <thiago.bauermann@linaro.org>,
	gdb-patches@sourceware.org
Cc: Luis Machado <luis.machado@arm.com>
Subject: Re: [PATCH v2 6/6] gdb/aarch64: Detect vector length changes when debugging remotely
Date: Mon, 28 Nov 2022 11:36:42 -0500	[thread overview]
Message-ID: <5d3ca3ed-df52-444e-1652-99b149137707@simark.ca> (raw)
In-Reply-To: <20221126020452.1686509-7-thiago.bauermann@linaro.org>



On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote:
> If the remote target provides an expedited VG register, use it to update
> the thread's gdbarch. 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.

I get this when applying, can you address that?

Applying: gdb/aarch64: Detect vector length changes when debugging remotely
.git/rebase-apply/patch:164: indent with spaces.
                       "gdbarch_dump: update_architecture = <%s>\n",
.git/rebase-apply/patch:165: indent with spaces.
                       host_address_to_string (gdbarch->update_architecture));
.git/rebase-apply/patch:186: indent with spaces.
                                  gdbarch_update_architecture_ftype update_architecture)
warning: 3 lines add whitespace errors.

I think the idea makes sense.  Short of adding a packet for "get thread
architecture" or extending the stop reply format to add a note about a
thread having a special tdesc, I don't see another way to do this.  We
can't read the vq register using 'g', because to interpret the 'g'
result, we need to know the full architecture.  We wouldn't know for
sure the offset of vq in the reply.  By using expedited registers, we
just need to make sure vq's register number is stable.  I guess that is
the case?


> @@ -8068,6 +8078,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);


What happens with threads that are not yet known?  Imagine the process
starts with SVE == X, the main threads spawns a worker thread, the
worker thread updates its SVE to Y, and the worker thread hits a
breakpoint.  This is the first time we hear about that worker thread.
If we don't do a "gdbarch_update_architecture" for it and save it
somewhere, we'll never set the gdbarch with SVE == Y, will we?

Simon

  parent reply	other threads:[~2022-11-28 16:36 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-26  2:04 [PATCH v2 0/6] gdbserver improvements for AArch64 SVE support Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 1/6] gdbserver: Add asserts in register_size and register_data functions Thiago Jung Bauermann
2022-11-28 11:51   ` Luis Machado
2022-11-29  2:53     ` Thiago Jung Bauermann
2022-11-28 14:48   ` Simon Marchi
2022-11-28 14:53     ` Simon Marchi
2022-11-29  2:52       ` Thiago Jung Bauermann
2022-11-29  2:43     ` Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 2/6] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap Thiago Jung Bauermann
2022-11-28 11:50   ` Luis Machado
2022-11-28 15:01     ` Simon Marchi
2022-11-29  3:10       ` Thiago Jung Bauermann
2022-11-28 15:07   ` Simon Marchi
2022-11-28 15:20     ` Luis Machado
2022-11-29  3:17     ` Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 3/6] gdbserver/linux-aarch64: Factor out function to get aarch64_features Thiago Jung Bauermann
2022-11-28 11:54   ` Luis Machado
2022-11-29  3:19     ` Thiago Jung Bauermann
2022-11-28 15:12   ` Simon Marchi
2022-11-29  3:26     ` Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 4/6] gdbserver/linux-aarch64: When thread stops, update its target description Thiago Jung Bauermann
2022-11-28 12:06   ` Luis Machado
2022-11-29  3:59     ` Thiago Jung Bauermann
2022-11-28 15:47   ` Simon Marchi
2022-11-28 16:01     ` Luis Machado
2022-11-28 16:07       ` Simon Marchi
2022-11-29  4:30     ` Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 5/6] gdb/aarch64: Factor out most of the thread_architecture method Thiago Jung Bauermann
2022-11-28 12:09   ` Luis Machado
2022-11-29  4:32     ` Thiago Jung Bauermann
2022-11-28 16:09   ` Simon Marchi
2022-11-29  4:33     ` Thiago Jung Bauermann
2022-11-26  2:04 ` [PATCH v2 6/6] gdb/aarch64: Detect vector length changes when debugging remotely Thiago Jung Bauermann
2022-11-28 13:27   ` Luis Machado
2022-12-01  1:15     ` Thiago Jung Bauermann
2022-11-28 16:36   ` Simon Marchi [this message]
2022-12-01  3:16     ` Thiago Jung Bauermann
2022-12-01  8:32       ` Luis Machado
2022-12-01 16:16       ` Simon Marchi
2022-11-30  8:43   ` Luis Machado
2022-12-05 22:37     ` Thiago Jung Bauermann
2022-12-07 17:05       ` Luis Machado
2022-12-07 19:25         ` Simon Marchi
2022-12-07 23:01           ` Luis Machado
2022-12-09  2:20             ` 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=5d3ca3ed-df52-444e-1652-99b149137707@simark.ca \
    --to=simark@simark.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@arm.com \
    --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).