Re: [PATCH v3 17/24] AArch64: Add gdbserver MTE support

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

On 12/26/20 7:30 PM, Simon Marchi wrote:
> 
> 
> On 2020-11-09 12:04 p.m., Luis Machado via Gdb-patches wrote:
>> Updates on v2:
>>
>> - Updated target methods to contain a tag type parameter.
>>
>> --
>>
>> Adds the AArch64-specific memory tagging support (MTE) by implementing the
>> required hooks and checks for GDBserver.
>>
>> gdbserver/ChangeLog:
>>
>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
>>
>> 	* Makefile.in (SFILES): Add /../gdb/nat/aarch64-mte-linux-ptrace.c.
>> 	* configure.srv (aarch64*-*-linux*): Add arch/aarch64-mte-linux.o and
>> 	nat/aarch64-mte-linux-ptrace.o.
>> 	* linux-aarch64-low.cc: Include nat/aarch64-mte-linux-ptrace.h.
>> 	(class aarch64_target) <supports_memory_tagging>
>> 	<fetch_memtags, store_memtags>: New method overrides.
>> 	(aarch64_target::supports_memory_tagging)
>> 	(aarch64_target::fetch_memtags)
>> 	(aarch64_target::store_memtags): New methods.
>> ---
>>   gdbserver/Makefile.in          |  1 +
>>   gdbserver/configure.srv        |  2 ++
>>   gdbserver/linux-aarch64-low.cc | 61 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 64 insertions(+)
>>
>> diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
>> index 1969ed0ec3..45b605c3b5 100644
>> --- a/gdbserver/Makefile.in
>> +++ b/gdbserver/Makefile.in
>> @@ -221,6 +221,7 @@ SFILES = \
>>   	$(srcdir)/../gdb/arch/ppc-linux-common.c \
>>   	$(srcdir)/../gdb/arch/riscv.c \
>>   	$(srcdir)/../gdb/nat/aarch64-sve-linux-ptrace.c \
>> +	$(srcdir)/../gdb/nat/aarch64-mte-linux-ptrace.c \
>>   	$(srcdir)/../gdb/nat/linux-btrace.c \
>>   	$(srcdir)/../gdb/nat/linux-namespaces.c \
>>   	$(srcdir)/../gdb/nat/linux-osdata.c \
>> diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
>> index 833ad27c4c..0bbd990758 100644
>> --- a/gdbserver/configure.srv
>> +++ b/gdbserver/configure.srv
>> @@ -52,8 +52,10 @@ case "${gdbserver_host}" in
>>   			srv_tgtobj="$srv_tgtobj nat/aarch64-linux.o"
>>   			srv_tgtobj="$srv_tgtobj arch/aarch64-insn.o"
>>   			srv_tgtobj="$srv_tgtobj arch/aarch64.o"
>> +			srv_tgtobj="$srv_tgtobj arch/aarch64-mte-linux.o"
>>   			srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
>>   			srv_tgtobj="$srv_tgtobj nat/aarch64-sve-linux-ptrace.o"
>> +			srv_tgtobj="$srv_tgtobj nat/aarch64-mte-linux-ptrace.o"
> 
> Please keep the list sorted.
> 

Fixed.

>> +int
>> +aarch64_target::fetch_memtags (CORE_ADDR address, size_t len,
>> +			       gdb::byte_vector &tags, int type)
>> +{
>> +  /* Allocation tags are per-process, so any tid is fine.  */
>> +  int tid = lwpid_of (current_thread);
>> +
>> +  /* Allocation tag?  */
>> +  if (type == 1)
>> +    return aarch64_mte_fetch_memtags (tid, address, len, tags);
> 
> Is there a define you could use instead of the magic "1"?

Yes. I suppose I could share the definition with the GDB code. I'll fix 
the code.

> 
> Simon
>