[foreign-abi] RFR 8227718: Add support for SystemABI

[Nick Gasson]

Hi Maurizio,

I tried this on AArch64, looks good. Couple of minor things:

* Found one conflict in JavaLangAccess.java when I applied the patch on
  top of the foreign-abi branch. Looks like you need to rebase after
  "8227587: Add internal privileged System.loadLibrary" which also added
  a method at the end of this class.

* I got an error compiling CallGeneratorHelper.java:

/home/nicgas01/panama/test/jdk/java/foreign/CallGeneratorHelper.java:389:
error: cannot find symbol
                value = ForeignUnsafe.getOffset((MemoryAddress)value);
                                     ^
  symbol:   method getOffset(MemoryAddress)
  location: class ForeignUnsafe

  I think this should be ForeignUnsafe.getUnsafeOffset?

* There were two test failures caused by the AArch64ABI boxValue()
  method writing off the end of the destination buffer. When a struct is
  passed in a series of registers boxValue would always copy some
  multiple of the storage size (64 bits) but this is wrong if the struct
  is e.g. 12 bytes. The change below fixes that:

--- a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64ABI.java
+++ b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64ABI.java
@@ -220,8 +220,10 @@ public class AArch64ABI implements SystemABI {
                      for (ArgumentBinding binding : bindings) {
                         MemoryAddress dst = rtmp.offset(binding.offset());
+                        long remainBytes = layout.byteSize() - binding.offset();
                         MemoryAddress.copy(srcPtrFunc.apply(binding), dst,
-                                     binding.storage().getSize());
+                                           Math.min(binding.storage().getSize(),
+                                                    remainBytes));
                     }
                 } else {
                     // Struct is passed by pointer


With these fixed the jdk_foreign jtreg group passes.

Thanks,
Nick


> Hi,
> this patch adds initial support for SystemABI low-level interface in a
> new branch, namely [foreign-abi]. This is the second step in the Panama
> ladder, as per the plan outlined in [1].
>
> http://cr.openjdk.java.net/~mcimadamore/panama/8227718_v2/
>
> So, the goal of this patch is essentially to backport the SystemABI
> interface available in the foreign branch [2] and make it available 'on
> top' of the foreign memory access API. As such, as part of this work I
> had to remove references to various abstractions that are defined in the
> high level C interop API, such as Scope, Pointer and LayoutType and
> replace them with what's available at a lower level.
>
> The result is good and I was even able to port most of the existing
> stress test (stdlib test, combinatorial upcall/downcall tests) on top of
> the lower level ABI implementation.
>
> In the backend we will still find two invokers - direct invoker, used in
> x64/Arch64 and the universal invoker, used as a fallback on all systems
> (and the only backend available for WIn64). The long term plan, as
> already mentioned, is to replace these backends with the more portable
> one being developed in the 'linkToNative' branch.
>
> While most of the port was smooth, I encountered some issues:
>
> * There's no way to test if a layout is a padding layout - to do this I
> added a quick and dirty helper method on the Util class which uses a
> reflection-based test. This is obviously not ideal. I think a better,
> long term move would be to move the implementation of _all_ layout
> classes outside of the public package jdk.incubating.foreign, so that we
> can add more helper methods that are not exposed outside (this is what
> we do for MemorySegment and MemoryAddress, whose impl is defined elsewhere).
>
> * There's no support for varargs call - while varargs method can be
> called, by providing a fully specialized signature (methodtype/function
> descriptor), calling a vararg method using a Java vararg is not
> supported. As pointed out in [3], using Java varargs to mimic C varargs
> is lossy - while it sort of works ok for downcalls (albeit with poor
> performances), completely breaks apart when it comes to upcalls. So,
> some other solution is required there; or, we could say that the only
> way to support varargs is to always instantiate the signature (albeit
> there are also problems there, especially for Windows ABI, which require
> vararg arguments to be passed differently, so they will have to be
> marked somehow, as discussed in [4]).
>
> * There's also no support for exotic ABI types such as x87 or complex
> types. While that's good for now, we obviously need some kind of robust
> story to get there. Again this topic is covered in more details here [4].
>
> Some bonus points of this approach:
>
> * using memory segments to model structs is very handy - if a function
> returns a struct by value, and you need to work with it and then discard
> it, you can do:
>
> try (MemorySegment struct = (MemorySegment)func.invokeExact(...)) {
>   //some logic here
> }
>
> And the struct will be freed after the TWR block is closed, as with any
> other segment.
>
> * deallocation of callbacks is also more deterministic - I've tweaked
> the upcall logic to create a 'callback segment' - which, when closed,
> asks the VM to deallocate the corresponding code blob. So, if you have a
> MemoryAddress which is the entry point of the VM code blob, you can
> simply get its segment and then close it; doing so will release any
> memory associated with the callback.
>
> Comments?
>
> Thanks
> Maurizio
>
> P.S.
> The tests pass on both Linux, Windows and Mac, but (as usual) I could
> not test Arm. It would be nice to know if things worked there too.
>
> [1] - http://cr.openjdk.java.net/~mcimadamore/panama/memaccess.html
> [2] -
> hg.openjdk.java.net/panama/dev/file/tip/src/java.base/share/classes/jdk/internal/foreign/abi/SystemABI.java
> [3] - http://cr.openjdk.java.net/~mcimadamore/panama/varargs.html
> [4] -
> https://mail.openjdk.java.net/pipermail/panama-dev/2019-July/005975.html