[foreign-memaccess+abi] RFR: 8287516: Implement fallback Linker

[Maurizio Cimadamore]

On Tue, 17 Jan 2023 17:32:41 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Implement a [libffi](https://github.com/libffi/libffi) based fallback linker. The fallback linker comes with a native library that can be included in the jdk by configuring with `--enable-fallback-linker`. This requires the libffi library to be available as well. On the zero VM configuration the fallback linker is enabled by default.
> 
> CABI is refactored a bit. It now supports explicitly setting the CABI using the `jdk.internal.foreign.CABI` system property (which is very handy for testing). Instead of falling back to `null` when the platform is not supported, we instead fall back to checking if the fallback library is supported (`FALLBACK`), and if not, default to the new `UNSUPPORTED` enum constant, which is then mapped to an `UnsupportedOperationException` in the Linker::nativeLinker implementation.
> 
> Checking whether the a full port of the foreign linker is available is done through the new jdk.internal.vm.ForeignLinkerSupport class. This follows the precedent set by Loom's jdk.internal.vm.ContinuationSupport class. The implementation simply returns `true` on platforms where we have a port.
> 
> The system lookup is changed to be selected based on the platform, rather than CABI. This allows selecting the right library implementation when running on a zero VM configuration as well. The split is now just between Windows and non-Windows platforms.
> 
> Finally, I had to slightly modify `globalDefinitions_zero.hpp` to conditionally define `FFI_GO_CLOSURES`, since I was getting a redefinition error using GCC. The macro is also defined in `ffitarget.h` which is included by `ffi.h`. The define in globalDefinitions comes from this PR: https://github.com/openjdk/jdk/pull/8195 which indicates that the define is only needed on Mac Os X. So I switched out the guard to check for `__APPLE__` instead. (the check whether it is already defined doesn't really do anything, since `FFI_GO_CLOSURES` is defined by including `ffi.h`).
> 
> The Java implementation is found in the `jdk.internal.foreign.abi.fallback` package, and consists of the following classes:
> - `FallbackLinker`. The main entry point, an instance of `Linker`. Contains the code for arranging and executing calls.
> - `LibFallback`. A thin wrapper around the native `libfallback` library found in the java.base module, which acts as an interface to the libffi library.
> - `FFIType`. Contains code for creating native `ffi_type` struct instances from memory layouts.
> - The `TypeClass`, `FFIStatus`, and `FFIABI` supporting enums, which should be pretty self-explanatory.
> 
> I've ran the `jdk_foreign` tests in the following configurations:
> - Windows.
> - Linux with the fallback linker lib included, but not used.
> - Linux with fallback linker and running with `-Djdk.internal.foreign.CABI=FALLBACK` (this uses the fallback implementation).
> - Linux zero VM (uses `FALLBACK` as CABI by default, because we don't have a port for `zero`).

The code looks good and easy to follow - good job!
I'm a bit worried re. testing: given that libffi is not included in a regular JDK build, we don't have ways to test this in our CI, which seems bad (how do we detect regressions?).

src/java.base/share/classes/jdk/internal/foreign/Utils.java line 219:

> 217:     }
> 218: 
> 219:     public static StructLayout computeStructLayout(MemoryLayout... elements) {

Not too sure about this - at least this being here. The general spirit of Linker classification is that padding bits are inserted as required by the client. Now, it seems you need this inside the fallback FFI, so that you can create the precise layout of a struct where some of the fields might be platform-dependent - in that case I'd say it would be better to move this function over there?

src/java.base/share/classes/jdk/internal/foreign/abi/fallback/TypeClass.java line 31:

> 29: import java.lang.foreign.ValueLayout;
> 30: 
> 31: enum TypeClass {

Is this necessary? The mapping between layouts and FFI types seems pretty 1-1. If we keep this, would it make sense to make it "smarter" by adding the write/read functions in here? (I also understand if you want to keep it as classification token).

-------------

Marked as reviewed by mcimadamore (Committer).

PR: https://git.openjdk.org/panama-foreign/pull/770