Re: [RFA] sh-tdep.c (sh_use_struct_convention): Restructure and fix

Andrew Cagney
2003-10-06T16:31:38Z

> On Oct 4, 1:35pm, Andrew Cagney wrote:
>
>
>> >> See: http://sources.redhat.com/ml/gdb-patches/2003-10/msg00033.html.
>> >> The ppc64_sysv_return_value code in ppc-sysv-tdep.c, has been written in
>> >> a way that allows a quick update to this new iterface.
>
>> >
>> > Andrew,
>> >
>> > There are pros and cons to the approach that you used in
>> > ppc64_sysv_abi_return_value().
>> >
>> > On the pro side - and this is definitely a good thing - you keep the
>> > struct convention information together with the implementation of how
>> > to return a value.
>
>>
>> It is stronger than that, it moves all of the ABIs struct convention
>> logic to one place.
>
>
> Huh? What other parts are there? (I fail to see why it's stronger
> than what I stated.)

See my reply to corinna. The core functions set_return_value and
using_struct_return both make assumptions about the ABIs return-value
conventioins. In particular, set_return_value doesn't allow the return
of any structs, even when the ABI allows it. value_being_returned was
similar, until I restructured it.

>> At present key parts of the logic are scattered
>> inconsistently across core parts of GDB
>
>
> Please explain. How did the (traditional) ppc struct return
> mechanisms get scattered across core parts of GDB?

See above.

>> The result is a longer but more correct function.
>
>
> More correct? There is absolutely nothing which prevents a
> traditional "use_struct_convention" function from being correct.
> What's "more correct" than "correct"?

See above.

Of the existing architectures only sparc and m68hc11 thought to
implement RETURN_VALUE_ON_STACK. This is worrying since I don't believe
it to be possible to implement a typical modern ABI without either:

- splitting the logic between RETURN_VALUE_ON_STACK and
USE_STRUCT_CONVENTION (or making RETURN_VALUE_ON_STACK return 1).

- implementing the logic once in return_value

Ex: The PPC64's char array, and float/complex conventions. I'd expect
the SH ABI (assuming that someone can find it?) to have similar edge cases.

> I'll grant you that it may be easier to verify correctness by having
> the struct convention code placed side by side with the value
> (extract/store) return code.
>
> And then again, it might not. I can certainly envision an
> architecture which has a very simple struct return convention, but for
> which it's immensely complicated to implement the mechanics of that
> convention. (E.g, maybe a lot of twiddling is extract/store parts of
> the code to get all the bits in the correct places.) In such a case,
> having an easy to read "use_struct_convention" function would make it
> easier to verify with respect to an ABI doc.

Per my comment below, if this were an OO language it would have been
implemented differently. "return_value" would likely return an object
that contained return-value read and write methods (While I'm still
tempted to do this, I think doing it would be seriously overengineering
the interface). Also, for the case you describe, it could easily
written as:

if (value in register)
if (inval)
extract_return_value ()
if (outval)
store_return_value ()
return RETURN_VALUE_REGISTER_CONVENTION;
else
return RETURN_VALUE_STRUCT_CONVENTION;

>> > But this is also a con because you've spread the definition of
>> > "use_struct_convention" out over a much larger number of lines. It
>> > isn't (IMO) as easy to comprehend when arranged in this way.
>
>>
>> As my spec for the interface points out, if this were an OO language it
>> would have a different interface.
>>
>
>> > The jury is still out (at least as far as I'm concerned) as to
>> > which approach is better. I do happen to think that your approach
>> > is better for ppc64 (and ppc too), but this may not necessarily be the
>> > case for other architectures.
>
>>
>> This is a technical problem. The return_value patch fixes the problem
>> of GDB not being able to handle all cases of:
>>
>> return return-value-in-register
>>
>> correctly. If Corinna instead implements use_struct_convention, then
>> that bug won't be fixed.
>
>
> You're referring to your WIP patch, right? (Or did I miss a commit?)
> You could certainly arrange to have you're WIP patch use the current
> interfaces, couldn't you? If you did, then that bug would be fixed
> even with the traditional use_struct_convention code.

No. That isn't possible. See:

> This also finally makes it possible to fix:
> http://sources.redhat.com/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gdb&pr=659
> along with many many other cases where GDB was incorrectly claiming that it wasn't able to find a return value.

the current store_return_value interface is _not_ required to handle the
storing of small struct conventions in registers. The new interface is.

Andrew