[v3] Fix printing of non-address types when memory tagging is enabled

Message ID 1b1ecf62-ac66-5c4c-ebce-3bc353e9ceee@palves.net
State New
Headers show
Series
  • [v3] Fix printing of non-address types when memory tagging is enabled
Related show

Commit Message

Pedro Alves July 7, 2021, 10:49 a.m.
On 2021-07-06 5:32 p.m., Luis Machado wrote:
> On 7/5/21 8:06 PM, Pedro Alves wrote:

>> Hi!

>>

>> On 2021-07-05 3:32 p.m., Luis Machado wrote:


>>>>>> My thinking is that it should be possible to tell upfront whether you're asking to

>>>>>> convert something to an address that it doesn't make sense to try.

>>>>>

>>>>> It is certainly possible, but the complexity of trying to handle all of the corner cases of working with types/values makes it not worth the effort in my opinion. Even GDB's type/value handling code is lengthy and doesn't handle all the possible casts. The user can throw all sorts of weird expressions at the print command, and unfortunately this code would need to get through that to figure out if it has any tags.

>>>>

>>>> I think I must be missing something.  You're working with the result of the expression, not the

>>>> intermediate values of the expression.  And you're looking at the value and only converting it

>>>> to address if it is TYPE_CODE_PTR, which means that value is a scalar.  What do the user

>>>> expressions have to do with it

>>>

>>> Right. We're only dealing with the result of the expression. If that result has a type/content that causes GDB's value/type functions to throw, then the optimized out checks won't catch that.

>>>

>>> For example, if we feed a 128-bit pointer to the print command, value_as_address will throw with an invalid cast.

>>

>> Then how can GDB ever work with such pointers even without the tag checking in printcmd.c?

>> Such a port would have to fix that anyhow, no?  Likely with a custom

>> gdbarch_pointer_to_address implementation.

>>

> 

> Yes, but this is mainline GDB, and the rationale is to anticipate that need by covering that case in this patch.


Sorry, but that doesn't make sense -- there's no mainline arch with 128-bit pointers.  There can't be, it would
hit that issue all over the place.  Any arch with 128-bit pointers will need to implement gdbarch_pointer_to_address
and thus that exception won't be hit.

>>>

>>> So, will a try/catch in printcmd.c be a reasonable solution then? I can change the code back to do that. And also check the optimized out condition in the arch hook.

>>

>> I think so, and with printing the error, because I think that an error here would

>> either mean something unexpected happened that we should improve and we best not swallow

>> it so we can address it in future, or, it is something interesting for the user.

>>

>> See patch at the bottom.

>>

> 

> I gave this a go and it looks good testsuite-wise.


OK.

> Unfortunately I don't recall exactly what was the case for references. GDB does bend things a little so users have a better experience. GDB will sometimes implicitly dereference some things when the language wouldn't, for example.


That's not the case for references, there's no bending here.  References are just pointers with a different sugar coat.
GDB automatically prints the object the reference references, but that's unrelated -- it'd be like GDB automatically
deferencing pointers.  Reading the reference itself is just like reading a pointer.

> 

> These failures showed up in the testsuite by the way. That's why I wrote the patch.

> 



>> Also, can you add some tests?  The need for this clearly went unnoticed due to lack of test coverage.

> 

> The testsuite did catch these. There were problems with optimized out values, method pointers, references (so it seemed at the time) and trying to fetch addresses from non-pointer types. All of those were caught by running the testsuite on a MTE-capable setup.


Heh.  Curious.  So it wasn't possible to run the testsuite on such a setup before the series went in?  Or what changed
since then?

> 

> What sort of test did you have in mind, other than gdb.arch/aarch64-mte.exp, which already covers printing things and adjusting tags?


Hmm, the fact that this wasn't noticed before made me assume that memory tag checking was off by default, so I was
thinking of some tests that turn it on and print non-pointer/reference things.  But I now see that it's on by default,
so forget about it.

Here's the patch again, now with updated git commit log.  Please double check.
The only code change is that I dropped the ending period in the fprintf call, as some
errors already end with a period.

I updated the ChangeLog as I suspect you'll want this in the release branch.

For putting it on the branch we'll need a bugzilla bug though, so I'll leave it to
you from here: file a bug, and then mentioning the bug in the git commit log, and
then merging this, master and branch.

From a20c524db9c58fa7ee57aa0cb05d934b267965ce Mon Sep 17 00:00:00 2001
From: Luis Machado <luis.machado@linaro.org>

Date: Mon, 5 Jul 2021 18:51:11 +0100
Subject: [PATCH] Fix printing of non-address types when memory tagging is
 enabled

When the architecture supports memory tagging, we handle
pointer/reference types in a special way, so we can validate tags and
show mismatches.

Unfortunately, the currently implementation errors out when the user
prints non-address values: composite types, floats, references, member
functions and other things.

Vector registers:

 (gdb) p $v0
 Value can't be converted to integer.

Non-existent internal variables:

 (gdb) p $foo
 Value can't be converted to integer.

The same happens for complex types and printing struct/union types.

There are a few problems here.

The first one is that after print_command_1 evaluates the expression
to print, the tag validation code call value_as_address
unconditionally, without making sure we have have a suitable type
where it makes to sense to call it.  That results in value_as_address
(if it isn't given a pointer-like type) trying to treat the value as
an integer and convert it to an address, which #1 - doesn't make sense
(i.e., no sense in validating tags after "print 1"), and throws for
non-integer-convertible types.  We fix this by making sure we have a
pointer or reference type first, and only if so then proceed to check
if the address-like value has tags.

The second is that we're calling value_as_address even if we have an
optimized out or unavailable value, which throws, because the value's
contents aren't fully accessible/readable.  This error currently
escapes out and aborts the print.  This case is fixed by checking for
optimized out / unavailable explicitly.

Third, the tag checking process does not gracefully handle exceptions.
If any exception is thrown from the tag validation code, we abort the
print.  E.g., the target may fail to access tags via a running thread.
Or the needed /proc files aren't available.  Or some other untold
reason.  This is a bit too rigid.  This commit changes print_command_1
to catch errors, print them, and still continue with the normal
expression printing path instead of erroring out and printing nothing
useful.

With this patch, printing works correctly again:

 (gdb) p $v0
 $1 = {d = {f = {2.0546950501119882e-81, 2.0546950501119882e-81}, u = {3399988123389603631, 3399988123389603631}, s = {
       3399988123389603631, 3399988123389603631}}, s = {f = {1.59329203e-10, 1.59329203e-10, 1.59329203e-10, 1.59329203e-10}, u = {
       791621423, 791621423, 791621423, 791621423}, s = {791621423, 791621423, 791621423, 791621423}}, h = {bf = {1.592e-10,
       1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10}, f = {0.11224, 0.11224, 0.11224, 0.11224, 0.11224,
       0.11224, 0.11224, 0.11224}, u = {12079, 12079, 12079, 12079, 12079, 12079, 12079, 12079}, s = {12079, 12079, 12079, 12079,
       12079, 12079, 12079, 12079}}, b = {u = {47 <repeats 16 times>}, s = {47 <repeats 16 times>}}, q = {u = {
       62718710765820030520700417840365121327}, s = {62718710765820030520700417840365121327}}}
 (gdb) p $foo
 $2 = void
 (gdb) p 2 + 2i
 $3 = 2 + 2i

gdb/ChangeLog
YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>
	    Pedro Alves  <pedro@palves.net>

	* gdbarch.sh: Updated documentation for gdbarch_tagged_address_p.
	* gdbarch.h: Regenerate.
	* printcmd.c (should_validate_memtags): Reorder comparisons and only
	validate tags for pointer and reference types.  Skip validation of
	optimized out or unavailable values.
	(print_command_1): Guard call memory tagging validation code with
	a try/catch block.

Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: I82bf00ac88d23553b3f7563c9872dfa6ca1f2207
---
 gdb/gdbarch.h  |  3 +-
 gdb/gdbarch.sh |  3 +-
 gdb/printcmd.c | 81 ++++++++++++++++++++++++++++++++------------------
 3 files changed, 56 insertions(+), 31 deletions(-)


base-commit: 74ace054855321bc5271dd7b354131cd0b71333a
-- 
2.26.2

Comments

Joel Brobecker July 17, 2021, 6:53 p.m. | #1
Hey Pedro, hey Luis,

> gdb/ChangeLog

> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

> 	    Pedro Alves  <pedro@palves.net>

> 

> 	* gdbarch.sh: Updated documentation for gdbarch_tagged_address_p.

> 	* gdbarch.h: Regenerate.

> 	* printcmd.c (should_validate_memtags): Reorder comparisons and only

> 	validate tags for pointer and reference types.  Skip validation of

> 	optimized out or unavailable values.

> 	(print_command_1): Guard call memory tagging validation code with

> 	a try/catch block.


What's the status for this version of the patch that Pedro suggested?
Do we think this is something we could approve and push sometime soon?

Thanks!
-- 
Joel
will schmidt via Gdb-patches July 19, 2021, 9:37 a.m. | #2
Hi,

On 7/17/21 3:53 PM, Joel Brobecker wrote:
> Hey Pedro, hey Luis,

> 

>> gdb/ChangeLog

>> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

>> 	    Pedro Alves  <pedro@palves.net>

>>

>> 	* gdbarch.sh: Updated documentation for gdbarch_tagged_address_p.

>> 	* gdbarch.h: Regenerate.

>> 	* printcmd.c (should_validate_memtags): Reorder comparisons and only

>> 	validate tags for pointer and reference types.  Skip validation of

>> 	optimized out or unavailable values.

>> 	(print_command_1): Guard call memory tagging validation code with

>> 	a try/catch block.

> 

> What's the status for this version of the patch that Pedro suggested?

> Do we think this is something we could approve and push sometime soon?


I'm coming back from vacation today. I'll check.

> 

> Thanks!

>
will schmidt via Gdb-patches July 19, 2021, 6:50 p.m. | #3
On 7/7/21 7:49 AM, Pedro Alves wrote:
> On 2021-07-06 5:32 p.m., Luis Machado wrote:

>> On 7/5/21 8:06 PM, Pedro Alves wrote:

>>> Hi!

>>>

>>> On 2021-07-05 3:32 p.m., Luis Machado wrote:

> 

>>>>>>> My thinking is that it should be possible to tell upfront whether you're asking to

>>>>>>> convert something to an address that it doesn't make sense to try.

>>>>>>

>>>>>> It is certainly possible, but the complexity of trying to handle all of the corner cases of working with types/values makes it not worth the effort in my opinion. Even GDB's type/value handling code is lengthy and doesn't handle all the possible casts. The user can throw all sorts of weird expressions at the print command, and unfortunately this code would need to get through that to figure out if it has any tags.

>>>>>

>>>>> I think I must be missing something.  You're working with the result of the expression, not the

>>>>> intermediate values of the expression.  And you're looking at the value and only converting it

>>>>> to address if it is TYPE_CODE_PTR, which means that value is a scalar.  What do the user

>>>>> expressions have to do with it

>>>>

>>>> Right. We're only dealing with the result of the expression. If that result has a type/content that causes GDB's value/type functions to throw, then the optimized out checks won't catch that.

>>>>

>>>> For example, if we feed a 128-bit pointer to the print command, value_as_address will throw with an invalid cast.

>>>

>>> Then how can GDB ever work with such pointers even without the tag checking in printcmd.c?

>>> Such a port would have to fix that anyhow, no?  Likely with a custom

>>> gdbarch_pointer_to_address implementation.

>>>

>>

>> Yes, but this is mainline GDB, and the rationale is to anticipate that need by covering that case in this patch.

> 

> Sorry, but that doesn't make sense -- there's no mainline arch with 128-bit pointers.  There can't be, it would

> hit that issue all over the place.  Any arch with 128-bit pointers will need to implement gdbarch_pointer_to_address

> and thus that exception won't be hit.

> 


Yes. But when I say this is mainline GDB, I mean I'm not aiming at 
fixing 128-bit pointers, but I'm merely anticipating that 128-bit 
pointers being fed to this code pose a problem that should be fixed.

>>>>

>>>> So, will a try/catch in printcmd.c be a reasonable solution then? I can change the code back to do that. And also check the optimized out condition in the arch hook.

>>>

>>> I think so, and with printing the error, because I think that an error here would

>>> either mean something unexpected happened that we should improve and we best not swallow

>>> it so we can address it in future, or, it is something interesting for the user.

>>>

>>> See patch at the bottom.

>>>

>>

>> I gave this a go and it looks good testsuite-wise.

> 

> OK.

> 

>> Unfortunately I don't recall exactly what was the case for references. GDB does bend things a little so users have a better experience. GDB will sometimes implicitly dereference some things when the language wouldn't, for example.

> 

> That's not the case for references, there's no bending here.  References are just pointers with a different sugar coat.

> GDB automatically prints the object the reference references, but that's unrelated -- it'd be like GDB automatically

> deferencing pointers.  Reading the reference itself is just like reading a pointer.

> 


Sounds good then.

>>

>> These failures showed up in the testsuite by the way. That's why I wrote the patch.

>>

> 

> 

>>> Also, can you add some tests?  The need for this clearly went unnoticed due to lack of test coverage.

>>

>> The testsuite did catch these. There were problems with optimized out values, method pointers, references (so it seemed at the time) and trying to fetch addresses from non-pointer types. All of those were caught by running the testsuite on a MTE-capable setup.

> 

> Heh.  Curious.  So it wasn't possible to run the testsuite on such a setup before the series went in?  Or what changed

> since then?

> 


MTE is not available on hardware yet. It also wasn't available on a 
mainline kernel until some time ago. Therefore it needs to be exercised 
from within an emulator environment. A GDB testsuite run on a setup like 
that takes a few hours to complete.

So it was perfectly possible to run the testsuite on such a setup. I did 
it a number of times, and again when the changes went in. Hence why I 
wrote the patch when I noticed the failures (that weren't critical 
really, as aarch64 is the only target supporting memory tagging with the 
new interface, and that is not available on hardware yet).

Between copying GDB builds, updated kernels and filtering testsuite 
runs, a mistake might have happened that prevented me from noticing this 
earlier.


>>

>> What sort of test did you have in mind, other than gdb.arch/aarch64-mte.exp, which already covers printing things and adjusting tags?

> 

> Hmm, the fact that this wasn't noticed before made me assume that memory tag checking was off by default, so I was

> thinking of some tests that turn it on and print non-pointer/reference things.  But I now see that it's on by default,

> so forget about it.

> 

> Here's the patch again, now with updated git commit log.  Please double check.

> The only code change is that I dropped the ending period in the fprintf call, as some

> errors already end with a period.


I gave it a try again and it looks OK testsuite-wise.

> 

> I updated the ChangeLog as I suspect you'll want this in the release branch.

> 

> For putting it on the branch we'll need a bugzilla bug though, so I'll leave it to

> you from here: file a bug, and then mentioning the bug in the git commit log, and

> then merging this, master and branch.

> 

>  From a20c524db9c58fa7ee57aa0cb05d934b267965ce Mon Sep 17 00:00:00 2001

> From: Luis Machado <luis.machado@linaro.org>

> Date: Mon, 5 Jul 2021 18:51:11 +0100

> Subject: [PATCH] Fix printing of non-address types when memory tagging is

>   enabled

> 

> When the architecture supports memory tagging, we handle

> pointer/reference types in a special way, so we can validate tags and

> show mismatches.

> 

> Unfortunately, the currently implementation errors out when the user

> prints non-address values: composite types, floats, references, member

> functions and other things.

> 

> Vector registers:

> 

>   (gdb) p $v0

>   Value can't be converted to integer.

> 

> Non-existent internal variables:

> 

>   (gdb) p $foo

>   Value can't be converted to integer.

> 

> The same happens for complex types and printing struct/union types.

> 

> There are a few problems here.

> 

> The first one is that after print_command_1 evaluates the expression

> to print, the tag validation code call value_as_address

> unconditionally, without making sure we have have a suitable type

> where it makes to sense to call it.  That results in value_as_address

> (if it isn't given a pointer-like type) trying to treat the value as

> an integer and convert it to an address, which #1 - doesn't make sense

> (i.e., no sense in validating tags after "print 1"), and throws for

> non-integer-convertible types.  We fix this by making sure we have a

> pointer or reference type first, and only if so then proceed to check

> if the address-like value has tags.

> 

> The second is that we're calling value_as_address even if we have an

> optimized out or unavailable value, which throws, because the value's

> contents aren't fully accessible/readable.  This error currently

> escapes out and aborts the print.  This case is fixed by checking for

> optimized out / unavailable explicitly.

> 

> Third, the tag checking process does not gracefully handle exceptions.

> If any exception is thrown from the tag validation code, we abort the

> print.  E.g., the target may fail to access tags via a running thread.

> Or the needed /proc files aren't available.  Or some other untold

> reason.  This is a bit too rigid.  This commit changes print_command_1

> to catch errors, print them, and still continue with the normal

> expression printing path instead of erroring out and printing nothing

> useful.

> 

> With this patch, printing works correctly again:

> 

>   (gdb) p $v0

>   $1 = {d = {f = {2.0546950501119882e-81, 2.0546950501119882e-81}, u = {3399988123389603631, 3399988123389603631}, s = {

>         3399988123389603631, 3399988123389603631}}, s = {f = {1.59329203e-10, 1.59329203e-10, 1.59329203e-10, 1.59329203e-10}, u = {

>         791621423, 791621423, 791621423, 791621423}, s = {791621423, 791621423, 791621423, 791621423}}, h = {bf = {1.592e-10,

>         1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10, 1.592e-10}, f = {0.11224, 0.11224, 0.11224, 0.11224, 0.11224,

>         0.11224, 0.11224, 0.11224}, u = {12079, 12079, 12079, 12079, 12079, 12079, 12079, 12079}, s = {12079, 12079, 12079, 12079,

>         12079, 12079, 12079, 12079}}, b = {u = {47 <repeats 16 times>}, s = {47 <repeats 16 times>}}, q = {u = {

>         62718710765820030520700417840365121327}, s = {62718710765820030520700417840365121327}}}

>   (gdb) p $foo

>   $2 = void

>   (gdb) p 2 + 2i

>   $3 = 2 + 2i

> 

> gdb/ChangeLog

> YYYY-MM-DD  Luis Machado  <luis.machado@linaro.org>

> 	    Pedro Alves  <pedro@palves.net>

> 

> 	* gdbarch.sh: Updated documentation for gdbarch_tagged_address_p.

> 	* gdbarch.h: Regenerate.

> 	* printcmd.c (should_validate_memtags): Reorder comparisons and only

> 	validate tags for pointer and reference types.  Skip validation of

> 	optimized out or unavailable values.

> 	(print_command_1): Guard call memory tagging validation code with

> 	a try/catch block.

> 

> Co-Authored-By: Pedro Alves <pedro@palves.net>

> Change-Id: I82bf00ac88d23553b3f7563c9872dfa6ca1f2207

> ---

>   gdb/gdbarch.h  |  3 +-

>   gdb/gdbarch.sh |  3 +-

>   gdb/printcmd.c | 81 ++++++++++++++++++++++++++++++++------------------

>   3 files changed, 56 insertions(+), 31 deletions(-)

> 

> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h

> index ece765b826f..7db3e36d76a 100644

> --- a/gdb/gdbarch.h

> +++ b/gdb/gdbarch.h

> @@ -730,7 +730,8 @@ typedef std::string (gdbarch_memtag_to_string_ftype) (struct gdbarch *gdbarch, s

>   extern std::string gdbarch_memtag_to_string (struct gdbarch *gdbarch, struct value *tag);

>   extern void set_gdbarch_memtag_to_string (struct gdbarch *gdbarch, gdbarch_memtag_to_string_ftype *memtag_to_string);

>   

> -/* Return true if ADDRESS contains a tag and false otherwise. */

> +/* Return true if ADDRESS contains a tag and false otherwise.  ADDRESS

> +   must be either a pointer or a reference type. */

>   

>   typedef bool (gdbarch_tagged_address_p_ftype) (struct gdbarch *gdbarch, struct value *address);

>   extern bool gdbarch_tagged_address_p (struct gdbarch *gdbarch, struct value *address);

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh

> index d9332c2103e..9bc9de91c30 100755

> --- a/gdb/gdbarch.sh

> +++ b/gdb/gdbarch.sh

> @@ -608,7 +608,8 @@ v;int;significant_addr_bit;;;;;;0

>   # Return a string representation of the memory tag TAG.

>   m;std::string;memtag_to_string;struct value *tag;tag;;default_memtag_to_string;;0

>   

> -# Return true if ADDRESS contains a tag and false otherwise.

> +# Return true if ADDRESS contains a tag and false otherwise.  ADDRESS

> +# must be either a pointer or a reference type.

>   m;bool;tagged_address_p;struct value *address;address;;default_tagged_address_p;;0

>   

>   # Return true if the tag from ADDRESS matches the memory tag for that

> diff --git a/gdb/printcmd.c b/gdb/printcmd.c

> index 3cd42f817f5..416b87c69c6 100644

> --- a/gdb/printcmd.c

> +++ b/gdb/printcmd.c

> @@ -1266,19 +1266,26 @@ print_value (value *val, const value_print_options &opts)

>   static bool

>   should_validate_memtags (struct value *value)

>   {

> -  if (target_supports_memory_tagging ()

> -      && gdbarch_tagged_address_p (target_gdbarch (), value))

> -    {

> -      gdb_assert (value != nullptr && value_type (value) != nullptr);

> +  gdb_assert (value != nullptr && value_type (value) != nullptr);

>   

> -      enum type_code code = value_type (value)->code ();

> +  if (!target_supports_memory_tagging ())

> +    return false;

>   

> -      return (code == TYPE_CODE_PTR

> -	      || code == TYPE_CODE_REF

> -	      || code == TYPE_CODE_METHODPTR

> -	      || code == TYPE_CODE_MEMBERPTR);

> -    }

> -  return false;

> +  enum type_code code = value_type (value)->code ();

> +

> +  /* Skip non-address values.  */

> +  if (code != TYPE_CODE_PTR

> +      && !TYPE_IS_REFERENCE (value_type (value)))

> +    return false;

> +

> +  /* OK, we have an address value.  Check we have a complete value we

> +     can extract.  */

> +  if (value_optimized_out (value)

> +      || !value_entirely_available (value))

> +    return false;

> +

> +  /* We do.  Check whether it includes any tags.  */

> +  return gdbarch_tagged_address_p (target_gdbarch (), value);

>   }

>   

>   /* Helper for parsing arguments for print_command_1.  */

> @@ -1321,26 +1328,42 @@ print_command_1 (const char *args, int voidprint)

>   		    value_type (val)->code () != TYPE_CODE_VOID))

>       {

>         /* If memory tagging validation is on, check if the tag is valid.  */

> -      if (print_opts.memory_tag_violations && should_validate_memtags (val)

> -	  && !gdbarch_memtag_matches_p (target_gdbarch (), val))

> +      if (print_opts.memory_tag_violations)

>   	{

> -	  /* Fetch the logical tag.  */

> -	  struct value *tag

> -	    = gdbarch_get_memtag (target_gdbarch (), val,

> -				  memtag_type::logical);

> -	  std::string ltag

> -	    = gdbarch_memtag_to_string (target_gdbarch (), tag);

> -

> -	  /* Fetch the allocation tag.  */

> -	  tag = gdbarch_get_memtag (target_gdbarch (), val,

> -				    memtag_type::allocation);

> -	  std::string atag

> -	    = gdbarch_memtag_to_string (target_gdbarch (), tag);

> -

> -	  printf_filtered (_("Logical tag (%s) does not match the "

> -			     "allocation tag (%s).\n"),

> -			   ltag.c_str (), atag.c_str ());

> +	  try

> +	    {

> +	      if (should_validate_memtags (val)

> +		  && !gdbarch_memtag_matches_p (target_gdbarch (), val))

> +		{

> +		  /* Fetch the logical tag.  */

> +		  struct value *tag

> +		    = gdbarch_get_memtag (target_gdbarch (), val,

> +					  memtag_type::logical);

> +		  std::string ltag

> +		    = gdbarch_memtag_to_string (target_gdbarch (), tag);

> +

> +		  /* Fetch the allocation tag.  */

> +		  tag = gdbarch_get_memtag (target_gdbarch (), val,

> +					    memtag_type::allocation);

> +		  std::string atag

> +		    = gdbarch_memtag_to_string (target_gdbarch (), tag);

> +

> +		  printf_filtered (_("Logical tag (%s) does not match the "

> +				     "allocation tag (%s).\n"),

> +				   ltag.c_str (), atag.c_str ());

> +		}

> +	    }

> +	  catch (gdb_exception_error &ex)

> +	    {

> +	      if (ex.error == TARGET_CLOSE_ERROR)

> +		throw;

> +

> +	      fprintf_filtered (gdb_stderr,

> +				_("Could not validate memory tag: %s\n"),

> +				ex.message->c_str ());

> +	    }

>   	}

> +

>         print_value (val, print_opts);

>       }

>   }

> 

> base-commit: 74ace054855321bc5271dd7b354131cd0b71333a

>

Patch

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index ece765b826f..7db3e36d76a 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -730,7 +730,8 @@  typedef std::string (gdbarch_memtag_to_string_ftype) (struct gdbarch *gdbarch, s
 extern std::string gdbarch_memtag_to_string (struct gdbarch *gdbarch, struct value *tag);
 extern void set_gdbarch_memtag_to_string (struct gdbarch *gdbarch, gdbarch_memtag_to_string_ftype *memtag_to_string);
 
-/* Return true if ADDRESS contains a tag and false otherwise. */
+/* Return true if ADDRESS contains a tag and false otherwise.  ADDRESS
+   must be either a pointer or a reference type. */
 
 typedef bool (gdbarch_tagged_address_p_ftype) (struct gdbarch *gdbarch, struct value *address);
 extern bool gdbarch_tagged_address_p (struct gdbarch *gdbarch, struct value *address);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index d9332c2103e..9bc9de91c30 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -608,7 +608,8 @@  v;int;significant_addr_bit;;;;;;0
 # Return a string representation of the memory tag TAG.
 m;std::string;memtag_to_string;struct value *tag;tag;;default_memtag_to_string;;0
 
-# Return true if ADDRESS contains a tag and false otherwise.
+# Return true if ADDRESS contains a tag and false otherwise.  ADDRESS
+# must be either a pointer or a reference type.
 m;bool;tagged_address_p;struct value *address;address;;default_tagged_address_p;;0
 
 # Return true if the tag from ADDRESS matches the memory tag for that
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 3cd42f817f5..416b87c69c6 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1266,19 +1266,26 @@  print_value (value *val, const value_print_options &opts)
 static bool
 should_validate_memtags (struct value *value)
 {
-  if (target_supports_memory_tagging ()
-      && gdbarch_tagged_address_p (target_gdbarch (), value))
-    {
-      gdb_assert (value != nullptr && value_type (value) != nullptr);
+  gdb_assert (value != nullptr && value_type (value) != nullptr);
 
-      enum type_code code = value_type (value)->code ();
+  if (!target_supports_memory_tagging ())
+    return false;
 
-      return (code == TYPE_CODE_PTR
-	      || code == TYPE_CODE_REF
-	      || code == TYPE_CODE_METHODPTR
-	      || code == TYPE_CODE_MEMBERPTR);
-    }
-  return false;
+  enum type_code code = value_type (value)->code ();
+
+  /* Skip non-address values.  */
+  if (code != TYPE_CODE_PTR
+      && !TYPE_IS_REFERENCE (value_type (value)))
+    return false;
+
+  /* OK, we have an address value.  Check we have a complete value we
+     can extract.  */
+  if (value_optimized_out (value)
+      || !value_entirely_available (value))
+    return false;
+
+  /* We do.  Check whether it includes any tags.  */
+  return gdbarch_tagged_address_p (target_gdbarch (), value);
 }
 
 /* Helper for parsing arguments for print_command_1.  */
@@ -1321,26 +1328,42 @@  print_command_1 (const char *args, int voidprint)
 		    value_type (val)->code () != TYPE_CODE_VOID))
     {
       /* If memory tagging validation is on, check if the tag is valid.  */
-      if (print_opts.memory_tag_violations && should_validate_memtags (val)
-	  && !gdbarch_memtag_matches_p (target_gdbarch (), val))
+      if (print_opts.memory_tag_violations)
 	{
-	  /* Fetch the logical tag.  */
-	  struct value *tag
-	    = gdbarch_get_memtag (target_gdbarch (), val,
-				  memtag_type::logical);
-	  std::string ltag
-	    = gdbarch_memtag_to_string (target_gdbarch (), tag);
-
-	  /* Fetch the allocation tag.  */
-	  tag = gdbarch_get_memtag (target_gdbarch (), val,
-				    memtag_type::allocation);
-	  std::string atag
-	    = gdbarch_memtag_to_string (target_gdbarch (), tag);
-
-	  printf_filtered (_("Logical tag (%s) does not match the "
-			     "allocation tag (%s).\n"),
-			   ltag.c_str (), atag.c_str ());
+	  try
+	    {
+	      if (should_validate_memtags (val)
+		  && !gdbarch_memtag_matches_p (target_gdbarch (), val))
+		{
+		  /* Fetch the logical tag.  */
+		  struct value *tag
+		    = gdbarch_get_memtag (target_gdbarch (), val,
+					  memtag_type::logical);
+		  std::string ltag
+		    = gdbarch_memtag_to_string (target_gdbarch (), tag);
+
+		  /* Fetch the allocation tag.  */
+		  tag = gdbarch_get_memtag (target_gdbarch (), val,
+					    memtag_type::allocation);
+		  std::string atag
+		    = gdbarch_memtag_to_string (target_gdbarch (), tag);
+
+		  printf_filtered (_("Logical tag (%s) does not match the "
+				     "allocation tag (%s).\n"),
+				   ltag.c_str (), atag.c_str ());
+		}
+	    }
+	  catch (gdb_exception_error &ex)
+	    {
+	      if (ex.error == TARGET_CLOSE_ERROR)
+		throw;
+
+	      fprintf_filtered (gdb_stderr,
+				_("Could not validate memory tag: %s\n"),
+				ex.message->c_str ());
+	    }
 	}
+
       print_value (val, print_opts);
     }
 }