* [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
@ 2014-07-21 18:21 Uros Bizjak
2014-07-25 9:03 ` Uros Bizjak
2014-09-20 10:04 ` Anthony Green
0 siblings, 2 replies; 6+ messages in thread
From: Uros Bizjak @ 2014-07-21 18:21 UTC (permalink / raw)
To: libffi-discuss, gcc-patches
Cc: Anthony Green, Richard Henderson, Ian Lance Taylor
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
Hello!
Attached patch fixes libgo reflect test failure with libffi closures.
The gccgo compiler started to use FFI closures recently; the compiler
passes ffi_type_void for structures with zero members.
ffi_call form src/alpha/ffi.c allows FFI_TYPE_VOID arguments in
non-debug mode through the default: case, but ffi_closure_osf_inner
aborts with this type of argument.
The patch changes the default case in ffi_closure_osf_inner from abort
to FFI_ASSERT, an this way synchronizes argument handling in both
cases.
2014-07-21 Uros Bizjak <ubizjak@gmail.com>
* src/alpha/ffi.c: Do not include stdlib.h.
(ffi_closure_osf_inner) <default>: Use FFI_ASSERT instead of abort.
Patch was tested with libffi testsuite on alphaev6-linux-gnu.
Additionally, the patch fixed reflect test from the libgo testsuite
and go.test/test/recover.go test from the gcc testsuite.
Uros.
[-- Attachment #2: f.diff.txt --]
[-- Type: text/plain, Size: 584 bytes --]
Index: src/alpha/ffi.c
===================================================================
--- src/alpha/ffi.c (revision 212882)
+++ src/alpha/ffi.c (working copy)
@@ -27,7 +27,6 @@
#include <ffi.h>
#include <ffi_common.h>
-#include <stdlib.h>
/* Force FFI_TYPE_LONGDOUBLE to be different than FFI_TYPE_DOUBLE;
all further uses in this file will refer to the 128-bit type. */
@@ -273,7 +272,7 @@ ffi_closure_osf_inner(ffi_closure *closure, void *
break;
default:
- abort ();
+ FFI_ASSERT (0);
}
argn += ALIGN(size, FFI_SIZEOF_ARG) / FFI_SIZEOF_ARG;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
2014-07-21 18:21 [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner Uros Bizjak
@ 2014-07-25 9:03 ` Uros Bizjak
2014-09-20 10:04 ` Anthony Green
1 sibling, 0 replies; 6+ messages in thread
From: Uros Bizjak @ 2014-07-25 9:03 UTC (permalink / raw)
To: libffi-discuss, gcc-patches
Cc: Anthony Green, Richard Henderson, Ian Lance Taylor
On Mon, Jul 21, 2014 at 8:21 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Attached patch fixes libgo reflect test failure with libffi closures.
> The gccgo compiler started to use FFI closures recently; the compiler
> passes ffi_type_void for structures with zero members.
>
> ffi_call form src/alpha/ffi.c allows FFI_TYPE_VOID arguments in
> non-debug mode through the default: case, but ffi_closure_osf_inner
> aborts with this type of argument.
>
> The patch changes the default case in ffi_closure_osf_inner from abort
> to FFI_ASSERT, an this way synchronizes argument handling in both
> cases.
>
> 2014-07-21 Uros Bizjak <ubizjak@gmail.com>
>
> * src/alpha/ffi.c: Do not include stdlib.h.
> (ffi_closure_osf_inner) <default>: Use FFI_ASSERT instead of abort.
>
> Patch was tested with libffi testsuite on alphaev6-linux-gnu.
> Additionally, the patch fixed reflect test from the libgo testsuite
> and go.test/test/recover.go test from the gcc testsuite.
I have installed the patch in gcc mainline SVN as r213049.
Uros.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
2014-07-21 18:21 [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner Uros Bizjak
2014-07-25 9:03 ` Uros Bizjak
@ 2014-09-20 10:04 ` Anthony Green
2014-09-20 18:28 ` Jay
` (2 more replies)
1 sibling, 3 replies; 6+ messages in thread
From: Anthony Green @ 2014-09-20 10:04 UTC (permalink / raw)
To: Uros Bizjak
Cc: libffi-discuss, gcc-patches, Richard Henderson, Ian Lance Taylor
[replying to an ancient post here..]
Uros Bizjak <ubizjak@gmail.com> writes:
> Hello!
>
> Attached patch fixes libgo reflect test failure with libffi closures.
> The gccgo compiler started to use FFI closures recently; the compiler
> passes ffi_type_void for structures with zero members.
Why not just pass an FFI_TYPE_STRUCT with zero members?
> ffi_call form src/alpha/ffi.c allows FFI_TYPE_VOID arguments in
> non-debug mode through the default: case, but ffi_closure_osf_inner
> aborts with this type of argument.
>
> The patch changes the default case in ffi_closure_osf_inner from abort
> to FFI_ASSERT, an this way synchronizes argument handling in both
> cases.
>
> 2014-07-21 Uros Bizjak <ubizjak@gmail.com>
>
> * src/alpha/ffi.c: Do not include stdlib.h.
> (ffi_closure_osf_inner) <default>: Use FFI_ASSERT instead of abort.
>
> Patch was tested with libffi testsuite on alphaev6-linux-gnu.
> Additionally, the patch fixed reflect test from the libgo testsuite
> and go.test/test/recover.go test from the gcc testsuite.
Why not add an FFI_TYPE_VOID case so it doesn't ever abort if that's
expected behaviour? The default case is there to catch unexpected
values.
AG
>
> Uros.
>
> Index: src/alpha/ffi.c
> ===================================================================
> --- src/alpha/ffi.c (revision 212882)
> +++ src/alpha/ffi.c (working copy)
> @@ -27,7 +27,6 @@
>
> #include <ffi.h>
> #include <ffi_common.h>
> -#include <stdlib.h>
>
> /* Force FFI_TYPE_LONGDOUBLE to be different than FFI_TYPE_DOUBLE;
> all further uses in this file will refer to the 128-bit type. */
> @@ -273,7 +272,7 @@ ffi_closure_osf_inner(ffi_closure *closure, void *
> break;
>
> default:
> - abort ();
> + FFI_ASSERT (0);
> }
>
> argn += ALIGN(size, FFI_SIZEOF_ARG) / FFI_SIZEOF_ARG;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
2014-09-20 10:04 ` Anthony Green
@ 2014-09-20 18:28 ` Jay
2014-09-20 22:04 ` Ian Lance Taylor
2014-09-21 9:17 ` Uros Bizjak
2 siblings, 0 replies; 6+ messages in thread
From: Jay @ 2014-09-20 18:28 UTC (permalink / raw)
To: Anthony Green
Cc: Uros Bizjak, <libffi-discuss@sourceware.org>,
gcc-patches, Richard Henderson, Ian Lance Taylor
On Sep 20, 2014, at 3:04 AM, Anthony Green <green@moxielogic.com> wrote:
>
> Why not just pass an FFI_TYPE_STRUCT with zero members?
My information may be old or irrelevant but I have used structs with no members with gcc backend, but with nonzero size and alignment, and ran into backend problems, particularly on sparc64, passing them as parameters.
Is that what is being used here?
Maybe best to add some members to achieve equivalent size/alignment?
- Jay
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
2014-09-20 10:04 ` Anthony Green
2014-09-20 18:28 ` Jay
@ 2014-09-20 22:04 ` Ian Lance Taylor
2014-09-21 9:17 ` Uros Bizjak
2 siblings, 0 replies; 6+ messages in thread
From: Ian Lance Taylor @ 2014-09-20 22:04 UTC (permalink / raw)
To: Anthony Green; +Cc: Uros Bizjak, libffi-discuss, gcc-patches, Richard Henderson
On Sat, Sep 20, 2014 at 3:04 AM, Anthony Green <green@moxielogic.com> wrote:
>
>> Attached patch fixes libgo reflect test failure with libffi closures.
>> The gccgo compiler started to use FFI closures recently; the compiler
>> passes ffi_type_void for structures with zero members.
>
> Why not just pass an FFI_TYPE_STRUCT with zero members?
Because when an empty struct is used as a return type libffi returns
a failure from ffi_prep_cif_core:
/* Initialize the return type if necessary */
if ((cif->rtype->size == 0) && (initialize_aggregate(cif->rtype) != FFI_OK))
return FFI_BAD_TYPEDEF;
This is because initialize_aggregate fails:
if (UNLIKELY(arg == NULL || arg->elements == NULL))
return FFI_BAD_TYPEDEF;
I haven't looked farther.
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner
2014-09-20 10:04 ` Anthony Green
2014-09-20 18:28 ` Jay
2014-09-20 22:04 ` Ian Lance Taylor
@ 2014-09-21 9:17 ` Uros Bizjak
2 siblings, 0 replies; 6+ messages in thread
From: Uros Bizjak @ 2014-09-21 9:17 UTC (permalink / raw)
To: Anthony Green
Cc: libffi-discuss, gcc-patches, Richard Henderson, Ian Lance Taylor
On Sat, Sep 20, 2014 at 12:04 PM, Anthony Green <green@moxielogic.com> wrote:
>
> [replying to an ancient post here..]
>
> Uros Bizjak <ubizjak@gmail.com> writes:
>
>> Hello!
>>
>> Attached patch fixes libgo reflect test failure with libffi closures.
>> The gccgo compiler started to use FFI closures recently; the compiler
>> passes ffi_type_void for structures with zero members.
>
> Why not just pass an FFI_TYPE_STRUCT with zero members?
>
>> ffi_call form src/alpha/ffi.c allows FFI_TYPE_VOID arguments in
>> non-debug mode through the default: case, but ffi_closure_osf_inner
>> aborts with this type of argument.
>>
>> The patch changes the default case in ffi_closure_osf_inner from abort
>> to FFI_ASSERT, an this way synchronizes argument handling in both
>> cases.
>>
>> 2014-07-21 Uros Bizjak <ubizjak@gmail.com>
>>
>> * src/alpha/ffi.c: Do not include stdlib.h.
>> (ffi_closure_osf_inner) <default>: Use FFI_ASSERT instead of abort.
>>
>> Patch was tested with libffi testsuite on alphaev6-linux-gnu.
>> Additionally, the patch fixed reflect test from the libgo testsuite
>> and go.test/test/recover.go test from the gcc testsuite.
>
> Why not add an FFI_TYPE_VOID case so it doesn't ever abort if that's
> expected behaviour? The default case is there to catch unexpected
> values.
The patch just equalizes handling of unknown arguments to ffi_call.
Also, the approach is the same as for x86_64 and i386 - these targets
doesn't abort for unknown arguments. There is no problem in adding
handling of FFI_TYPE_VOID to both functions, if this is the preferred
solution.
Uros.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-21 9:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21 18:21 [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner Uros Bizjak
2014-07-25 9:03 ` Uros Bizjak
2014-09-20 10:04 ` Anthony Green
2014-09-20 18:28 ` Jay
2014-09-20 22:04 ` Ian Lance Taylor
2014-09-21 9:17 ` Uros Bizjak
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).