From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5634 invoked by alias); 26 Jan 2008 15:47:00 -0000 Received: (qmail 5623 invoked by uid 22791); 26 Jan 2008 15:46:58 -0000 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 26 Jan 2008 15:46:26 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.1/8.14.1) with ESMTP id m0QFZfA2004511; Sat, 26 Jan 2008 16:35:41 +0100 (CET) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.1/8.14.1/Submit) id m0QFZddH026156; Sat, 26 Jan 2008 16:35:39 +0100 (CET) Date: Sat, 26 Jan 2008 18:18:00 -0000 Message-Id: <200801261535.m0QFZddH026156@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: bauerman@br.ibm.com CC: luisgpm@linux.vnet.ibm.com, drow@false.org, brobecker@adacore.com, gdb-patches@sourceware.org In-reply-to: <1201277155.11950.134.camel@localhost.localdomain> (message from Thiago Jung Bauermann on Fri, 25 Jan 2008 14:05:55 -0200) Subject: Re: [RFC] Linux-specific ppc32 ABI References: <1199991624.3343.19.camel@gargoyle> <20080111060629.GC12954@adacore.com> <1200066920.26270.9.camel@gargoyle> <20080111155733.GA3240@caradoc.them.org> <1200086736.26270.35.camel@gargoyle> <1201277155.11950.134.camel@localhost.localdomain> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00624.txt.bz2 > From: Thiago Jung Bauermann > 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