This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Linux-specific ppc32 ABI
> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Date: Fri, 25 Jan 2008 14:05:55 -0200
> > The SysV ABI says we should promote exceeding float parameters (the ones
> > that should go to the stack) to double, and store then with 8 bytes
> > alignment in the stack.
This makes quite a bit of sense, since it solves problems with
unprototyped functions.
> > GCC (XLC as well) doesn't promote floats to doubles and does not align
> > them to 8 bytes in the stack. Actually, it just aligns the prototyped
> > float parameters to 4 bytes in the stack.
So both compilers are buggy.
> > This is something that needs to be improved in the ABI text.
Well, I suppose you can change it, but I wouldn't call it an
improvement. Who controls the 32-bit PowerPC SystemV ABI these days?
I guess it makes sense for GDB to follow the broken GCC ABI for now,
since it doesn't seem there is a compiler that implements the SysV ABI
correctly.
> Ping?
>
> It would be nice if someone could test this patch in other systems which
> use the SysV ppc32 ABI, to see if they actually implement what is in the
> ABI specification or if they also do what Linux actually does. If the
> former, this patch could go in. If the latter, we could use the
> suggested flag in gdbarch_tdep to restrict this fix only to Linux.
It seems OpenBSD/powerpc is similar to Linux here. Without the patch I get
FAIL: gdb.base/callfuncs.exp: Call function with many float arguments.
which turns into a PASS if a apply the patch. The patch itself isn't
quite right though.
The biggest problem is that variable-sized arrays are not in ISO C90,
so you can't use them; use alloca(3) instead, or simply allocate a
fixed amount that's big enough.
The second problem is that the conversion result of
convert_typed_floating() never actually gets used (the old code had
the same problem). This must mean however, that doing the conversion
is completely redundant, and that the code can be simplified to:
/* Align to 4 bytes or 8 bytes depending on the type of
the argument (float or double). */
argoffset = align_up (argoffset, len);
if (write_pass)
write_memory (sp + argoffset, val, len);
argoffset += len;
I'm also not really happy with the way the comment is phrased.
How about the attached diff?
Index: ppc-sysv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-sysv-tdep.c,v
retrieving revision 1.45
diff -u -p -r1.45 ppc-sysv-tdep.c
--- ppc-sysv-tdep.c 1 Jan 2008 22:53:12 -0000 1.45
+++ ppc-sysv-tdep.c 26 Jan 2008 15:24:36 -0000
@@ -129,17 +129,19 @@ ppc_sysv_abi_push_dummy_call (struct gdb
}
else
{
- /* SysV ABI converts floats to doubles before
- writing them to an 8 byte aligned stack location. */
- argoffset = align_up (argoffset, 8);
+ /* The SysV ABI tells us to convert floats to
+ doubles before writing them to an 8 byte aligned
+ stack location. Unfortunately GCC does not do
+ that, and stores floats without into 4 bytes
+ aligned locations without converting them to
+ doubles. */
+
+ /* Align to 4 bytes or 8 bytes depending on the type of
+ the argument (float or double). */
+ argoffset = align_up (argoffset, len);
if (write_pass)
- {
- char memval[8];
- convert_typed_floating (val, type, memval,
- builtin_type_ieee_double);
write_memory (sp + argoffset, val, len);
- }
- argoffset += 8;
+ argoffset += len;
}
}
else if (TYPE_CODE (type) == TYPE_CODE_FLT