Re: [PATCH v3 10/24] AArch64: Add MTE register set support for GDB and gdbserver

Simon Marchi via Gdb-patches
2020-12-26T00:17:39Z

On 2020-11-09 12:04 p.m., Luis Machado via Gdb-patches wrote:
> AArch64 MTE support in the Linux kernel exposes a new register
> through ptrace. This patch adds the required code to support it.
>
> include/ChangeLog:
>
> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
>
> * elf/common.h (NT_ARM_TAGGED_ADDR_CTRL): Define.
>
> gdb/ChangeLog:
>
> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
>
> * aarch64-linux-nat.c (fetch_mteregs_from_thread): New function.
> (store_mteregs_to_thread): New function.
> (aarch64_linux_nat_target::fetch_registers): Update to call
> fetch_mteregs_from_thread.
> (aarch64_linux_nat_target::store_registers): Update to call
> store_mteregs_to_thread.
> * aarch64-tdep.c (aarch64_mte_register_names): New struct.
> (aarch64_cannot_store_register): Handle MTE registers.
> (aarch64_gdbarch_init): Initialize and setup MTE registers.
> * aarch64-tdep.h (gdbarch_tdep) <mte_reg_base>: New field.
> <has_mte>: New method.
> * arch/aarch64-linux.h (AARCH64_LINUX_SIZEOF_MTE): Define.
>
> gdbserver/ChangeLog:
>
> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
>
> * linux-aarch64-low.cc (aarch64_fill_mteregset): New function.
> (aarch64_store_mteregset): New function.
> (aarch64_regsets): Add MTE register set entry.
> (aarch64_sve_regsets): Add MTE register set entry.
> ---
> gdb/aarch64-linux-nat.c | 70 ++++++++++++++++++++++++++++++++++
> gdb/aarch64-tdep.c | 24 ++++++++++++
> gdb/aarch64-tdep.h | 9 +++++
> gdb/arch/aarch64-mte-linux.h | 3 ++
> gdbserver/linux-aarch64-low.cc | 29 ++++++++++++++
> include/elf/common.h | 3 ++
> 6 files changed, 138 insertions(+)
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 1392ec440c..dea34da669 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -461,6 +461,60 @@ fetch_pauth_masks_from_thread (struct regcache *regcache)
> &pauth_regset[1]);
> }
>
> +/* Fill GDB's register array with the MTE register values from
> + the current thread. */
> +
> +static void
> +fetch_mteregs_from_thread (struct regcache *regcache)
> +{
> + struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
> + int regno = tdep->mte_reg_base;
> +
> + gdb_assert (regno != -1);
> +
> + uint64_t tag_ctl = 0;
> + struct iovec iovec;
> +
> + iovec.iov_base = &tag_ctl;
> + iovec.iov_len = sizeof (tag_ctl);
> +
> + int tid = regcache->ptid ().lwp ();

Should this use get_ptrace_pid? Same in the store version.

> + if (ptrace (PTRACE_GETREGSET, tid, NT_ARM_TAGGED_ADDR_CTRL,
> + &iovec) != 0)

The indentation is wrong on this last line (and it would all fit on one line).

> diff --git a/include/elf/common.h b/include/elf/common.h
> index fc672de9f2..334e9e301f 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -660,6 +660,9 @@
> /* note name must be "LINUX". */
> #define NT_ARM_PAC_MASK 0x406 /* AArch pointer authentication code masks */
> /* note name must be "LINUX". */
> +#define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* AArch64 tagged address control
> + (prctl()) */
> + /* note name must be "LINUX". */
> #define NT_ARC_V2 0x600 /* ARC HS accumulator/extra registers. */
> /* note name must be "LINUX". */
> #define NT_SIGINFO 0x53494749 /* Fields of siginfo_t. */

What this changed approved by binutils?

Otherwise, this LGTM.

Simon