8287244: Add bound check in indexed memory access var handle

Reviewed-by: psandoz, jvernee

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java

@@ -369,6 +369,9 @@ public sealed interface MemoryLayout permits AbstractLayout, SequenceLayout, Gro
      * arguments, whereas {@code c_1}, {@code c_2}, ... {@code c_m} are <em>static</em> offset constants
      * and {@code s_1}, {@code s_2}, ... {@code s_n} are <em>static</em> stride constants which are derived from
      * the layout path.
+     * <p>
+     * Additionally, the provided dynamic values must conform to some bound which is derived from the layout path, that is,
+     * {@code 0 <= x_i < b_i}, where {@code 1 <= i <= n}, or {@link IndexOutOfBoundsException} is thrown.
      *
      * @apiNote the resulting var handle will feature an additional {@code long} access coordinate for every
      * unspecified sequence access component contained in this layout path. Moreover, the resulting var handle
@@ -515,6 +518,7 @@ public sealed interface MemoryLayout permits AbstractLayout, SequenceLayout, Gro
          * Returns a path element which selects the element layout in a <em>range</em> of positions in a sequence layout.
          * The range is expressed as a pair of starting index (inclusive) {@code S} and step factor (which can also be negative)
          * {@code F}.
+         * <p>
          * If a path with free dimensions {@code n} is combined with the path element returned by this method,
          * the number of free dimensions of the resulting path will be {@code 1 + n}. If the free dimension associated
          * with this path is bound by an index {@code I}, the resulting accessed offset can be obtained with the following
@@ -525,6 +529,14 @@ public sealed interface MemoryLayout permits AbstractLayout, SequenceLayout, Gro
          * }</pre></blockquote>
          *
          * where {@code E} is the size (in bytes) of the sequence element layout.
+         * <p>
+         * Additionally, if {@code C} is the sequence element count, it follows that {@code 0 <= I < B},
+         * where {@code B} is computed as follows:
+         *
+         * <ul>
+         *    <li>if {@code F > 0}, then {@code B = ceilDiv(C - S, F)}</li>
+         *    <li>if {@code F < 0}, then {@code B = ceilDiv(-(S + 1), -F)}</li>
+         * </ul>
          *
          * @param start the index of the first sequence element to be selected.
          * @param step the step factor at which subsequence sequence elements are to be selected.
@@ -544,8 +556,19 @@ public sealed interface MemoryLayout permits AbstractLayout, SequenceLayout, Gro
 
         /**
          * Returns a path element which selects an unspecified element layout in a sequence layout.
+         * <p>
          * If a path with free dimensions {@code n} is combined with the path element returned by this method,
-         * the number of free dimensions of the resulting path will be {@code 1 + n}.
+         * the number of free dimensions of the resulting path will be {@code 1 + n}. If the free dimension associated
+         * with this path is bound by an index {@code I}, the resulting accessed offset can be obtained with the following
+         * formula:
+         *
+         * <blockquote><pre>{@code
+         * E * I
+         * }</pre></blockquote>
+         *
+         * where {@code E} is the size (in bytes) of the sequence element layout.
+         * <p>
+         * Additionally, if {@code C} is the sequence element count, it follows that {@code 0 <= I < C}.
          *
          * @return a path element which selects an unspecified sequence element layout.
          */

src/java.base/share/classes/java/lang/foreign/ValueLayout.java

@@ -137,7 +137,7 @@ public sealed class ValueLayout extends AbstractLayout implements MemoryLayout {
      * Can be used to access a multi-dimensional array whose layout is as follows:
      *
      * {@snippet lang=java :
-     * MemoryLayout arrayLayout = MemoryLayout.sequenceLayout(-1,
+     * SequenceLayout arrayLayout = MemoryLayout.sequenceLayout(-1,
      *                                      MemoryLayout.sequenceLayout(10,
      *                                                  MemoryLayout.sequenceLayout(20, ValueLayout.JAVA_INT)));
      * }
@@ -151,6 +151,22 @@ public sealed class ValueLayout extends AbstractLayout implements MemoryLayout {
      * offset = (10 * 20 * 4 * x) + (20 * 4 * y) + (4 * z)
      * }</pre></blockquote>
      *
+     * Additionally, the values of {@code x}, {@code y} and {@code z} are constrained as follows:
+     * <ul>
+     *     <li>{@code 0 <= x < arrayLayout.elementCount() }</li>
+     *     <li>{@code 0 <= y < 10 }</li>
+     *     <li>{@code 0 <= z < 20 }</li>
+     * </ul>
+     * <p>
+     * Consider the following access expressions:
+     * {@snippet lang=java :
+     * int value1 = arrayHandle.get(10, 2, 4); // ok, accessed offset = 8176
+     * int value2 = arrayHandle.get(0, 0, 30); // out of bounds value for z
+     * }
+     * In the first case, access is well-formed, as the values for {@code x}, {@code y} and {@code z} conform to
+     * the bounds specified above. In the second case, access fails with {@link IndexOutOfBoundsException},
+     * as the value for {@code z} is outside its specified bounds.
+     *
      * @param shape the size of each nested array dimension.
      * @return a var handle which can be used to dereference a multi-dimensional array, featuring {@code shape.length + 1}
      * {@code long} coordinates.

src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java

@@ -25,6 +25,8 @@
  */
 package jdk.internal.foreign;
 
+import jdk.internal.vm.annotation.ForceInline;
+
 import java.lang.foreign.GroupLayout;
 import java.lang.foreign.MemoryLayout;
 import java.lang.foreign.MemorySegment;
@@ -34,6 +36,8 @@ import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.invoke.VarHandle;
+import java.util.Arrays;
+import java.util.Objects;
 import java.util.function.UnaryOperator;
 
 /**
@@ -53,7 +57,7 @@ public class LayoutPath {
         try {
             MethodHandles.Lookup lookup = MethodHandles.lookup();
             MH_ADD_SCALED_OFFSET = lookup.findStatic(LayoutPath.class, "addScaledOffset",
-                    MethodType.methodType(long.class, long.class, long.class, long.class));
+                    MethodType.methodType(long.class, long.class, long.class, long.class, long.class));
             MH_SLICE = lookup.findVirtual(MemorySegment.class, "asSlice",
                     MethodType.methodType(MemorySegment.class, long.class, long.class));
         } catch (Throwable ex) {
@@ -66,10 +70,13 @@ public class LayoutPath {
     private final LayoutPath enclosing;
     private final long[] strides;
 
-    private LayoutPath(MemoryLayout layout, long offset, long[] strides, LayoutPath enclosing) {
+    private final long[] bounds;
+
+    private LayoutPath(MemoryLayout layout, long offset, long[] strides, long[] bounds, LayoutPath enclosing) {
         this.layout = layout;
         this.offset = offset;
         this.strides = strides;
+        this.bounds = bounds;
         this.enclosing = enclosing;
     }
 
@@ -79,7 +86,7 @@ public class LayoutPath {
         check(SequenceLayout.class, "attempting to select a sequence element from a non-sequence layout");
         SequenceLayout seq = (SequenceLayout)layout;
         MemoryLayout elem = seq.elementLayout();
-        return LayoutPath.nestedPath(elem, offset, addStride(elem.bitSize()), this);
+        return LayoutPath.nestedPath(elem, offset, addStride(elem.bitSize()), addBound(seq.elementCount()), this);
     }
 
     public LayoutPath sequenceElement(long start, long step) {
@@ -88,20 +95,21 @@ public class LayoutPath {
         checkSequenceBounds(seq, start);
         MemoryLayout elem = seq.elementLayout();
         long elemSize = elem.bitSize();
-        return LayoutPath.nestedPath(elem, offset + (start * elemSize), addStride(elemSize * step), this);
+        long nelems = step > 0 ?
+                seq.elementCount() - start :
+                start + 1;
+        long maxIndex = Math.ceilDiv(nelems, Math.abs(step));
+        return LayoutPath.nestedPath(elem, offset + (start * elemSize),
+                                     addStride(elemSize * step), addBound(maxIndex), this);
     }
 
     public LayoutPath sequenceElement(long index) {
         check(SequenceLayout.class, "attempting to select a sequence element from a non-sequence layout");
         SequenceLayout seq = (SequenceLayout)layout;
         checkSequenceBounds(seq, index);
-        long elemOffset = 0;
+        long elemSize = seq.elementLayout().bitSize();
-        if (index > 0) {
+        long elemOffset = elemSize * index;
-            //if index == 0, we do not depend on sequence element size, so skip
+        return LayoutPath.nestedPath(seq.elementLayout(), offset + elemOffset, strides, bounds, this);
-            long elemSize = seq.elementLayout().bitSize();
-            elemOffset = elemSize * index;
-        }
-        return LayoutPath.nestedPath(seq.elementLayout(), offset + elemOffset, strides, this);
     }
 
     public LayoutPath groupElement(String name) {
@@ -122,7 +130,7 @@ public class LayoutPath {
         if (elem == null) {
             throw badLayoutPath("cannot resolve '" + name + "' in layout " + layout);
         }
-        return LayoutPath.nestedPath(elem, this.offset + offset, strides, this);
+        return LayoutPath.nestedPath(elem, this.offset + offset, strides, bounds, this);
     }
 
     // Layout path projections
@@ -140,7 +148,8 @@ public class LayoutPath {
         VarHandle handle = Utils.makeSegmentViewVarHandle(valueLayout);
         for (int i = strides.length - 1; i >= 0; i--) {
             MethodHandle collector = MethodHandles.insertArguments(MH_ADD_SCALED_OFFSET, 2,
-                    Utils.bitsToBytesOrThrow(strides[i], IllegalArgumentException::new));
+                    Utils.bitsToBytesOrThrow(strides[i], IllegalArgumentException::new),
+                    bounds[i]);
             // (J, ...) -> J to (J, J, ...) -> J
             // i.e. new coord is prefixed. Last coord will correspond to innermost layout
             handle = MethodHandles.collectCoordinates(handle, 1, collector);
@@ -150,14 +159,16 @@ public class LayoutPath {
         return handle;
     }
 
-    private static long addScaledOffset(long base, long index, long stride) {
+    @ForceInline
+    private static long addScaledOffset(long base, long index, long stride, long bound) {
+        Objects.checkIndex(index, bound);
         return base + (stride * index);
     }
 
     public MethodHandle offsetHandle() {
         MethodHandle mh = MethodHandles.identity(long.class);
         for (int i = strides.length - 1; i >=0; i--) {
-            MethodHandle collector = MethodHandles.insertArguments(MH_ADD_SCALED_OFFSET, 2, strides[i]);
+            MethodHandle collector = MethodHandles.insertArguments(MH_ADD_SCALED_OFFSET, 2, strides[i], bounds[i]);
             // (J, ...) -> J to (J, J, ...) -> J
             // i.e. new coord is prefixed. Last coord will correspond to innermost layout
             mh = MethodHandles.collectArguments(mh, 0, collector);
@@ -189,11 +200,11 @@ public class LayoutPath {
     // Layout path construction
 
     public static LayoutPath rootPath(MemoryLayout layout) {
-        return new LayoutPath(layout, 0L, EMPTY_STRIDES, null);
+        return new LayoutPath(layout, 0L, EMPTY_STRIDES, EMPTY_BOUNDS, null);
     }
 
-    private static LayoutPath nestedPath(MemoryLayout layout, long offset, long[] strides, LayoutPath encl) {
+    private static LayoutPath nestedPath(MemoryLayout layout, long offset, long[] strides, long[] bounds, LayoutPath encl) {
-        return new LayoutPath(layout, offset, strides, encl);
+        return new LayoutPath(layout, offset, strides, bounds, encl);
     }
 
     // Helper methods
@@ -235,13 +246,19 @@ public class LayoutPath {
     }
 
     private long[] addStride(long stride) {
-        long[] newStrides = new long[strides.length + 1];
+        long[] newStrides = Arrays.copyOf(strides, strides.length + 1);
-        System.arraycopy(strides, 0, newStrides, 0, strides.length);
         newStrides[strides.length] = stride;
         return newStrides;
     }
 
+    private long[] addBound(long maxIndex) {
+        long[] newBounds = Arrays.copyOf(bounds, bounds.length + 1);
+        newBounds[bounds.length] = maxIndex;
+        return newBounds;
+    }
+
     private static final long[] EMPTY_STRIDES = new long[0];
+    private static final long[] EMPTY_BOUNDS = new long[0];
 
     /**
      * This class provides an immutable implementation for the {@code PathElement} interface. A path element implementation

test/jdk/java/foreign/TestSlices.java

@@ -59,6 +59,15 @@ public class TestSlices {
         }
     }
 
+    @Test(dataProvider = "slices")
+    public void testSliceBadIndex(VarHandle handle, int lo, int hi, int[] values) {
+        try (MemorySession session = MemorySession.openConfined()) {
+            MemorySegment segment = MemorySegment.allocateNative(LAYOUT, session);
+            assertThrows(() -> handle.get(segment, lo, 0));
+            assertThrows(() -> handle.get(segment, 0, hi));
+        }
+    }
+
     static void checkSlice(MemorySegment segment, VarHandle handle, long i_max, long j_max, int... values) {
         int index = 0;
         for (long i = 0 ; i < i_max ; i++) {