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: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.