From: Andrew Burgess <aburgess@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame
Date: Tue, 14 Nov 2023 12:20:19 +0000 [thread overview]
Message-ID: <87o7fwr0fw.fsf@redhat.com> (raw)
In-Reply-To: <20231108051222.1275306-18-simon.marchi@polymtl.ca>
Simon Marchi <simon.marchi@polymtl.ca> writes:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Add a new variant of gdbarch_pseudo_register_write that takes a
> frame_info in order to write raw registers. Use this new method when
> available:
>
> - in put_frame_register, when trying to write a pseudo register to a given frame
> - in regcache::cooked_write
>
> No implementation is migrated to use this new method (that will come in
> subsequent patches), so no behavior change is expected here.
>
> The objective is to fix writing pseudo registers to non-current
> frames. See previous commit "gdb: read pseudo register through
> frame" for a more detailed explanation.
>
> Change-Id: Ie7fe364a15a4d86c2ecb09de2b4baa08c45555ac
> ---
> gdb/frame.c | 9 +++++-
> gdb/gdbarch-gen.h | 15 +++++++++-
> gdb/gdbarch.c | 32 ++++++++++++++++++++
> gdb/gdbarch_components.py | 19 ++++++++++++
> gdb/regcache.c | 4 +++
> gdb/value.c | 63 +++++++++++++++++++++++++++++++++++++++
> gdb/value.h | 22 ++++++++++++++
> 7 files changed, 162 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 3035f87369ca..81836d6d5357 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1452,7 +1452,14 @@ put_frame_register (frame_info_ptr next_frame, int regnum,
> break;
> }
> case lval_register:
> - get_current_regcache ()->cooked_write (realnum, buf, 1.0f);
> + /* Not sure if that's always true... but we have a problem if not. */
> + gdb_assert (size == register_size (gdbarch, realnum));
> +
> + if (realnum < gdbarch_num_regs (gdbarch)
> + || !gdbarch_pseudo_register_write_p (gdbarch))
> + get_current_regcache ()->cooked_write (realnum, buf, 1.0f);
> + else
> + gdbarch_pseudo_register_write (gdbarch, next_frame, realnum, buf);
> break;
> default:
> error (_("Attempt to assign to an unmodifiable value."));
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 3160aa8a9613..9cf58490d0b6 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -200,12 +200,25 @@ typedef struct value * (gdbarch_pseudo_register_read_value_ftype) (struct gdbarc
> extern struct value * gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, frame_info_ptr next_frame, int cookednum);
> extern void set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch, gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value);
>
> +/* Write bytes in BUF to pseudo register with number PSEUDO_REG_NUM.
> +
> + Raw registers backing the pseudo register should be written to using
> + NEXT_FRAME. */
> +
> +extern bool gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch);
> +
> +typedef void (gdbarch_pseudo_register_write_ftype) (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view<const gdb_byte> buf);
> +extern void gdbarch_pseudo_register_write (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view<const gdb_byte> buf);
> +extern void set_gdbarch_pseudo_register_write (struct gdbarch *gdbarch, gdbarch_pseudo_register_write_ftype *pseudo_register_write);
> +
> /* Write bytes to a pseudo register.
>
> This is marked as deprecated because it gets passed a regcache for
> implementations to write raw registers in. This doesn't work for unwound
> frames, where the raw registers backing the pseudo registers may have been
> - saved elsewhere. */
> + saved elsewhere.
> +
> + Implementations should be migrated to implement pseudo_register_write instead. */
>
> extern bool gdbarch_deprecated_pseudo_register_write_p (struct gdbarch *gdbarch);
>
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index e198d339f6ba..d584305fefb2 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -74,6 +74,7 @@ struct gdbarch
> gdbarch_virtual_frame_pointer_ftype *virtual_frame_pointer = legacy_virtual_frame_pointer;
> gdbarch_pseudo_register_read_ftype *pseudo_register_read = nullptr;
> gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value = nullptr;
> + gdbarch_pseudo_register_write_ftype *pseudo_register_write = nullptr;
> gdbarch_deprecated_pseudo_register_write_ftype *deprecated_pseudo_register_write = nullptr;
> int num_regs = -1;
> int num_pseudo_regs = 0;
> @@ -330,6 +331,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
> /* Skip verify of virtual_frame_pointer, invalid_p == 0 */
> /* Skip verify of pseudo_register_read, has predicate. */
> /* Skip verify of pseudo_register_read_value, has predicate. */
> + /* Skip verify of pseudo_register_write, has predicate. */
> /* Skip verify of deprecated_pseudo_register_write, has predicate. */
> if (gdbarch->num_regs == -1)
> log.puts ("\n\tnum_regs");
> @@ -649,6 +651,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
> gdb_printf (file,
> "gdbarch_dump: pseudo_register_read_value = <%s>\n",
> host_address_to_string (gdbarch->pseudo_register_read_value));
> + gdb_printf (file,
> + "gdbarch_dump: gdbarch_pseudo_register_write_p() = %d\n",
> + gdbarch_pseudo_register_write_p (gdbarch));
> + gdb_printf (file,
> + "gdbarch_dump: pseudo_register_write = <%s>\n",
> + host_address_to_string (gdbarch->pseudo_register_write));
> gdb_printf (file,
> "gdbarch_dump: gdbarch_deprecated_pseudo_register_write_p() = %d\n",
> gdbarch_deprecated_pseudo_register_write_p (gdbarch));
> @@ -1902,6 +1910,30 @@ set_gdbarch_pseudo_register_read_value (struct gdbarch *gdbarch,
> gdbarch->pseudo_register_read_value = pseudo_register_read_value;
> }
>
> +bool
> +gdbarch_pseudo_register_write_p (struct gdbarch *gdbarch)
> +{
> + gdb_assert (gdbarch != NULL);
> + return gdbarch->pseudo_register_write != NULL;
> +}
> +
> +void
> +gdbarch_pseudo_register_write (struct gdbarch *gdbarch, frame_info_ptr next_frame, int pseudo_reg_num, gdb::array_view<const gdb_byte> buf)
> +{
> + gdb_assert (gdbarch != NULL);
> + gdb_assert (gdbarch->pseudo_register_write != NULL);
> + if (gdbarch_debug >= 2)
> + gdb_printf (gdb_stdlog, "gdbarch_pseudo_register_write called\n");
> + gdbarch->pseudo_register_write (gdbarch, next_frame, pseudo_reg_num, buf);
> +}
> +
> +void
> +set_gdbarch_pseudo_register_write (struct gdbarch *gdbarch,
> + gdbarch_pseudo_register_write_ftype pseudo_register_write)
> +{
> + gdbarch->pseudo_register_write = pseudo_register_write;
> +}
> +
> bool
> gdbarch_deprecated_pseudo_register_write_p (struct gdbarch *gdbarch)
> {
> diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
> index 1100da160550..ce8169b19a57 100644
> --- a/gdb/gdbarch_components.py
> +++ b/gdb/gdbarch_components.py
> @@ -418,6 +418,23 @@ never be called.
> predicate=True,
> )
>
> +Method(
> + comment="""
> +Write bytes in BUF to pseudo register with number PSEUDO_REG_NUM.
> +
> +Raw registers backing the pseudo register should be written to using
> +NEXT_FRAME.
> +""",
> + type="void",
> + name="pseudo_register_write",
> + params=[
> + ("frame_info_ptr", "next_frame"),
> + ("int", "pseudo_reg_num"),
> + ("gdb::array_view<const gdb_byte>", "buf"),
> + ],
> + predicate=True,
> +)
> +
> Method(
> comment="""
> Write bytes to a pseudo register.
> @@ -426,6 +443,8 @@ This is marked as deprecated because it gets passed a regcache for
> implementations to write raw registers in. This doesn't work for unwound
> frames, where the raw registers backing the pseudo registers may have been
> saved elsewhere.
> +
> +Implementations should be migrated to implement pseudo_register_write instead.
> """,
> type="void",
> name="deprecated_pseudo_register_write",
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index dadc949434ee..16b117e767be 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -929,6 +929,10 @@ regcache::cooked_write (int regnum, gdb::array_view<const gdb_byte> src, float)
>
> if (regnum < num_raw_registers ())
> raw_write (regnum, src, 1.0f);
> + else if (gdbarch_pseudo_register_write_p (m_descr->gdbarch))
> + gdbarch_pseudo_register_write
> + (m_descr->gdbarch, get_next_frame_sentinel_okay (get_current_frame ()),
> + regnum, src);
> else
> gdbarch_deprecated_pseudo_register_write (m_descr->gdbarch, this, regnum,
> src.data ());
> diff --git a/gdb/value.c b/gdb/value.c
> index 27a79dc62fc1..51dca972a587 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -4057,6 +4057,22 @@ pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num,
>
> /* See value.h. */
>
> +void
> +pseudo_to_raw_part (frame_info_ptr next_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_num, int raw_offset)
> +{
> + int raw_reg_size = register_size (get_frame_arch (next_frame), raw_reg_num);
> +
> + /* When overflowing a register, put_frame_register_bytes writes to the
> + subsequent registers. We don't want that behavior here, so make sure
> + the write is wholly within register RAW_REG_NUM. */
> + gdb_assert (raw_offset + pseudo_buf.size () <= raw_reg_size);
> + put_frame_register_bytes (next_frame, raw_reg_num, raw_offset, pseudo_buf);
> +}
> +
> +/* See value.h. */
> +
> value *
> pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
> int raw_reg_1_num, int raw_reg_2_num)
> @@ -4080,6 +4096,27 @@ pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
> return pseudo_reg_val;
> }
>
> +void
> +pseudo_to_concat_raw (frame_info_ptr next_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_1_num, int raw_reg_2_num)
This function, and its overload are missing a /* See value.h. */ comment.
> +{
> + int src_offset = 0;
> + gdbarch *arch = frame_unwind_arch (next_frame);
> +
> + int raw_reg_1_size = register_size (arch, raw_reg_1_num);
> + put_frame_register_bytes (next_frame, raw_reg_1_num, 0,
> + pseudo_buf.slice (src_offset, raw_reg_1_size));
> + src_offset += raw_reg_1_size;
> +
> + int raw_reg_2_size = register_size (arch, raw_reg_2_num);
> + put_frame_register_bytes (next_frame, raw_reg_2_num, 0,
> + pseudo_buf.slice (src_offset, raw_reg_2_size));
> + src_offset += raw_reg_2_size;
> +
> + gdb_assert (src_offset == pseudo_buf.size ());
> +}
> +
> /* See value.h. */
>
> value *
> @@ -4111,6 +4148,32 @@ pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
> return pseudo_reg_val;
> }
>
> +void
> +pseudo_to_concat_raw (frame_info_ptr next_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_1_num, int raw_reg_2_num, int raw_reg_3_num)
> +{
> + int src_offset = 0;
> + gdbarch *arch = frame_unwind_arch (next_frame);
> +
> + int raw_reg_1_size = register_size (arch, raw_reg_1_num);
> + put_frame_register_bytes (next_frame, raw_reg_1_num, 0,
> + pseudo_buf.slice (src_offset, raw_reg_1_size));
> + src_offset += raw_reg_1_size;
> +
> + int raw_reg_2_size = register_size (arch, raw_reg_2_num);
> + put_frame_register_bytes (next_frame, raw_reg_2_num, 0,
> + pseudo_buf.slice (src_offset, raw_reg_2_size));
> + src_offset += raw_reg_2_size;
> +
> + int raw_reg_3_size = register_size (arch, raw_reg_3_num);
> + put_frame_register_bytes (next_frame, raw_reg_3_num, 0,
> + pseudo_buf.slice (src_offset, raw_reg_3_size));
> + src_offset += raw_reg_3_size;
> +
> + gdb_assert (src_offset == pseudo_buf.size ());
> +}
> +
> /* Implementation of the convenience function $_isvoid. */
>
> static struct value *
> diff --git a/gdb/value.h b/gdb/value.h
> index b5c097ad58bf..6a74d4e2c2ee 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -1661,6 +1661,13 @@ struct scoped_array_length_limiting
> value *pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num,
> int raw_reg_num, int raw_offset);
>
> +/* Write PSEUDO_BUF, the contents of a pseudo register, to part of raw register
> + RAW_REG_NUM starting at RAW_OFFSET. */
> +
> +void pseudo_to_raw_part (frame_info_ptr this_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_num, int raw_offset);
> +
> /* Create a value for pseudo register PSEUDO_REG_NUM by concatenating raw
> registers RAW_REG_1_NUM and RAW_REG_2_NUM.
>
> @@ -1670,10 +1677,25 @@ value *pseudo_from_raw_part (frame_info_ptr next_frame, int pseudo_reg_num,
> value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
> int raw_reg_1_num, int raw_reg_2_num);
>
> +/* Write PSEUDO_BUF, the contents of a pseudo register, to the two raw registers
> + RAW_REG_1_NUM and RAW_REG_2_NUM. */
> +
> +void pseudo_to_concat_raw (frame_info_ptr this_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_1_num, int raw_reg_2_num);
> +
> /* Same as the above, but with three raw registers. */
>
> value *pseudo_from_concat_raw (frame_info_ptr next_frame, int pseudo_reg_num,
> int raw_reg_1_num, int raw_reg_2_num,
> int raw_reg_3_num);
>
> +/* Write PSEUDO_BUF, the contents of a pseudo register, to the tthreewo raw
> + registers RAW_REG_1_NUM, RAW_REG_2_NUM and RAW_REG_3_NUM. */
typo s/tthreewo/three/.
Thanks,
Andrew
> +
> +void pseudo_to_concat_raw (frame_info_ptr this_frame,
> + gdb::array_view<const gdb_byte> pseudo_buf,
> + int raw_reg_1_num, int raw_reg_2_num,
> + int raw_reg_3_num);
> +
> #endif /* !defined (VALUE_H) */
> --
> 2.42.1
next prev parent reply other threads:[~2023-11-14 12:20 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-08 5:00 [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-08 5:00 ` [PATCH 01/24] gdb: don't handle i386 k registers as pseudo registers Simon Marchi
2023-11-11 19:29 ` John Baldwin
2023-11-08 5:00 ` [PATCH 02/24] gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h Simon Marchi
2023-11-11 19:42 ` John Baldwin
2023-11-08 5:00 ` [PATCH 03/24] gdb: make store_integer take an array_view Simon Marchi
2023-11-08 5:00 ` [PATCH 04/24] gdb: simplify conditions in regcache::{read,write,raw_collect,raw_supply}_part Simon Marchi
2023-11-08 5:00 ` [PATCH 05/24] gdb: change regcache interface to use array_view Simon Marchi
2023-11-13 13:43 ` Andrew Burgess
2023-11-13 14:00 ` Andrew Burgess
2023-11-13 16:47 ` Simon Marchi
2023-11-08 5:00 ` [PATCH 06/24] gdb: fix bugs in {get,put}_frame_register_bytes Simon Marchi
2023-11-13 15:00 ` Andrew Burgess
2023-11-13 19:51 ` Simon Marchi
2023-11-08 5:00 ` [PATCH 07/24] gdb: make put_frame_register take an array_view Simon Marchi
2023-11-08 5:00 ` [PATCH 08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame Simon Marchi
2023-11-08 5:00 ` [PATCH 09/24] gdb: remove frame_register Simon Marchi
2023-11-08 5:00 ` [PATCH 10/24] gdb: make put_frame_register take the next frame Simon Marchi
2023-11-08 5:00 ` [PATCH 11/24] gdb: make put_frame_register_bytes " Simon Marchi
2023-11-08 5:00 ` [PATCH 12/24] gdb: make get_frame_register_bytes " Simon Marchi
2023-11-08 5:00 ` [PATCH 13/24] gdb: add value::allocate_register Simon Marchi
2023-11-08 5:00 ` [PATCH 14/24] gdb: read pseudo register through frame Simon Marchi
2023-11-11 20:11 ` John Baldwin
2023-11-08 5:00 ` [PATCH 15/24] gdb: change parameter name in frame_unwind_register_unsigned declaration Simon Marchi
2023-11-08 5:01 ` [PATCH 16/24] gdb: rename gdbarch_pseudo_register_write to gdbarch_deprecated_pseudo_register_write Simon Marchi
2023-11-14 12:12 ` Andrew Burgess
2023-11-14 15:16 ` Simon Marchi
2023-11-08 5:01 ` [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame Simon Marchi
2023-11-14 12:20 ` Andrew Burgess [this message]
2023-11-14 15:20 ` Simon Marchi
2023-11-08 5:01 ` [PATCH 18/24] gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write Simon Marchi
2023-11-11 20:16 ` John Baldwin
2023-11-13 2:59 ` Simon Marchi
2023-11-08 5:01 ` [PATCH 19/24] gdb: make aarch64_za_offsets_from_regnum return za_offsets Simon Marchi
2023-11-08 5:01 ` [PATCH 20/24] gdb: add missing raw register read in aarch64_sme_pseudo_register_write Simon Marchi
2023-11-08 5:01 ` [PATCH 21/24] gdb: migrate aarch64 to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08 5:01 ` [PATCH 22/24] gdb: migrate arm to gdbarch_pseudo_register_read_value Simon Marchi
2023-11-08 5:01 ` [PATCH 23/24] gdb: migrate arm to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08 5:01 ` [PATCH 24/24] gdb/testsuite: add tests for unwinding of pseudo registers Simon Marchi
2023-11-08 5:16 ` [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-09 3:05 ` Simon Marchi
2023-11-08 11:57 ` Luis Machado
2023-11-08 15:47 ` Simon Marchi
2023-11-08 17:08 ` Luis Machado
2023-11-08 19:34 ` Simon Marchi
2023-11-09 19:04 ` Simon Marchi
2023-11-13 13:10 ` Luis Machado
2023-11-13 15:08 ` Luis Machado
2023-11-11 20:26 ` John Baldwin
2023-11-13 3:03 ` Simon Marchi
2023-12-01 16:27 Simon Marchi
2023-12-01 16:27 ` [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame Simon Marchi
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=87o7fwr0fw.fsf@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.com \
--cc=simon.marchi@polymtl.ca \
/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).