From: Anthony Green <green@moxielogic.com>
To: Mark H Weaver <mhw@netris.org>
Cc: "libffi-discuss@sourceware.org" <libffi-discuss@sourceware.org>
Subject: Re: [PATCH] Fix handling of uint32_t arguments on the MIPS N32 ABI.
Date: Sat, 26 Oct 2013 12:32:00 -0000 [thread overview]
Message-ID: <CACxje5-pskL4MOiHmVzGLv8Nnu1msMty_a0X-pQOk=-dHCSaDA@mail.gmail.com> (raw)
In-Reply-To: <87fvrrzvws.fsf@netris.org>
Thanks Mark. I've committed this change.
https://github.com/atgreen/libffi/commit/d3372c54ce7117e80d389ba875dc5b6b2213c71e
AG
On Wed, Oct 23, 2013 at 2:25 PM, Mark H Weaver <mhw@netris.org> wrote:
> Hello all,
>
> A failing glib test on MIPS N32 has alerted me to a bug in the handling
> of unsigned 32-bit integers in libffi. The MIPSpro N32 ABI Handbook [*]
> states:
>
> "32-bit integer (int) parameters are always sign-extended when passed
> in registers, whether of signed or unsigned type. [This issue does
> not arise in the o32-bit ABI.]" (chapter 2, page 6)
>
> [*] http://techpubs.sgi.com/library/manuals/2000/007-2816-005/pdf/007-2816-005.pdf
>
> and indeed GCC generates code that assumes this.
> For example, GCC 4.7.3 compiles:
>
> --8<---------------cut here---------------start------------->8---
> #include <stdint.h>
> #include <stdlib.h>
>
> void
> do_test (uint32_t a)
> {
> if (a != 4294967253U)
> exit (1);
> }
> --8<---------------cut here---------------end--------------->8---
>
> as follows (with extraneous cruft stripped):
>
> --8<---------------cut here---------------start------------->8---
> do_test:
> li $2,-43 # 0xffffffffffffffd5
> beq $4,$2,.L5
> nop
>
> addiu $sp,$sp,-16
> sd $31,8($sp)
> jal exit
> li $4,1 # 0x1
>
> .L5:
> j $31
> nop
> --8<---------------cut here---------------end--------------->8---
>
> However, the current libffi code zero-extends 32-bit unsigned integers,
> which violates this N32 FFI requirement.
>
> See below for a patch to fix this problem. I've tested this patch on
> the preliminary port of GNU Guix to MIPS N32, which uses a minimally
> patched GNU toolchain targetting mips64el-linux-gnu.
>
> Without this patch, glib-2.38.0 fails its test suite, specifically the
> gobject/test/signal.c test, which uses libffi to pass a large 32-bit
> unsigned integer to a test function that checks the received value.
> With this patch, glib-2.38.0 passes its test suite.
>
> Comments and suggestions welcome.
>
> Regards,
> Mark
>
>
> Signed-off-by: Mark H Weaver <mhw@netris.org>
> ---
> ChangeLog | 5 +++++
> src/mips/ffi.c | 7 +++++++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index eceef84..e7e5d94 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2013-10-23 Mark H Weaver <mhw@netris.org>
> +
> + * src/mips/ffi.c: Fix handling of uint32_t arguments on the
> + MIPS N32 ABI.
> +
> 2013-10-13 Sandra Loosemore <sandra@codesourcery.com>
>
> * README: Add Nios II to table of supported platforms.
> diff --git a/src/mips/ffi.c b/src/mips/ffi.c
> index 03121e3..5d0dd70 100644
> --- a/src/mips/ffi.c
> +++ b/src/mips/ffi.c
> @@ -170,7 +170,14 @@ static void ffi_prep_args(char *stack,
> break;
>
> case FFI_TYPE_UINT32:
> +#ifdef FFI_MIPS_N32
> + /* The N32 ABI requires that 32-bit integers
> + be sign-extended to 64-bits, regardless of
> + whether they are signed or unsigned. */
> + *(ffi_arg *)argp = *(SINT32 *)(* p_argv);
> +#else
> *(ffi_arg *)argp = *(UINT32 *)(* p_argv);
> +#endif
> break;
>
> /* This can only happen with 64bit slots. */
> --
> 1.7.5.4
>
prev parent reply other threads:[~2013-10-26 12:32 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-23 18:27 Mark H Weaver
2013-10-26 12:32 ` Anthony Green [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACxje5-pskL4MOiHmVzGLv8Nnu1msMty_a0X-pQOk=-dHCSaDA@mail.gmail.com' \
--to=green@moxielogic.com \
--cc=libffi-discuss@sourceware.org \
--cc=mhw@netris.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).