| Message ID | 20240404064819.2848899-3-gustavo.romero@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add another way to check tagged addresses on remote targets | expand |
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
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 + 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"));
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(-)