[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 :-)

Some initial comments

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

> 362:  * ensures that the obtained segment can be passed, opaquely, to other pointer-accepting foreign functions.
> 363:  * <p>
> 364:  * To access native zero-length memory segments, clients several options, all of which are <em>unsafe</em>.

Suggestion:

 * To access native zero-length memory segments, clients have several options, all of which are <em>unsafe</em>.

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

> 364:  * To access native zero-length memory segments, clients several options, all of which are <em>unsafe</em>.
> 365:  * <p>
> 366:  * First, clients can unsafely resize a zero-length memory segment using an unbounded {@linkplain #asSliceUnbounded(long, long)}

Suggestion:

 * First, clients can unsafely resize a zero-length memory segment using an {@linkplain #asSliceUnbounded(long, long) unbounded}

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

> 375:  *
> 376:  * Alternatively, if the size of the foreign segment is known statically, clients can associate a
> 377:  * {@linkplain OfAddress#withTargetLayout(MemoryLayout) target layout} to the address layout used to obtain the

Suggestion:

 * {@linkplain OfAddress#withTargetLayout(MemoryLayout) target layout} with the address layout used to obtain the

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

> 396:  *
> 397:  * Both {@link #asSliceUnbounded(long, long)}, {@link ValueLayout.OfAddress#withTargetLayout(MemoryLayout)}
> 398:  * and {@link #ofAddress(long, long, SegmentScope)} are

There are more than two variants now:
Suggestion:

 * All of {@link #asSliceUnbounded(long, long)}, {@link ValueLayout.OfAddress#withTargetLayout(MemoryLayout)}
 * and {@link #ofAddress(long, long, SegmentScope)} are

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

> 524:     /**
> 525:      * Returns a slice of this memory segment, at the given offset. The returned segment's address is the address
> 526:      * of this segment plus the given offset; its size is specified by the given argument.

Suggestion:

     * of this segment plus the given offset; its size is specified by the given size argument.

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

> 435:          */
> 436:         @CallerSensitive
> 437:         OfAddress withTargetLayout(MemoryLayout layout);

I think it could still be nice to have `asUnbounded` as a shortcut for `withTargetLayout(sequenceLayout(JAVA_BYTE))`.

WDYT?

src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 351:

> 349: 
> 350:     static BoxAddress boxAddressRaw(long size) {
> 351:         return new BoxAddress(size, 1, false);

Now that we have the full layout, we might want to add an overload of `boxAddressRaw` that takes a pointee layout, and takes the alignment into account as well. (or just change this method to accept an alignment as well). Most of the call sites of this method are passing the result of `Utils.pointeeSize(layout)` (where `layout` is the address layout).

Also, the main difference with `boxAddress` seems to be the lack of scope being used. So, maybe it would be better to name this method `boxAddressNoScope`.

test/jdk/java/foreign/StdLibTest.java line 240:

> 238: 
> 239:             Tm(MemorySegment addr) {
> 240:                 this.base = addr.asSlice(0, LAYOUT);

Shouldn't the right size already be set by the `gmtime` handle now? i.e. I think this `asSlice` call is redundant.

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

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