[foreign-memaccess+abi] RFR: 8301360: Add dereference layout paths to memory layout API

[Jorn Vernee]

On Mon, 30 Jan 2023 15:25:54 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> Now that we have a more general way to attach target layouts to address layouts (see https://git.openjdk.org/panama-foreign/pull/775), it might be useful to add support for layout paths which perform dereference of a given address value. Consider this case:
> 
> 
> struct b {
>    int x;
> };
> 
> struct a {
>     struct b *b;
> };
> 
> 
> Here, our goal is to construct a layout path for "a->b.x".
> 
> To do this, we use a *dereference path element* (see `PathElement#dereferenceElement`). These comes in two flavors:
> 
> * no argument factory - this assumes that the address layout being dereferenced has some target layout set;
> * layout argument factory - this assumes that the address layout being dereferenced does *not* have a target layout set (e.g. it is not always possible to associate a target layout, especially in cyclic cases like a -> b -> a).
> 
> When a dereference element is applied to an existing path, a var handle that fetches the address pointed to by the current path is pushed on the stack, and a new layout path is started. This way, dereference path can be used to "chain" together multiple layout paths. All the intermediate memory read operations occur using a *plain* get. For more complex access operation (e.g. if atomic access is required for the whole dereference chain), multiple var handles have to be obtained.
> 
> It is an error to pass a dereference path to other layout operations such as `select`, `byte/bitOffset[Handle]`.

Implementation looks good. I think there's some issue with resizing the segment for the layout accepting overload though (see inline comment).

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 638:

> 636:          * The path element returned by this method does not alter the number of free dimensions of any path
> 637:          * that is combined with such element. Using this path layout to dereference an address layout
> 638:          * that has no target layout results in an {@link IllegalArgumentException} (e.g. when

This IAE should be specified on `varHandle` as well.

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 652:

> 650:          * The path element returned by this method does not alter the number of free dimensions of any path
> 651:          * that is combined with such element. Using this path layout to dereference an address layout
> 652:          * that already has a target layout results in an {@link IllegalArgumentException} (e.g. when

Same here

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 658:

> 656:          * @return a path element which dereferences an address layout.
> 657:          */
> 658:         static PathElement dereferenceElement(MemoryLayout layout) {

This method needs to be restricted, I believe. Since it allows resizing a MemorySegment. The other overload of `dereferenceElement` is already covered because attaching a pointee layout to an address layout is restricted already.

Consider the following:


StructLayout hasAddr = MemoryLayout.structLayout(ADDRESS.withName("addr"));

MemorySegment rawSeg = MemorySegment.allocateNative(0, SegmentScope.auto()); // inaccessible segment
// rawSeg.get(JAVA_INT, 0); // not allowed
MemorySegment addrHolder = MemorySegment.allocateNative(hasAddr, SegmentScope.auto());
addrHolder.set(ADDRESS, 0, rawSeg); // fine
VarHandle derefHandle = hasAddr.varHandle(groupElement("addr"), dereferenceElement(JAVA_INT));
int x = derefHandle.get(addrHolder); // oops!


Assuming I eye-balled this code correctly, I think this is problematic?

src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java line 278:

> 276:     private static LayoutPath derefPath(MemoryLayout layout, MethodHandle handle, LayoutPath encl) {
> 277:         MethodHandle[] handles = new MethodHandle[encl.derefAdapters.length + 1];
> 278:         System.arraycopy(encl.derefAdapters, 0, handles, 0, encl.derefAdapters.length);

This could be simplified using Arrays.copyOf:

Suggestion:

        MethodHandle[] handles = Arrays.copyOf(encl.derefAdapters, encl.derefAdapter.length + 1);


(no need for separate call to System.arraycopy)

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

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