Re: [RFC] Linux-specific ppc32 ABI

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

> 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