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

[Luis Machado]

On 7/1/21 4:52 PM, Pedro Alves wrote:
> On 2021-06-22 2:37 a.m., Luis Machado via Gdb-patches wrote:
> 
>> 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.  */
> 
> This seems like a too-large hammer to me.  I think you should check whether the value
> is optimized out explicitly instead.
> 
>       if (value_optimized_out (address)
>          || !value_entirely_available (address))
>         return false;
> 
> ... and you can do that in common code somewhere so that archs don't need to worry about it.

It's not just an optimized out case. If we have a 128-bit pointer, for 
example, GDB's conversion machinery will refuse to convert that value (I 
think GDB is hardcoded to handle at most 64-bit pointers). I think this 
throws from unpack_long, hence why I decided to guard this conversion call.

The optimized out case is one example of how this can throw.

> 
> And then, I think you should make the higher level code in printcmd.c try/catch as you
> were doing in the original patch, but make it print the error and continue with normal
> printing.

I don't think that would be desirable. Since we're dealing with a print 
command, we want to fail gracefully and silently (when possible) so we 
don't disturb the user experience of attempting to print some 
expression. We only want to report non-recoverable errors, like a 
dropped remote connection, file not found etc.

> 
>     warning/exception_fprintf (_("could not validate tag: %s", ex.message->c_str ());
> 
> I don't think you should guarantee that the hook doesn't throw an error.  After all,
> you can always get a "remote target closed" exception or a "file not found" error
> trying to read the tags.

It is true. There is no guarantee it won't throw from reading a remote 
file right now. I'll update this comment. We only want this hook to 
throw for non-recoverable errors.

> 
> Pedro Alves
>