Re: [PATCH] [gdb/exp] Fix cast handling for indirection

[Pedro Alves <pedro@palves.net>]

On 2024-05-04 00:17, Pedro Alves wrote:
> On 2024-05-03 18:30, Kevin Buettner wrote:
>> On Fri, 3 May 2024 17:04:41 +0100
>> Pedro Alves <pedro@palves.net> wrote:
>>
>>>> I was wondering if this patch causes gdb to accept some weird things
>>>> that might have been rejected in the past, by introducing a hidden cast.
>>>> Maybe print (char) *85732 does something surprising now.  I'm not
>>>> entirely sure if that's bad.  
>>>
>>> I am totally surprised that:
>>>
>>>  +# Regression test for PR31693.
>>>  +gdb_test "p (char)*a_loc ()" " = 97 'a'"
>>>
>>> this actually works, instead of telling the user:
>>>
>>>   "'a_loc' has unknown return type; cast the call to its declared return type"
>>>
>>> It seems like a misfeature to me to assume that "char *" is the right type.
>>>
>>> Thus, I don't agree with the patch.
>>
>> Using a GDB built with Tom de Vries's patch, I see:
>>
>> (gdb) p *a_loc()
>> 'a_loc' has unknown return type; cast the call to its declared return type
>>
>> This is the same as the pre-patch behavior.
>>
>> With Tom's patch, GDB now infers the function's return type, based
>> on the cast:
>>
>> (gdb) p (char)*a_loc()
>> $1 = 97 'a'
>>
> 
> But that is not what GDB told you to do.  It told you to cast the _call_,
> not the result of de-referencing the result of the call.  
> It is telling cast to the declared return type, which is "char *".  I.e.,
> it is telling you to write:
> 
>  (gdb) p *(char *)a_loc()
> 
> See 7022349d5c86 ("Stop assuming no-debug-info functions return int").
> 
> This cast here:
> 
>  (gdb) p (char)*a_loc()
> 
> ... should not affect the call's return type.  That is decided before
> the * operator is involved.  In the same way, this:
> 
>  (gdb) p (long long)*a_loc()
> 
> should not result in gdb assuming that a_loc() returns a "long long *",
> that it wrong.  It should still error out with
> 
>   'a_loc' has unknown return type; cast the call to its declared return type
> 
> and so the user should write:
> 
>  (gdb) p (long long) *(char *)a_loc()
> 
> and then with this last expression a proper sign extension is applied when
> char is converted to long long, if char is signed.  I.e., in steps:
> 
>  1: (char *)a_loc() -> call, and get char * return value
>  2: *(char *)a_loc() -> deref, and get char value
>  3: (long long) *(char *)a_loc() -> sign extend char value -> long long value
> 

I kind of emphasized the sign extension part above, but to be clear, with

  (gdb) p (long long)*a_loc()

and gdb assuming that means a_loc returns "long long *", gdb incorrectly reads a
64-bit value off of the pointer address, which is totally bogus and would not
be what gdb would do if it had debug info for a_loc(), in which case GDB would
know that it returns char *, and thus would deref only one byte and behave like
described above in the 1: 2: 3: steps.  The behavior of the expression should not
change like that depending on whether you have debug info.  Thus, GDB should error
out.