Re: [patch] Handle return small struct in rs600 (size is not 4/8)

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> X-SWARE-Spam-Status: No, hits=-2.3 required=5.0	tests=AWL,BAYES_00,RP_MATCHES_RCVD,TW_CP,TW_EG
> Date: Mon, 15 Aug 2011 23:22:17 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> Hi,
> It looks to me that ppc-sysv-tdep.c:do_ppc_sysv_return_value doesn't
> consider the case that returning a small struct (size <= 8) whose size
> is not 4 or 8.
> 
> Supposing we have a struct defined as below,
> 
> struct C
> {char c1; char c2; char c3;};
> struct C c;
> c.c1 = 'a'; c.c2 = 'b'; c.c3 = 'c';
> 
> The raw memory content of c is 0x616263XX (big-endian) or 0xXX636261
> (little-endian).  When returning c, according to Power Arch ABI:
> "Aggregates or unions whose size is less than or equal to eight bytes
> shall be returned in r3 and r4, as if they were first stored in memory
> area and then the low-addressed word were loaded in r3 and the
> high-addressed word were loaded into r4.", the content of r3 should be
> 0x616263 (big-endian) or 0x636261 (little-endian).

That's not how I read the ABI.  If you store that struct in a
zero-initialized 8-byte buffer you'll have the following sequence of 8
bytes:

0x61 0x62 0x63 0x00 0x00 0x00 0x00 0x00

Viewed as two big-endian words this becomes:

0x61626300 0x00000000

and as two little-endian words this becomes:

0x00636261 0x00000000

So in the little-endian case r3 will indeed be 0x636261 like you say,
but in the big-endian case r3 will be 0x61626300.

> When gdb reads r3's content via regcache_cooked_read into a buf, the
> content of buf looks like this,
>            buf:  [0] [1] [2] [3]
> big-endian    :  00  61  62  63
> little-endian :  61  62  63  00

If that's really what you're seeing, then GCC must not implement this
part of the ABI correctly.  Not really surprising since GCC has a long
history of getting corner cases like this wrong.

Now the question is if this just happens to be broken in the
particular version of GCC you're using or whether this has always been
broken.  Eh, wait a moment...

...what version of GDB are you looking at?  The current code in
ppc-sysv-tdep.c already handles the broken way GCC implements this.
Just make sure your target uses ppc_sysv_abi_broken_return_value()
instead of ppc_sysv_abi_return_value().  The NetBSD/powerpc and
OpenBSD/powerpc targets already do this.  Guessing that you're on
Linux and that GCC is the primary compiler on that platform, it
probably needs that same treatment.

> Regression tested on a powerpc variant board.  Many fails in
> gdb.base/structs.exp are fixed.  Is this patch OK?

So no, I'd say this isn't ok.