RFR: 8307527: MacOS Zero builds fail with undefined FFI_GO_CLOSURES after JDK-8304265

[Jorn Vernee]

On Mon, 8 May 2023 10:31:20 GMT, Aleksey Shipilev <shade at openjdk.org> 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