diff mbox series

[v4,2/8] gdb: aarch64: Move MTE address check out of set_memtag

Message ID 20240416140728.198163-3-gustavo.romero@linaro.org
State New
Headers
Series Add another way to check tagged addresses on remote targets |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed

Commit Message

Gustavo Romero April 16, 2024, 2:07 p.m. UTC 
  Remove check in parse_set_allocation_tag_input as it is redundant:
currently the check happens at the end of parse_set_allocation_tag_input
and also in set_memtag (called after parse_set_allocation_tag_input).

After it, 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
on MTE address checks.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
 gdb/aarch64-linux-tdep.c |  4 ----
 gdb/printcmd.c           | 10 +++++-----
 2 files changed, 5 insertions(+), 9 deletions(-)

Comments

Luis Machado April 16, 2024, 4:30 p.m. UTC | #1 
On 4/16/24 15:07, Gustavo Romero wrote:
> Remove check in parse_set_allocation_tag_input as it is redundant:
> currently the check happens at the end of parse_set_allocation_tag_input
> and also in set_memtag (called after parse_set_allocation_tag_input).
> 
> After it, 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
> on MTE address checks.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>  gdb/aarch64-linux-tdep.c |  4 ----
>  gdb/printcmd.c           | 10 +++++-----
>  2 files changed, 5 insertions(+), 9 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..5635f605314 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -3101,11 +3101,6 @@ parse_set_allocation_tag_input (const char *args, struct value **val,
>      error (_("Error parsing tags argument. Tags should be 2 digits per byte."));
>  
>    tags = hex2bin (tags_string.c_str ());
> -
> -  /* If the address is not in a region memory mapped with a memory tagging
> -     flag, it is no use trying to access/manipulate its allocation tag.  */
> -  if (!gdbarch_tagged_address_p (current_inferior ()->arch (), *val))
> -    show_addr_not_tagged (value_as_address (*val));
>  }
>  
>  /* Implement the "memory-tag set-allocation-tag" command.
> @@ -3127,6 +3122,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"));

Thanks.

Approved-By: Luis Machado <luis.machado@arm.com>
diff mbox series

Patch

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..5635f605314 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -3101,11 +3101,6 @@  parse_set_allocation_tag_input (const char *args, struct value **val,
     error (_("Error parsing tags argument. Tags should be 2 digits per byte."));
 
   tags = hex2bin (tags_string.c_str ());
-
-  /* If the address is not in a region memory mapped with a memory tagging
-     flag, it is no use trying to access/manipulate its allocation tag.  */
-  if (!gdbarch_tagged_address_p (current_inferior ()->arch (), *val))
-    show_addr_not_tagged (value_as_address (*val));
 }
 
 /* Implement the "memory-tag set-allocation-tag" command.
@@ -3127,6 +3122,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"));