[v3,2/7] gdb: aarch64: Move MTE address check out of set_memtag
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
Move MTE address check out of set_memtag and add this check to the
upper layer, before set_memtag is called. This is a preparation for
using a target hook instead of a gdbarch hook MTE address checks.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
gdb/aarch64-linux-tdep.c | 4 ----
gdb/printcmd.c | 5 +++++
2 files changed, 5 insertions(+), 4 deletions(-)
Comments
On 4/4/24 07:48, Gustavo Romero wrote:
> Move MTE address check out of set_memtag and add this check to the
> upper layer, before set_memtag is called. This is a preparation for
> using a target hook instead of a gdbarch hook MTE address checks.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> gdb/aarch64-linux-tdep.c | 4 ----
> gdb/printcmd.c | 5 +++++
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 50055ac3f48..8e6e63d4dcb 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -2525,10 +2525,6 @@ aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
> /* Remove the top byte. */
> addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>
> - /* Make sure we are dealing with a tagged address to begin with. */
> - if (!aarch64_linux_tagged_address_p (gdbarch, address))
> - return false;
> -
> /* With G being the number of tag granules and N the number of tags
> passed in, we can have the following cases:
>
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index cb0d32aa4bc..774e3ec74ae 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -3127,6 +3127,11 @@ memory_tag_set_allocation_tag_command (const char *args, int from_tty)
> /* Parse the input. */
> parse_set_allocation_tag_input (args, &val, &length, tags);
>
> + /* If the address is not in a region memory mapped with a memory tagging
A nit:
s/memory mapped/memory-mapped ?
> + flag, it is no use trying to manipulate its allocation tag. */
> + if (!gdbarch_tagged_address_p (current_inferior ()->arch (), val))
> + show_addr_not_tagged (value_as_address(val));
> +
Looks like memory_tag_set_allocation_tag_command calls parse_set_allocation_tag_input,
and the latter has the exact same check as you are adding here. Is this then redundant?
> if (!gdbarch_set_memtags (current_inferior ()->arch (), val, length, tags,
> memtag_type::allocation))
> gdb_printf (_("Could not update the allocation tag(s).\n"));
Hi Luis,
On 4/4/24 11:17 AM, Luis Machado wrote:
> On 4/4/24 07:48, Gustavo Romero wrote:
>> Move MTE address check out of set_memtag and add this check to the
>> upper layer, before set_memtag is called. This is a preparation for
>> using a target hook instead of a gdbarch hook MTE address checks.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>> gdb/aarch64-linux-tdep.c | 4 ----
>> gdb/printcmd.c | 5 +++++
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
>> index 50055ac3f48..8e6e63d4dcb 100644
>> --- a/gdb/aarch64-linux-tdep.c
>> +++ b/gdb/aarch64-linux-tdep.c
>> @@ -2525,10 +2525,6 @@ aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
>> /* Remove the top byte. */
>> addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>>
>> - /* Make sure we are dealing with a tagged address to begin with. */
>> - if (!aarch64_linux_tagged_address_p (gdbarch, address))
>> - return false;
>> -
>> /* With G being the number of tag granules and N the number of tags
>> passed in, we can have the following cases:
>>
>> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
>> index cb0d32aa4bc..774e3ec74ae 100644
>> --- a/gdb/printcmd.c
>> +++ b/gdb/printcmd.c
>> @@ -3127,6 +3127,11 @@ memory_tag_set_allocation_tag_command (const char *args, int from_tty)
>> /* Parse the input. */
>> parse_set_allocation_tag_input (args, &val, &length, tags);
>>
>> + /* If the address is not in a region memory mapped with a memory tagging
>
> A nit:
>
> s/memory mapped/memory-mapped ?
>
>> + flag, it is no use trying to manipulate its allocation tag. */
>> + if (!gdbarch_tagged_address_p (current_inferior ()->arch (), val))
>> + show_addr_not_tagged (value_as_address(val));
>> +
>
> Looks like memory_tag_set_allocation_tag_command calls parse_set_allocation_tag_input,
> and the latter has the exact same check as you are adding here. Is this then redundant?
hmm, right, it is (and was) redundant. I'm removing the check from parse_set_allocation_tag_input.
Thanks,
Gustavo
@@ -2525,10 +2525,6 @@ aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
/* Remove the top byte. */
addr = gdbarch_remove_non_address_bits (gdbarch, addr);
- /* Make sure we are dealing with a tagged address to begin with. */
- if (!aarch64_linux_tagged_address_p (gdbarch, address))
- return false;
-
/* With G being the number of tag granules and N the number of tags
passed in, we can have the following cases:
@@ -3127,6 +3127,11 @@ memory_tag_set_allocation_tag_command (const char *args, int from_tty)
/* Parse the input. */
parse_set_allocation_tag_input (args, &val, &length, tags);
+ /* If the address is not in a region memory mapped with a memory tagging
+ flag, it is no use trying to manipulate its allocation tag. */
+ if (!gdbarch_tagged_address_p (current_inferior ()->arch (), val))
+ show_addr_not_tagged (value_as_address(val));
+
if (!gdbarch_set_memtags (current_inferior ()->arch (), val, length, tags,
memtag_type::allocation))
gdb_printf (_("Could not update the allocation tag(s).\n"));