Re: [PATCH v2 12/24] AArch64: Implement memory tagging target methods for AArch64

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 10/29/20 11:21 AM, Alan Hayward wrote:
> 
> 
>> On 22 Oct 2020, at 21:00, Luis Machado <luis.machado@linaro.org> wrote:
>>
>> Updates on v2:
>>
>> - Added type parameter to the target method implementations.
>>
>> --
>>
>> The patch implements the memory tagging target hooks for AArch64, so we
>> can handle MTE.
>>
>> gdb/ChangeLog:
>>
>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* Makefile.in (ALL_64_TARGET_OBS): Add arch/aarch64-mte-linux.o.
>> 	(HFILES_NO_SRCDIR): Add arch/aarch64-mte-linux.h and
>> 	nat/aarch64-mte-linux-ptrace.h.
>> 	* aarch64-linux-nat.c: Include nat/aarch64-mte-linux-ptrace.h.
>> 	(aarch64_linux_nat_target) <supports_memory_tagging>: New method
>> 	override.
>> 	<fetch_memtags>: New method override.
>> 	<store_memtags>: New method override.
>> 	(aarch64_linux_nat_target::supports_memory_tagging): New method.
>> 	(aarch64_linux_nat_target::fetch_memtags): New method.
>> 	(aarch64_linux_nat_target::store_memtags): New method.
>> 	* arch/aarch64-mte-linux.c: New file.
>> 	* arch/aarch64-mte-linux.h: Include gdbsupport/common-defs.h.
>> 	(MTE_GRANULE_SIZE): Define.
>> 	(get_tag_granules): New prototype.
>> 	* configure.nat (NATDEPFILES): Add nat/aarch64-mte-linux-ptrace.o.
>> 	* configure.tgt (aarch64*-*-linux*): Add arch/aarch64-mte-linux.o.
>> 	* nat/aarch64-mte-linux-ptrace.c: New file.
>> 	* nat/aarch64-mte-linux-ptrace.h: New file.
>> ---
>> gdb/Makefile.in                    |   1 +
>> gdb/aarch64-linux-nat.c            |  50 ++++++++
>> gdb/arch/aarch64-mte-linux.c       |  34 +++++
>> gdb/arch/aarch64-mte-linux.h       |  10 ++
>> gdb/configure.nat                  |   3 +-
>> gdb/configure.tgt                  |   1 +
>> gdb/nat/aarch64-mte-linux-ptrace.c | 200 +++++++++++++++++++++++++++++
>> gdb/nat/aarch64-mte-linux-ptrace.h |  17 +++
>> 8 files changed, 315 insertions(+), 1 deletion(-)
>> create mode 100644 gdb/arch/aarch64-mte-linux.c
>> create mode 100644 gdb/nat/aarch64-mte-linux-ptrace.c
>>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 8c9e6c9f6c..33a08a2288 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -692,6 +692,7 @@ ALL_64_TARGET_OBS = \
>> 	amd64-windows-tdep.o \
>> 	arch/aarch64.o \
>> 	arch/aarch64-insn.o \
>> +	arch/aarch64-mte-linux.o \
>> 	arch/amd64.o \
>> 	ia64-linux-tdep.o \
>> 	ia64-tdep.o \
>> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
>> index dea34da669..4edf5a0454 100644
>> --- a/gdb/aarch64-linux-nat.c
>> +++ b/gdb/aarch64-linux-nat.c
>> @@ -52,6 +52,8 @@
>>
>> #include "arch/aarch64-mte-linux.h"
>>
>> +#include "nat/aarch64-mte-linux-ptrace.h"
>> +
>> #ifndef TRAP_HWBKPT
>> #define TRAP_HWBKPT 0x0004
>> #endif
>> @@ -102,6 +104,16 @@ class aarch64_linux_nat_target final : public linux_nat_target
>>      override;
>>
>>    struct gdbarch *thread_architecture (ptid_t) override;
>> +
>> +  bool supports_memory_tagging () override;
>> +
>> +  /* Read memory allocation tags from memory via PTRACE.  */
>> +  int fetch_memtags (CORE_ADDR address, size_t len,
>> +		     gdb::byte_vector &tags, int type) override;
>> +
>> +  /* Write allocation tags to memory via PTRACE.  */
>> +  int store_memtags (CORE_ADDR address, size_t len,
>> +		     const gdb::byte_vector &tags, int type) override;
>> };
>>
>> static aarch64_linux_nat_target the_aarch64_linux_nat_target;
>> @@ -1050,6 +1062,44 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
>>    return gdbarch_find_by_info (info);
>> }
>>
>> +/* Implement the "supports_memory_tagging" target_ops method.  */
>> +
>> +bool
>> +aarch64_linux_nat_target::supports_memory_tagging ()
>> +{
>> +  return (linux_get_hwcap2 (this) & HWCAP2_MTE) != 0;
>> +}
>> +
>> +/* Implement the "fetch_memtags" target_ops method.  */
>> +
>> +int
>> +aarch64_linux_nat_target::fetch_memtags (CORE_ADDR address, size_t len,
>> +					 gdb::byte_vector &tags, int type)
> 
> I’m a little unsure as to where the type is coming from. Who in the call stack
> is explicitly passing the value 1?

Someone invoking the target methods directly or invoking the gdbarch 
hooks. For example, in gdb/printcmd.c:

   if (gdbarch_set_memtags (target_gdbarch (), val, 0, tags,
                            tag_logical) != 0)

Or:


   std::string tag = gdbarch_memtag_to_string (target_gdbarch (),
                                               val, tag_type);



> It’s different from the LOGICAL and ALLOCATION enum values used elsewhere?

No. Just a different type (int). The remote layer shouldn't try to 
interpret these tag types though. It should just forward them to the 
other side.