8278897: Alignment of heap segments is not enforced correctly

Reviewed-by: jvernee

Author: mcimadamore

src/java.base/share/classes/java/lang/invoke/MemoryAccessVarHandleBase.java

@@ -50,7 +50,7 @@ abstract class MemoryAccessVarHandleBase extends VarHandle {
  this.alignmentMask = alignmentMask;
  }
 
- static IllegalStateException newIllegalStateExceptionForMisalignedAccess(long address) {
- return new IllegalStateException("Misaligned access at address: " + address);
+ static IllegalArgumentException newIllegalArgumentExceptionForMisalignedAccess(long address) {
+ return new IllegalArgumentException("Misaligned access at address: " + address);
  }
 }

src/java.base/share/classes/java/lang/invoke/X-VarHandleMemoryAccess.java.template

@@ -106,7 +106,7 @@ final class MemoryAccessVarHandle$Type$Helper extends MemoryAccessVarHandleBase
  static long offset(boolean skipAlignmentMaskCheck, MemorySegmentProxy bb, long offset, long alignmentMask) {
  long address = offsetNoVMAlignCheck(skipAlignmentMaskCheck, bb, offset, alignmentMask);
  if ((address & VM_ALIGN) != 0) {
- throw MemoryAccessVarHandleBase.newIllegalStateExceptionForMisalignedAccess(address);
+ throw MemoryAccessVarHandleBase.newIllegalArgumentExceptionForMisalignedAccess(address);
  }
  return address;
  }
@@ -115,14 +115,15 @@ final class MemoryAccessVarHandle$Type$Helper extends MemoryAccessVarHandleBase
  static long offsetNoVMAlignCheck(boolean skipAlignmentMaskCheck, MemorySegmentProxy bb, long offset, long alignmentMask) {
  long base = bb.unsafeGetOffset();
  long address = base + offset;
+ long maxAlignMask = bb.maxAlignMask();
  if (skipAlignmentMaskCheck) {
  //note: the offset portion has already been aligned-checked, by construction
- if ((base & alignmentMask) != 0) {
- throw MemoryAccessVarHandleBase.newIllegalStateExceptionForMisalignedAccess(address);
+ if (((base | maxAlignMask) & alignmentMask) != 0) {
+ throw MemoryAccessVarHandleBase.newIllegalArgumentExceptionForMisalignedAccess(address);
  }
  } else {
- if ((address & alignmentMask) != 0) {
- throw MemoryAccessVarHandleBase.newIllegalStateExceptionForMisalignedAccess(address);
+ if (((address | maxAlignMask) & alignmentMask) != 0) {
+ throw MemoryAccessVarHandleBase.newIllegalArgumentExceptionForMisalignedAccess(address);
  }
  }
  return address;

src/java.base/share/classes/jdk/internal/access/foreign/MemorySegmentProxy.java

@@ -44,6 +44,7 @@ public abstract class MemorySegmentProxy {
  public abstract Object unsafeGetBase();
  public abstract boolean isSmall();
  public abstract ScopedMemoryAccess.Scope scope();
+ public abstract long maxAlignMask();
 
  /* Helper functions for offset computations. These are required so that we can avoid issuing long opcodes
  * (e.g. LMUL, LADD) when we're operating on 'small' segments (segments whose length can be expressed with an int).

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java

@@ -124,8 +124,12 @@ public Spliterator<MemorySegment> spliterator(MemoryLayout elementLayout) {
  if (elementLayout.byteSize() == 0) {
  throw new IllegalArgumentException("Element layout size cannot be zero");
  }
- if (byteSize() % elementLayout.byteSize() != 0) {
- throw new IllegalArgumentException("Segment size is no a multiple of layout size");
+ Utils.checkElementAlignment(elementLayout, "Element layout alignment greater than its size");
+ if (!isAlignedForElement(0, elementLayout)) {
+ throw new IllegalArgumentException("Incompatible alignment constraints");
+ }
+ if (!Utils.isAligned(byteSize(), elementLayout.byteSize())) {
+ throw new IllegalArgumentException("Segment size is not a multiple of layout size");
  }
  return new SegmentSplitter(elementLayout.byteSize(), byteSize() / elementLayout.byteSize(),
  this);
@@ -383,8 +387,13 @@ private boolean isSet(int mask) {
  return (this.mask & mask) != 0;
  }
 
+ @ForceInline
+ public final boolean isAlignedForElement(long offset, MemoryLayout layout) {
+ return (((unsafeGetOffset() + offset) | maxAlignMask()) & (layout.byteAlignment() - 1)) == 0;
+ }
+
  private int checkArraySize(String typeName, int elemSize) {
- if (length % elemSize != 0) {
+ if (!Utils.isAligned(length, elemSize)) {
  throw new IllegalStateException(String.format("Segment size is not a multiple of %d. Size: %d", elemSize, length));
  }
  long arraySize = length / elemSize;

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java

@@ -47,6 +47,11 @@ public abstract class HeapMemorySegmentImpl<H> extends AbstractMemorySegmentImpl
  private static final Unsafe UNSAFE = Unsafe.getUnsafe();
  private static final int BYTE_ARR_BASE = UNSAFE.arrayBaseOffset(byte[].class);
 
+ private static final long MAX_ALIGN_1 = 1;
+ private static final long MAX_ALIGN_2 = 2;
+ private static final long MAX_ALIGN_4 = 4;
+ private static final long MAX_ALIGN_8 = 8;
+
  final long offset;
  final H base;
 
@@ -100,6 +105,11 @@ public static MemorySegment fromArray(byte[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_BYTE_INDEX_SCALE;
  return new OfByte(Unsafe.ARRAY_BYTE_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_1;
+ }
  }
 
  public static class OfChar extends HeapMemorySegmentImpl<char[]> {
@@ -123,6 +133,11 @@ public static MemorySegment fromArray(char[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_CHAR_INDEX_SCALE;
  return new OfChar(Unsafe.ARRAY_CHAR_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_2;
+ }
  }
 
  public static class OfShort extends HeapMemorySegmentImpl<short[]> {
@@ -146,6 +161,11 @@ public static MemorySegment fromArray(short[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_SHORT_INDEX_SCALE;
  return new OfShort(Unsafe.ARRAY_SHORT_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_2;
+ }
  }
 
  public static class OfInt extends HeapMemorySegmentImpl<int[]> {
@@ -169,6 +189,11 @@ public static MemorySegment fromArray(int[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_INT_INDEX_SCALE;
  return new OfInt(Unsafe.ARRAY_INT_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_4;
+ }
  }
 
  public static class OfLong extends HeapMemorySegmentImpl<long[]> {
@@ -192,6 +217,11 @@ public static MemorySegment fromArray(long[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_LONG_INDEX_SCALE;
  return new OfLong(Unsafe.ARRAY_LONG_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_8;
+ }
  }
 
  public static class OfFloat extends HeapMemorySegmentImpl<float[]> {
@@ -215,6 +245,11 @@ public static MemorySegment fromArray(float[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_FLOAT_INDEX_SCALE;
  return new OfFloat(Unsafe.ARRAY_FLOAT_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_4;
+ }
  }
 
  public static class OfDouble extends HeapMemorySegmentImpl<double[]> {
@@ -238,6 +273,11 @@ public static MemorySegment fromArray(double[] arr) {
  long byteSize = (long)arr.length * Unsafe.ARRAY_DOUBLE_INDEX_SCALE;
  return new OfDouble(Unsafe.ARRAY_DOUBLE_BASE_OFFSET, arr, byteSize, defaultAccessModes(byteSize));
  }
+
+ @Override
+ public long maxAlignMask() {
+ return MAX_ALIGN_8;
+ }
  }
 
 }

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java

@@ -282,11 +282,11 @@ private static IllegalArgumentException badLayoutPath(String cause) {
  private static void checkAlignment(LayoutPath path) {
  MemoryLayout layout = path.layout;
  long alignment = layout.bitAlignment();
- if (path.offset % alignment != 0) {
+ if (!Utils.isAligned(path.offset, alignment)) {
  throw new UnsupportedOperationException("Invalid alignment requirements for layout " + layout);
  }
  for (long stride : path.strides) {
- if (stride % alignment != 0) {
+ if (!Utils.isAligned(stride, alignment)) {
  throw new UnsupportedOperationException("Alignment requirements for layout " + layout + " do not match stride " + stride);
  }
  }

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java

@@ -267,6 +267,7 @@ public void set(ValueLayout.OfAddress layout, long offset, Addressable value) {
  @CallerSensitive
  public char getAtIndex(ValueLayout.OfChar layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -275,6 +276,7 @@ public char getAtIndex(ValueLayout.OfChar layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfChar layout, long index, char value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -283,6 +285,7 @@ public void setAtIndex(ValueLayout.OfChar layout, long index, char value) {
  @CallerSensitive
  public short getAtIndex(ValueLayout.OfShort layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -291,6 +294,7 @@ public short getAtIndex(ValueLayout.OfShort layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfShort layout, long index, short value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -299,6 +303,7 @@ public void setAtIndex(ValueLayout.OfShort layout, long index, short value) {
  @CallerSensitive
  public int getAtIndex(ValueLayout.OfInt layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -307,6 +312,7 @@ public int getAtIndex(ValueLayout.OfInt layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfInt layout, long index, int value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -315,6 +321,7 @@ public void setAtIndex(ValueLayout.OfInt layout, long index, int value) {
  @CallerSensitive
  public float getAtIndex(ValueLayout.OfFloat layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -323,6 +330,7 @@ public float getAtIndex(ValueLayout.OfFloat layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfFloat layout, long index, float value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -331,6 +339,7 @@ public void setAtIndex(ValueLayout.OfFloat layout, long index, float value) {
  @CallerSensitive
  public long getAtIndex(ValueLayout.OfLong layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -339,6 +348,7 @@ public long getAtIndex(ValueLayout.OfLong layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfLong layout, long index, long value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -347,6 +357,7 @@ public void setAtIndex(ValueLayout.OfLong layout, long index, long value) {
  @CallerSensitive
  public double getAtIndex(ValueLayout.OfDouble layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -355,6 +366,7 @@ public double getAtIndex(ValueLayout.OfDouble layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfDouble layout, long index, double value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value);
  }
 
@@ -363,6 +375,7 @@ public void setAtIndex(ValueLayout.OfDouble layout, long index, double value) {
  @CallerSensitive
  public MemoryAddress getAtIndex(ValueLayout.OfAddress layout, long index) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  return NativeMemorySegmentImpl.EVERYTHING.get(layout, toRawLongValue() + (index * layout.byteSize()));
  }
 
@@ -371,6 +384,7 @@ public MemoryAddress getAtIndex(ValueLayout.OfAddress layout, long index) {
  @CallerSensitive
  public void setAtIndex(ValueLayout.OfAddress layout, long index, Addressable value) {
  Reflection.ensureNativeAccess(Reflection.getCallerClass());
+ Utils.checkElementAlignment(layout, "Layout alignment greater than its size");
  NativeMemorySegmentImpl.EVERYTHING.set(layout, toRawLongValue() + (index * layout.byteSize()), value.address());
  }
 }

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

@@ -92,6 +92,11 @@ Object base() {
  return null;
  }
 
+ @Override
+ public long maxAlignMask() {
+ return 0;
+ }
+
  // factories
 
  public static MemorySegment makeNativeSegment(long bytesSize, long alignmentBytes, ResourceScopeImpl scope) {