8310053: VarHandle and slice handle derived from layout are lacking alignment check

Reviewed-by: mcimadamore
This commit is contained in:
Jorn Vernee 2023-06-21 00:03:13 +00:00
parent 45eaf5edd8
commit e022e87654
4 changed files with 94 additions and 17 deletions

View File

@ -380,6 +380,10 @@ public sealed interface MemoryLayout permits SequenceLayout, GroupLayout, Paddin
* Additionally, the provided dynamic values must conform to bounds which are derived from the layout path, that is,
* {@code 0 <= x_i < b_i}, where {@code 1 <= i <= n}, or {@link IndexOutOfBoundsException} is thrown.
* <p>
* The base address must be <a href="MemorySegment.html#segment-alignment">aligned</a> according to the {@linkplain
* #byteAlignment() alignment constraint} of the root layout (this layout). Note that this can be more strict
* (but not less) than the alignment constraint of the selected value layout.
* <p>
* Multiple paths can be chained, with <a href=#deref-path-elements>dereference path elements</a>.
* A dereference path element constructs a fresh native memory segment whose base address is the address value
* read obtained by accessing a memory segment at the offset determined by the layout path elements immediately preceding
@ -436,6 +440,10 @@ public sealed interface MemoryLayout permits SequenceLayout, GroupLayout, Paddin
* long size = select(elements).byteSize();
* MemorySegment slice = segment.asSlice(offset, size);
* }
* <p>
* The segment to be sliced must be <a href="MemorySegment.html#segment-alignment">aligned</a> according to the
* {@linkplain #byteAlignment() alignment constraint} of the root layout (this layout). Note that this can be more
* strict (but not less) than the alignment constraint of the selected value layout.
*
* @apiNote The returned method handle can be used to obtain a memory segment slice, similarly to {@link MemorySegment#asSlice(long, long)},
* but more flexibly, as some indices can be specified when invoking the method handle.

View File

@ -58,6 +58,8 @@ public class LayoutPath {
private static final MethodHandle MH_ADD_SCALED_OFFSET;
private static final MethodHandle MH_SLICE;
private static final MethodHandle MH_SLICE_LAYOUT;
private static final MethodHandle MH_CHECK_ALIGN;
private static final MethodHandle MH_SEGMENT_RESIZE;
static {
@ -67,6 +69,10 @@ public class LayoutPath {
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));
MH_SLICE_LAYOUT = lookup.findVirtual(MemorySegment.class, "asSlice",
MethodType.methodType(MemorySegment.class, long.class, MemoryLayout.class));
MH_CHECK_ALIGN = lookup.findStatic(LayoutPath.class, "checkAlign",
MethodType.methodType(MemorySegment.class, MemorySegment.class, MemoryLayout.class));
MH_SEGMENT_RESIZE = lookup.findStatic(LayoutPath.class, "resizeSegment",
MethodType.methodType(MemorySegment.class, MemorySegment.class, MemoryLayout.class));
} catch (Throwable ex) {
@ -193,17 +199,19 @@ public class LayoutPath {
throw new IllegalArgumentException("Path does not select a value layout");
}
VarHandle handle = Utils.makeSegmentViewVarHandle(valueLayout);
for (int i = strides.length - 1; i >= 0; 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
handle = MethodHandles.collectCoordinates(handle, 1, collector);
// If we have an enclosing layout, drop the alignment check for the accessed element,
// we check the root layout instead
ValueLayout accessedLayout = enclosing != null ? valueLayout.withByteAlignment(1) : valueLayout;
VarHandle handle = Utils.makeSegmentViewVarHandle(accessedLayout);
handle = MethodHandles.collectCoordinates(handle, 1, offsetHandle());
// we only have to check the alignment of the root layout for the first dereference we do,
// as each dereference checks the alignment of the target address when constructing its segment
// (see Utils::longToAddress)
if (derefAdapters.length == 0 && enclosing != null) {
MethodHandle checkHandle = MethodHandles.insertArguments(MH_CHECK_ALIGN, 1, rootLayout());
handle = MethodHandles.filterCoordinates(handle, 0, checkHandle);
}
handle = MethodHandles.insertCoordinates(handle, 1,
offset);
if (adapt) {
for (int i = derefAdapters.length; i > 0; i--) {
@ -231,16 +239,37 @@ public class LayoutPath {
return mh;
}
public MethodHandle sliceHandle() {
MethodHandle offsetHandle = offsetHandle(); // byte offset
private MemoryLayout rootLayout() {
return enclosing != null ? enclosing.rootLayout() : this.layout;
}
MethodHandle sliceHandle = MH_SLICE; // (MS, long, long) -> MS
sliceHandle = MethodHandles.insertArguments(sliceHandle, 2, layout.byteSize()); // (MS, long) -> MS
sliceHandle = MethodHandles.collectArguments(sliceHandle, 1, offsetHandle); // (MS, ...) -> MS
public MethodHandle sliceHandle() {
MethodHandle sliceHandle;
if (enclosing != null) {
// drop the alignment check for the accessed element, we check the root layout instead
sliceHandle = MH_SLICE; // (MS, long, long) -> MS
sliceHandle = MethodHandles.insertArguments(sliceHandle, 2, layout.byteSize()); // (MS, long) -> MS
} else {
sliceHandle = MH_SLICE_LAYOUT; // (MS, long, MemoryLayout) -> MS
sliceHandle = MethodHandles.insertArguments(sliceHandle, 2, layout); // (MS, long) -> MS
}
sliceHandle = MethodHandles.collectArguments(sliceHandle, 1, offsetHandle()); // (MS, ...) -> MS
if (enclosing != null) {
MethodHandle checkHandle = MethodHandles.insertArguments(MH_CHECK_ALIGN, 1, rootLayout());
sliceHandle = MethodHandles.filterArguments(sliceHandle, 0, checkHandle);
}
return sliceHandle;
}
private static MemorySegment checkAlign(MemorySegment segment, MemoryLayout constraint) {
if (!((AbstractMemorySegmentImpl) segment).isAlignedForElement(0, constraint)) {
throw new IllegalArgumentException("Target offset incompatible with alignment constraints: " + constraint.byteAlignment());
}
return segment;
}
public MemoryLayout layout() {
return layout;
}

View File

@ -143,4 +143,16 @@ public class TestDereferencePath {
void badDerefAddressNoTarget() {
A_MULTI_NO_TARGET.varHandle(PathElement.groupElement("bs"), PathElement.dereferenceElement());
}
@Test(expectedExceptions = IllegalArgumentException.class)
void badDerefMisAligned() {
MemoryLayout struct = MemoryLayout.structLayout(
ValueLayout.ADDRESS.withTargetLayout(ValueLayout.JAVA_INT).withName("x"));
try (Arena arena = Arena.ofConfined()) {
MemorySegment segment = arena.allocate(struct.byteSize() + 1).asSlice(1);
VarHandle vhX = struct.varHandle(PathElement.groupElement("x"), PathElement.dereferenceElement());
vhX.set(segment, 42); // should throw
}
}
}

View File

@ -31,10 +31,10 @@
import java.lang.foreign.*;
import java.lang.foreign.MemoryLayout.PathElement;
import org.testng.SkipException;
import org.testng.annotations.*;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.VarHandle;
import java.util.ArrayList;
import java.util.List;
import java.util.function.IntFunction;
@ -42,6 +42,7 @@ import java.util.function.IntFunction;
import static java.lang.foreign.MemoryLayout.PathElement.groupElement;
import static java.lang.foreign.MemoryLayout.PathElement.sequenceElement;
import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.JAVA_SHORT;
import static org.testng.Assert.*;
public class TestLayoutPaths {
@ -134,6 +135,34 @@ public class TestLayoutPaths {
seq.byteOffsetHandle(sequenceElement(5, 1)); // invalid range (starting position is outside the sequence)
}
@Test
public void testBadAlignmentOfRoot() throws Throwable {
MemoryLayout struct = MemoryLayout.structLayout(
JAVA_INT,
JAVA_SHORT.withName("x"));
assertEquals(struct.byteAlignment(), 4);
try (Arena arena = Arena.ofConfined()) {
MemorySegment seg = arena.allocate(struct.byteSize() + 2, struct.byteAlignment()).asSlice(2);
assertEquals(seg.address() % JAVA_SHORT.byteAlignment(), 0); // should be aligned
assertNotEquals(seg.address() % struct.byteAlignment(), 0); // should not be aligned
String expectedMessage = "Target offset incompatible with alignment constraints: " + struct.byteAlignment();
VarHandle vhX = struct.varHandle(groupElement("x"));
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> {
vhX.set(seg, (short) 42);
});
assertEquals(iae.getMessage(), expectedMessage);
MethodHandle sliceX = struct.sliceHandle(groupElement("x"));
iae = expectThrows(IllegalArgumentException.class, () -> {
MemorySegment slice = (MemorySegment) sliceX.invokeExact(seg);
});
assertEquals(iae.getMessage(), expectedMessage);
}
}
@Test
public void testBadSequencePathInOffset() {
SequenceLayout seq = MemoryLayout.sequenceLayout(10, JAVA_INT);
@ -338,4 +367,3 @@ public class TestLayoutPaths {
}
}