[foreign-memaccess+abi] RFR: 8301228: Add more ways to resize zero-length memory segments

[Jorn Vernee]

On Thu, 26 Jan 2023 12:53:15 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> When dealing with native libraries, it is not uncommon to obtain zero-length memory segments. Since the size of these segment is zero (for safety reasons), clients need to be able to resize these segments.
> 
> At the time of writing, there are two ways to resize zero-length memory segment:
> 
> * By calling `MemorySegment.ofAddress`, and obtaining a new segment with desired base address, size and lifetime.
> * By using an "unbounded" layout (e.g. OfAddress.asUnbounded()) in order to perform address dereference.
> 
> Both approaches can be improved. First, while `MemorySegment.ofAddress` is a good primitive to create truly custom native segments, sometimes the only thing a client wants to do is to quickly be able to resize the segment to the desired size.
> 
> Secondly, while unbounded address layouts are useful, there are cases where the size of the dereferenced segment is known statically. It is a bit sad that the API does not let clients to specify a _bounded_ size to be specified for a given address layout (as that would lead to safer code). Of course, in some cases the size would still be unknonw statically, so there has to be a way to go unbounded, if needed. 
> 
> This patch fixes rectifies the situation in two ways:
> 
> * by adding a method, namely `MemorySegment::asUnbounded()` which allows to obtain a view of a native segment with maximal size (i.e. `Long.MAX_VALUE`). Note that clients can first use this method, and then use regular slicing methods to further restrict the size, as required.
> * by replacing the `OfAddress::asUnbounded()` method with a more targeted `OfAddress::withTargetLayout(MemoryLayout)`. Note that the unbounded behavior can still be obtained by passing something like `MemoryLayout.sequenceLayout(JAVA_BYTE)`.
> 
> When working on the patch we have considered other options, such as that of adding unsafe slicing methods which can be thought of as the composition of `asUnbounded` with some other (safe) slicing method. And, whether or not to keep `OfAddress::asUnbounded` (as a shortcut).
> 
> I would like to start from the more principled approach in this patch. Since the methods we are talking about are all restricted, there is a certain appeal in trying to keep their number limited. And, duplicating all the slicing methods into new restricted variants can increase the footprint of the MemorySegment API for a relatively little gain. That said, other overloads can always be added as required, at a later point. 
> 
> Note that this patch also adds more safe slicing variants, namely:
> 
> * asSlice(long offset, MemoryLayout)
> * asSlice(long offset, long newSize, long byteAlignment)
> 
> The former is very handy when resizing an unbounded segment to a given layout (e.g. JAVA_INT). Since it takes a layout parameter, it makes sense to check if the resulting slice conforms to the alignment constraint specified by the memory layout. The second method is basically the "real" slicing primitive - which does resizing, offset and alignment check. All other slicing methods can be explained in terms of this.
> 
> Interestingly, thanks to the new slicing methods, we can now more easily deal with alignment checks from `slicingAllocator` and `prefixAllocator`.
> 
> Another note: for the time being, `MemorySegment::asUnbounded` will only work on native segments. While the implementation could be easily made total, I'm a bit skeptical of allowing out-of-bound access from a Java array :-)

src/java.base/share/classes/java/lang/foreign/MemorySegment.java line 571:

> 569:      * @throws IllegalCallerException if access to this method occurs from a module {@code M} and the command line option
> 570:      * {@code --enable-native-access} is specified, but does not mention the module name {@code M}, or
> 571:      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.

I thought we'd switched this to some simplified text that doesn't mention the `--enable-native-access` flag. (e.g. such as on `Linker::nativeLinker`: `@throws IllegalCallerException If the caller is in a module that does not have native access enabled.`)

src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 433:

> 431:          * ValueLayout.OfAddress unboundedLayout = addressLayout.withTargetLayout(
> 432:          *         MemoryLayout.sequenceLayout(ValueLayout.JAVA_BYTE));
> 433:          * }

Nice!

src/java.base/share/classes/jdk/internal/foreign/abi/riscv64/linux/LinuxRISCV64CallArranger.java line 399:

> 397:                     VMStorage storage = storageCalculator.getStorage(StorageType.INTEGER);
> 398:                     bindings.vmLoad(storage, long.class)
> 399:                             .boxAddress(layout);

This should still be `boxAddressRaw` (as for the other CallArrangers). This is a pointer passed as an argument to an upcall, or as a result from a downcall, which should not be tied to the call's scope (for downcalls that would mean it is instantly out of scope, though in BindingSpecializer, we only look at `needsScope`  for arguments, so it doesn't matter for downcalls atm).

test/jdk/java/foreign/TestAddressDereference.java line 27:

> 25:  * @test
> 26:  * @enablePreview
> 27:  * @requires os.arch=="amd64" | os.arch=="x86_64" | os.arch=="aarch64" | os.arch=="riscv64"

AFAIK this requires also needs a check for `sun.arch.data.model == "64"` as the linker doesn't work on 32-bit VMs. (could still be running a 32-bit VM on an amd64 machine)

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

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