From: Luis Machado <luis.machado@arm.com>
To: gdb-patches@sourceware.org
Subject: Re: [PATCH] [AArch64] Handle W registers as pseudo-registers instead of aliases of X registers
Date: Mon, 3 Oct 2022 14:16:40 +0100 [thread overview]
Message-ID: <6ac7bc4f-f43e-890b-1bd3-dd8478d32974@arm.com> (raw)
In-Reply-To: <20220922165256.57290-1-luis.machado@arm.com>
On 9/22/22 17:52, Luis Machado via Gdb-patches wrote:
> The aarch64 port handles W registers as aliases of X registers. This is
> incorrect because X registers are 64-bit and W registers are 32-bit.
>
> This patch teaches GDB how to handle W registers as pseudo-registers of
> 32-bit, the bottom half of the X registers.
>
> Testcase included.
> ---
> gdb/aarch64-tdep.c | 120 +++++++++++++-----
> gdb/aarch64-tdep.h | 4 +
> gdb/testsuite/gdb.arch/aarch64-w-registers.c | 22 ++++
> .../gdb.arch/aarch64-w-registers.exp | 100 +++++++++++++++
> 4 files changed, 212 insertions(+), 34 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-w-registers.c
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-w-registers.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index f94e810f904..2ad31df2738 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -72,40 +72,6 @@ static const struct
> {"fp", AARCH64_FP_REGNUM},
> {"lr", AARCH64_LR_REGNUM},
> {"sp", AARCH64_SP_REGNUM},
> -
> - /* 32-bit register names. */
> - {"w0", AARCH64_X0_REGNUM + 0},
> - {"w1", AARCH64_X0_REGNUM + 1},
> - {"w2", AARCH64_X0_REGNUM + 2},
> - {"w3", AARCH64_X0_REGNUM + 3},
> - {"w4", AARCH64_X0_REGNUM + 4},
> - {"w5", AARCH64_X0_REGNUM + 5},
> - {"w6", AARCH64_X0_REGNUM + 6},
> - {"w7", AARCH64_X0_REGNUM + 7},
> - {"w8", AARCH64_X0_REGNUM + 8},
> - {"w9", AARCH64_X0_REGNUM + 9},
> - {"w10", AARCH64_X0_REGNUM + 10},
> - {"w11", AARCH64_X0_REGNUM + 11},
> - {"w12", AARCH64_X0_REGNUM + 12},
> - {"w13", AARCH64_X0_REGNUM + 13},
> - {"w14", AARCH64_X0_REGNUM + 14},
> - {"w15", AARCH64_X0_REGNUM + 15},
> - {"w16", AARCH64_X0_REGNUM + 16},
> - {"w17", AARCH64_X0_REGNUM + 17},
> - {"w18", AARCH64_X0_REGNUM + 18},
> - {"w19", AARCH64_X0_REGNUM + 19},
> - {"w20", AARCH64_X0_REGNUM + 20},
> - {"w21", AARCH64_X0_REGNUM + 21},
> - {"w22", AARCH64_X0_REGNUM + 22},
> - {"w23", AARCH64_X0_REGNUM + 23},
> - {"w24", AARCH64_X0_REGNUM + 24},
> - {"w25", AARCH64_X0_REGNUM + 25},
> - {"w26", AARCH64_X0_REGNUM + 26},
> - {"w27", AARCH64_X0_REGNUM + 27},
> - {"w28", AARCH64_X0_REGNUM + 28},
> - {"w29", AARCH64_X0_REGNUM + 29},
> - {"w30", AARCH64_X0_REGNUM + 30},
> -
> /* specials */
> {"ip0", AARCH64_X0_REGNUM + 16},
> {"ip1", AARCH64_X0_REGNUM + 17}
> @@ -2556,6 +2522,21 @@ aarch64_gen_return_address (struct gdbarch *gdbarch,
> }
> \f
>
> +/* Return TRUE if REGNUM is a W pseudo-register number. Return FALSE
> + otherwise. */
> +
> +static bool
> +is_w_pseudo_register (struct gdbarch *gdbarch, int regnum)
> +{
> + aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +
> + if (tdep->w_pseudo_base <= regnum
> + && regnum < tdep->w_pseudo_base + tdep->w_pseudo_count)
> + return true;
> +
> + return false;
> +}
> +
> /* Return the pseudo register name corresponding to register regnum. */
>
> static const char *
> @@ -2563,6 +2544,19 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
> {
> aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
>
> + /* W pseudo-registers. Bottom halves of the X registers. */
> + static const char *const w_name[] =
> + {
> + "w0", "w1", "w2", "w3",
> + "w4", "w5", "w6", "w7",
> + "w8", "w9", "w10", "w11",
> + "w12", "w13", "w14", "w15",
> + "w16", "w17", "w18", "w19",
> + "w20", "w21", "w22", "w23",
> + "w24", "w25", "w26", "w27",
> + "w28", "w29", "w30",
> + };
> +
> static const char *const q_name[] =
> {
> "q0", "q1", "q2", "q3",
> @@ -2640,6 +2634,10 @@ aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
> if (p_regnum >= AARCH64_B0_REGNUM && p_regnum < AARCH64_B0_REGNUM + 32)
> return b_name[p_regnum - AARCH64_B0_REGNUM];
>
> + /* W pseudo-registers? */
> + if (is_w_pseudo_register (gdbarch, regnum))
> + return w_name[regnum - tdep->w_pseudo_base];
> +
> if (tdep->has_sve ())
> {
> static const char *const sve_v_name[] =
> @@ -2698,6 +2696,10 @@ aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
> && p_regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM)
> return aarch64_vnv_type (gdbarch);
>
> + /* W pseudo-registers are 32-bit. */
> + if (is_w_pseudo_register (gdbarch, regnum))
> + return builtin_type (gdbarch)->builtin_uint32;
> +
> if (tdep->has_pauth () && regnum == tdep->ra_sign_state_regnum)
> return builtin_type (gdbarch)->builtin_uint64;
>
> @@ -2772,6 +2774,28 @@ aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
> VALUE_LVAL (result_value) = lval_register;
> VALUE_REGNUM (result_value) = regnum;
>
> + if (is_w_pseudo_register (gdbarch, regnum))
> + {
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + /* Default offset for little endian. */
> + int offset = 0;
> +
> + if (byte_order == BFD_ENDIAN_BIG)
> + offset = 4;
> +
> + /* Find the correct X register to extract the data from. */
> + int x_regnum = AARCH64_X0_REGNUM + (regnum - tdep->w_pseudo_base);
> + gdb_byte data[4];
> +
> + /* Read the bottom 4 bytes of X. */
> + if (regcache->raw_read_part (x_regnum, offset, 4, data) != REG_VALID)
> + mark_value_bytes_unavailable (result_value, 0, 4);
> + else
> + memcpy (value_contents_raw (result_value).data (), data, 4);
> +
> + return result_value;
> + }
> +
> regnum -= gdbarch_num_regs (gdbarch);
>
> if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
> @@ -2837,6 +2861,27 @@ aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
> int regnum, const gdb_byte *buf)
> {
> aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +
> + if (is_w_pseudo_register (gdbarch, regnum))
> + {
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> + /* Default offset for little endian. */
> + int offset = 0;
> +
> + if (byte_order == BFD_ENDIAN_BIG)
> + offset = 4;
> +
> + /* Find the correct X register to extract the data from. */
> + int x_regnum = AARCH64_X0_REGNUM + (regnum - tdep->w_pseudo_base);
> +
> + /* First zero-out the contents of X. */
> + ULONGEST zero = 0;
> + regcache->raw_write (x_regnum, zero);
> + /* Write to the bottom 4 bytes of X. */
> + regcache->raw_write_part (x_regnum, offset, 4, buf);
> + return;
> + }
> +
> regnum -= gdbarch_num_regs (gdbarch);
>
> if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
> @@ -3582,6 +3627,9 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>
> num_regs += i;
> }
> + /* W pseudo-registers */
> + int first_w_regnum = num_pseudo_regs;
> + num_pseudo_regs += 31;
>
> if (!valid_p)
> return nullptr;
> @@ -3705,6 +3753,10 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> /* With the number of real registers updated, setup the pseudo-registers and
> record their numbers. */
>
> + /* Setup W pseudo-register numbers. */
> + tdep->w_pseudo_base = first_w_regnum + num_regs;
> + tdep->w_pseudo_count = 31;
> +
> /* Pointer authentication pseudo-registers. */
> if (tdep->has_pauth ())
> tdep->ra_sign_state_regnum = ra_sign_state_offset + num_regs;
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index d8513023c37..55ccf2e777d 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -118,6 +118,10 @@ struct aarch64_gdbarch_tdep : gdbarch_tdep_base
> {
> return tls_regnum != -1;
> }
> +
> + /* The W pseudo-registers. */
> + int w_pseudo_base = 0;
> + int w_pseudo_count = 0;
> };
>
> const target_desc *aarch64_read_description (const aarch64_features &features);
> diff --git a/gdb/testsuite/gdb.arch/aarch64-w-registers.c b/gdb/testsuite/gdb.arch/aarch64-w-registers.c
> new file mode 100644
> index 00000000000..7d12a20c520
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-w-registers.c
> @@ -0,0 +1,22 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2022 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +int
> +main ()
> +{
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.arch/aarch64-w-registers.exp b/gdb/testsuite/gdb.arch/aarch64-w-registers.exp
> new file mode 100644
> index 00000000000..72711fe660f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-w-registers.exp
> @@ -0,0 +1,100 @@
> +# Copyright (C) 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check if the W registers have the expected size and if setting/fetching
> +# values from W registers works correctly for both big and little endian.
> +
> +if {![is_aarch64_target]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +standard_testfile
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile {nodebug}]} {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +array set w_values {
> + 0 0x0
> + 1 0x10
> + 2 0x2010
> + 3 0x302010
> + 4 0x40302010
> + 5 0x40302010
> + 6 0x40302010
> + 7 0x40302010
> + 8 0x40302010
> +}
> +
> +array set x_values {
> + 0 0x0
> + 1 0x10
> + 2 0x2010
> + 3 0x302010
> + 4 0x40302010
> + 5 0x5040302010
> + 6 0x605040302010
> + 7 0x70605040302010
> + 8 0x8070605040302010
> +}
> +
> +# Exercise various things for register w<rn>
> +
> +proc test_register { rn } {
> + gdb_test "ptype \$w${rn}" "type = uint32_t"
> + gdb_test "p sizeof(\$w${rn})" " = 4"
> +
> + # Set all bits of x<rn>
> + gdb_test_no_output "set \$x${rn}=0xffffffffffffffff" \
> + "set all bits of x${rn}"
> +
> + # Test setting/fetching values
> + for {set i 0} {$i < 9} {incr i} {
> + global w_values
> + global x_values
> +
> + with_test_prefix "set w${rn} to $x_values($i)" {
> + # Set value of W and see the effects on W and X.
> + gdb_test_no_output "set \$w${rn}=$x_values($i)"
> + gdb_test "p/x \$w${rn}" "= $w_values($i)"
> + gdb_test "p/x \$x${rn}" "= $w_values($i)"
> + }
> +
> + with_test_prefix "set x${rn} to $x_values($i)" {
> + # Set value of X and see the effects on W and X.
> + gdb_test_no_output "set \$x${rn}=$x_values($i)"
> + gdb_test "p/x \$w${rn}" "= $w_values($i)"
> + gdb_test "p/x \$x${rn}" "= $x_values($i)"
> +
> + # Set all bits of x<rn>
> + gdb_test_no_output "set \$x${rn}=0xffffffffffffffff" \
> + "set all bits of x${rn}"
> + }
> + }
> +}
> +
> +# Run tests
> +foreach_with_prefix endian {"little" "big"} {
> + gdb_test "set endian ${endian}" "The target is set to ${endian} endian\."
> +
> + for {set i 0} {$i < 31} {incr i} {
> + test_register $i
> + }
> +}
Pushed now.
prev parent reply other threads:[~2022-10-03 13:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-22 16:52 Luis Machado
2022-10-03 13:16 ` Luis Machado [this message]
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=6ac7bc4f-f43e-890b-1bd3-dd8478d32974@arm.com \
--to=luis.machado@arm.com \
--cc=gdb-patches@sourceware.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).