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

[John Baldwin <jhb@FreeBSD.org>]

On 5/28/21 12:18 PM, Luis Machado wrote:
> On 5/28/21 4:08 PM, John Baldwin wrote:
>> On 5/18/21 1:20 PM, Luis Machado via Gdb-patches wrote:
>>> When the architecture supports memory tagging, we handle pointer types
>>> in a special way, so we can validate tags and show mismatches.
>>> @@ -1266,18 +1266,22 @@ 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 ();
>>> +  enum type_code code = value_type (value)->code ();
>>> -      return (code == TYPE_CODE_PTR
>>> -          || code == TYPE_CODE_REF
>>> -          || code == TYPE_CODE_METHODPTR
>>> -          || code == TYPE_CODE_MEMBERPTR);
>>> +  try
>>> +    {
>>> +      if (code == TYPE_CODE_PTR
>>> +      && target_supports_memory_tagging ()
>>> +      && gdbarch_tagged_address_p (target_gdbarch (), value))
>>> +    return true;
>>
>> Hmm, so why only TYPE_CODE_PTR in the new code but TYPE_CODE_REF and
>> method/member pointers in the old?
>>
> 
> My assessment of the situation wasn't entirely correct (too optimistic).
> Trying to handle all of these types in such a high level function is
> very complex and very error-prone. This is, in part, due to how GDB
> bends the handling of some types in order to show the data the user
> wants. Even the type-handling functions run into some conversion issues
> sometimes.
> 
> The member pointers, more specifically, are not even regular pointers,
> but a TYPE_CODE_INT of 16 bytes. Trying to interpret that as a pointer
> so we can do memory tag operations runs into conversion limitations of
> GDB's type-casting system.
> 
> Handling only TYPE_CODE_PTR is guaranteed to have the desired effect,
> since we want to be able to show the pointer's logical tag and also
> potentially want to set the allocation tag for the memory tag granule
> that pointer points to. A value of type TYPE_CODE_PTR will always have a
> valid value_address () return.
> 
> We can handle all the other types in the default way. There is obviously
> room for improvement in terms of what types we should/should not handle,
> but right now handling TYPE_CODE_PTR gives very useful results for the
> most common use cases.

Ok, sounds good to me.  That was the only bit that didn't seem obvious to
me when reading the diff.

-- 
John Baldwin