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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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