Re: [PATCH, libffi, alpha]: Use FFI_ASSERT in ffi_closure_osf_inner

[Uros Bizjak <ubizjak at gmail dot com>]

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.