This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] sh-tdep.c (sh_use_struct_convention): Restructure and fix
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