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

Alan Hayward via Gdb-patches
2020-10-29T17:32:26Z

> On 29 Oct 2020, at 14:45, Luis Machado <luis.machado@linaro.org> wrote:
>
> On 10/29/20 11:39 AM, Luis Machado wrote:
>> 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.
>
> Complementing the answer... It isn't the case at the moment, but one of the layers of the target could translate the generic enums (tag_logical/tag_allocation) to something different.
>
> That's why I don't want the remote target to be aware of what these tag types mean.

Understandable, and it makes sense. Probably not worth the translation complexity until
there are additional types.

Alan.