On Mon, 8 May 2023 10:31:20 GMT, Aleksey Shipilev <[email protected]> wrote:
>> After thinking a bit, I think I'd prefer this to be addressed in the build
>> system instead. e.g. something like:
>>
>>
>> diff --git a/make/autoconf/lib-ffi.m4 b/make/autoconf/lib-ffi.m4
>> index 0905c3cd225..83de5a4abf7 100644
>> --- a/make/autoconf/lib-ffi.m4
>> +++ b/make/autoconf/lib-ffi.m4
>> @@ -106,6 +106,13 @@ AC_DEFUN_ONCE([LIB_SETUP_LIBFFI],
>> AC_MSG_ERROR([Could not find libffi! $HELP_MSG])
>> fi
>>
>> + if test "x${OPENJDK_TARGET_CPU}" = "xaarch64" && test
>> "x${OPENJDK_TARGET_OS}" = xmacosx; then
>> + # ffi.h checks '#if FFI_GO_CLOSURES' which throws a warning in xcode
>> on aarch64 because the aarch64
>> + # ffitarget.h (included from ffi.h) doesn't explicitly define
>> FFI_GO_CLOSURES (like it does on e.g. x64).
>> + # define it explicitly here to avoid compilation errors
>> + LIBFFI_CFLAGS="$LIBFFI_CFLAGS -DFFI_GO_CLOSURES=0"
>> + fi
>> +
>> AC_MSG_CHECKING([if libffi works])
>> AC_LANG_PUSH(C)
>> OLD_CFLAGS="$CFLAGS"
>>
>>
>> And then remove the workaround from the source code. (`LIBFFI_CFLAGS` is
>> used to build both relevant libraries, and should also be used when a new
>> library is added that needs libffi. So this would avoid a repeat of this
>> issue)
>>
>> Either way, let's thoroughly document the issue this time around, so future
>> editors won't have to guess why this is needed.
>
>> And then remove the workaround from the source code. (`LIBFFI_CFLAGS` is
>> used to build both relevant libraries, and should also be used when a new
>> library is added that needs libffi. So this would avoid a repeat of this
>> issue)
>
> Yes, I added a variant of that fix that explicitly checks for
> `FFI_GO_CLOSURES` definition during configure. (Now I have to find a system
> which actually supports recent FFI to test it...
@shipilev While looking at something else, I noticed that the fallback linker
was not handling variadic arguments correctly (and some of our tests were
linking against the wrong descriptors). On macos/aarch64 with zero/fallback
linker this could cause crashes in some of the java/foreign tests that use
varidadic arguments (since the ABI is different for variadic args).
I have a patch here to fix this:
https://github.com/openjdk/jdk/compare/master...JornVernee:jdk:VAFixes
That helps, but there are still some failing tests when using the fallback
linker on macos/aarch64:
java/foreign/TestUpcallStack.java Failed.
Unexpected exit from test [exit code: 134]
java/foreign/arraystructs/TestArrayStructs.java#interpreted Failed.
Execution failed: `main' threw exception: java.lang.Exception: failures: 14
java/foreign/arraystructs/TestArrayStructs.java#specialized Failed.
Execution failed: `main' threw exception: java.lang.Exception: failures: 14
I think this might be an issue with libffi, but I'm not sure (I'm using version
3.4.2). Note that all of these failures are cases where arguments are being
passed on the stack.
Also, FYI, I'll be on vacation the next to weeks, so I will likely not be
available to reply here.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13827#issuecomment-1546265480
