[PING][PATCH] Fix printing of non pointer types when memory tagging is enabled

[Luis Machado]

On 6/21/21 10:37 PM, Luis Machado wrote:
> Updates on v2:
> 
> - Guard against thrown exceptions in the gdbarch_tagged_address_p hook as
>    opposed to doing it at a higher scope.
> 
> --
> 
> When the architecture supports memory tagging, we handle pointer types
> in a special way, so we can validate tags and show mismatches.
> 
> I noticed some errors while printing composite types, floats, references,
> member functions and other things that implicitly get dereferenced by GDB to
> show the contents rather than the pointer.
> 
> 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 couple problems. The first one is that we try to evaluate the
> expression to print and eventually call value_as_address (...) before making
> sure we have a suitable TYPE_CODE_PTR type. That may throw for some types. We
> fix this by making sure we have a TYPE_CODE_PTR first, and then proceed to
> check if we need to validate tags.
> 
> The second problem is that the evaluation process may naturally throw an
> error.  One such case is when we have an optimized out variable. Thus we
> guard the evaluation path with a try/catch.
> 
> If the evaluation throws, we want to resume the default expression printing
> path instead of erroring out and printing nothing useful.
> 
> This isn't ideal, but GDB does some magic, internally, in order to provide an
> improved user experience. This allows users to print the contents of some types
> without having to use the dereference operator.
> 
> With the 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>
> 
> 	* gdbarch.sh: Updated documentation for gdbarch_tagged_address_p.
> 	* gdbarch.h: Regenerate.
> 	* printcmd.c (should_validate_memtags): Reorder comparisons and only
> 	validate tags for TYPE_CODE_PTR types.
> 	* aarch64-linux-tdep.c (aarch64_linux_tagged_address_p): Guard call
> 	to value_as_address with a try/catch block.
> ---
>   gdb/aarch64-linux-tdep.c | 14 +++++++++++++-
>   gdb/gdbarch.h            |  3 ++-
>   gdb/gdbarch.sh           |  3 ++-
>   gdb/printcmd.c           | 20 ++++++++++----------
>   4 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index e9761ed2189..84ef616ee35 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1559,7 +1559,19 @@ aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, struct value *address)
>   {
>     gdb_assert (address != nullptr);
>   
> -  CORE_ADDR addr = value_as_address (address);
> +  CORE_ADDR addr;
> +
> +  /* value_as_address may throw if, for example, the value is optimized
> +     out.  Make sure we handle that gracefully.  */
> +  try
> +    {
> +      addr = value_as_address (address);
> +    }
> +  catch (const gdb_exception_error &ex)
> +    {
> +      /* Give up and assume the address is untagged.  */
> +      return false;
> +    }
>   
>     /* Remove the top byte for the memory range check.  */
>     addr = address_significant (gdbarch, addr);
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 7157e5596fd..4118e6c37ef 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.  The
> +   implementation of this hook should never throw an exception. */
>   
>   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 43e51341f97..77088228d9a 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.  The
> +# implementation of this hook should never throw an exception.
>   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 22fa5c074d1..cd7d002c503 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -1266,18 +1266,18 @@ 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);
> +  if (value == nullptr)
> +    return false;
>   
> -      enum type_code code = value_type (value)->code ();
> +  enum type_code code = value_type (value)->code ();
> +
> +  /* Only validate memory tags if we have a pointer type and if the address
> +     is within a tagged memory area.  */
> +  if (code == TYPE_CODE_PTR
> +      && target_supports_memory_tagging ()
> +      && gdbarch_tagged_address_p (target_gdbarch (), value))
> +	return true;
>   
> -      return (code == TYPE_CODE_PTR
> -	      || code == TYPE_CODE_REF
> -	      || code == TYPE_CODE_METHODPTR
> -	      || code == TYPE_CODE_MEMBERPTR);
> -    }
>     return false;
>   }
>   
>