Re: [PATCH] [AArch64] Fix PR gdb/28681

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 1/5/22 9:54 AM, Andrew Burgess wrote:
> * Luis Machado <luis.machado@linaro.org> [2022-01-04 14:22:54 -0300]:
> 
>> This is the same as commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608, but
>> fixing things for AArch64.
>>
>> With the patch, gdb.cp/non-trivial-retval.exp has full passes on
>> AArch64-Linux Ubuntu 20.04/18.04.
>> ---
>>   gdb/aarch64-tdep.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 70fb66954a4..802762f303c 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -2323,6 +2323,15 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>>   	  valbuf += X_REGISTER_SIZE;
>>   	}
>>       }
>> +  else if (!language_pass_by_reference (type).trivially_copyable)
>> +    {
>> +      /* If the object is a non-trivial C++ object, the result is passed as a
>> +	 pointer stored in X0.  */
>> +      CORE_ADDR addr;
>> +
>> +      regs->cooked_read (AARCH64_X0_REGNUM, &addr);
>> +      read_memory (addr, valbuf, TYPE_LENGTH (type));
>> +    }
>>     else
>>       {
>>         /* For a structure or union the behaviour is as if the value had
> 
> The aarch64_extract_return_value function is called exclusively from
> aarch64_return_value.
> 
> After calling this function the aarch64_return_value function returns
> RETURN_VALUE_REGISTER_CONVENTION, which I don't think is correct in
> the above case.
> 
> I think we should be returning one of either
> RETURN_VALUE_STRUCT_CONVENTION, RETURN_VALUE_ABI_RETURNS_ADDRESS or
> RETURN_VALUE_ABI_PRESERVES_ADDRESS.

Yes, it seems that way.

The original return convention does not cover this particular case. The 
current code may actually be incorrect, and 
RETURN_VALUE_ABI_RETURNS_ADDRESS seems to be a more correct option.

> 
> I wonder if aarch64_return_in_memory should be doing more of the work
> in this case?  But that's just a thought, I'm sure whatever you come
> up with will be fine, so long as the return type is correct.

Expanding aarch64_return_in_memory to check for non-trivially-copyable 
objects and filling in the readbuf with the right value should do it as 
well.

I have an updated patch for this.

Thanks,
Luis