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

[Maurizio Cimadamore]

On Thu, 26 Jan 2023 13:35:58 GMT, Jorn Vernee <jvernee 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/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?

Perhaps we can leave it - but I'm on the fence, as I think having too many options can also be confusing (esp. given that the options are restricted) - should then we also add slicing methods which just remove spatial bounds? Also, it's probably a bit of a naming issue as well - e.g. if the two methods were telescopic, with same name - but one had no argument and can be explained as calling the other, I'd be more ok with it. But having both `withTargetLayout` *and* `asUnbounded` feels like you can do two very different things, as the names are unrelated.

> 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`.

I have fixed this in the latest iteration (but I left the old name) - as I saw test failures because of this

> 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.

true - will fix

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

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