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

Luis Machado via Gdb-patches
2020-10-29T14:39:12Z

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.