ART: fix varhandle bounds check for split bytebuffer
Bug: 203822424
Test: art/test/run-test --host 712
Change-Id: I01db8401809ac21faea050ec1e33358be32ca1ec
diff --git a/runtime/mirror/var_handle.cc b/runtime/mirror/var_handle.cc
index 6788dce..d36a2ab 100644
--- a/runtime/mirror/var_handle.cc
+++ b/runtime/mirror/var_handle.cc
@@ -112,21 +112,32 @@
}
bool CheckElementIndex(Primitive::Type type,
- int32_t relative_index,
+ int32_t index,
int32_t start,
- int32_t limit) REQUIRES_SHARED(Locks::mutator_lock_) {
- int64_t index = start + relative_index;
- int64_t max_index = limit - Primitive::ComponentSize(type);
- if (index < start || index > max_index) {
- ThrowIndexOutOfBoundsException(index, limit - start);
+ int32_t length) REQUIRES_SHARED(Locks::mutator_lock_) {
+ // The underlying memory may be shared and offset from the start of allocated region,
+ // ie buffers can be created via ByteBuffer.split().
+ //
+ // `type` is the type of the value the caller is attempting to read / write.
+ // `index` represents the position the caller is trying to access in the underlying ByteBuffer
+ // or byte array. This is an offset from from `start` in bytes.
+ // `start` represents where the addressable memory begins relative to the base of the
+ // the underlying ByteBuffer or byte array.
+ // `length` represents the length of the addressable region.
+ //
+ // Thus the region being operated on is:
+ // `base` + `start` + `index` to `base` + `start` + `index` + `sizeof(type)`
+ int32_t max_index = length - start - Primitive::ComponentSize(type);
+ if (index < 0 || index > max_index) {
+ ThrowIndexOutOfBoundsException(index, length - start);
return false;
}
return true;
}
-bool CheckElementIndex(Primitive::Type type, int32_t index, int32_t range_limit)
+bool CheckElementIndex(Primitive::Type type, int32_t index, int32_t length)
REQUIRES_SHARED(Locks::mutator_lock_) {
- return CheckElementIndex(type, index, 0, range_limit);
+ return CheckElementIndex(type, index, 0, length);
}
// Returns true if access_mode only entails a memory read. False if
@@ -1945,9 +1956,10 @@
}
const int32_t byte_buffer_limit = byte_buffer->GetField32(
GetMemberOffset(WellKnownClasses::java_nio_ByteBuffer_limit));
+ const int32_t byte_buffer_length = byte_buffer_offset + byte_buffer_limit;
const Primitive::Type primitive_type = GetVarType()->GetPrimitiveType();
- if (!CheckElementIndex(primitive_type, byte_index, byte_buffer_offset, byte_buffer_limit)) {
+ if (!CheckElementIndex(primitive_type, byte_index, byte_buffer_offset, byte_buffer_length)) {
return false;
}
const int32_t checked_offset32 = byte_buffer_offset + byte_index;
diff --git a/test/712-varhandle-invocations/src/VarHandleArrayTests.java b/test/712-varhandle-invocations/src/VarHandleArrayTests.java
index 19f840b..249dde0 100644
--- a/test/712-varhandle-invocations/src/VarHandleArrayTests.java
+++ b/test/712-varhandle-invocations/src/VarHandleArrayTests.java
@@ -16,6 +16,8 @@
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
// These tests cover DoVarHandleInvokeCommon in interpreter_common.cc.
@@ -118,10 +120,206 @@
assertThrowsWMTE(() -> vho.compareAndSet(values, ONE, ZERO));
}
+ private short toHost(ByteOrder order, byte b0, byte b1) {
+ final int u0 = Byte.toUnsignedInt(b0);
+ final int u1 = Byte.toUnsignedInt(b1);
+ if (order == ByteOrder.LITTLE_ENDIAN) {
+ return (short) (u0 + (u1 << 8));
+ } else {
+ return (short) (u1 + (u0 << 8));
+ }
+ }
+
+ private int toHost(ByteOrder order, byte b0, byte b1, byte b2, byte b3) {
+ final int u0 = Byte.toUnsignedInt(b0);
+ final int u1 = Byte.toUnsignedInt(b1);
+ final int u2 = Byte.toUnsignedInt(b2);
+ final int u3 = Byte.toUnsignedInt(b3);
+ if (order == ByteOrder.LITTLE_ENDIAN) {
+ return u0 + (u1 << 8) + (u2 << 16) + (u3 << 24);
+ } else {
+ return u3 + (u2 << 8) + (u1 << 16) + (u0 << 24);
+ }
+ }
+
+ private void testByteArrayViewVarHandle() {
+ final int BITS_PER_BYTE = 8;
+ byte[] array = new byte[32];
+
+ final ByteOrder[] byteOrders =
+ new ByteOrder[] {ByteOrder.LITTLE_ENDIAN, ByteOrder.BIG_ENDIAN};
+
+ for (ByteOrder order : byteOrders) {
+ {
+ final VarHandle vhShort =
+ MethodHandles.byteArrayViewVarHandle(short[].class, order);
+ assertThrowsIOOBE(() -> vhShort.get(array, -1));
+ assertThrowsIOOBE(() -> vhShort.get(array, Integer.MIN_VALUE));
+ assertThrowsIOOBE(() -> vhShort.get(array, array.length));
+ assertThrowsIOOBE(() -> vhShort.get(array, array.length - 1));
+ assertThrowsIOOBE(() -> vhShort.get(array, Integer.MAX_VALUE));
+
+ for (int i = 0; i < array.length - 1; ++i) {
+ final boolean isAligned = (i % 2) == 0;
+ final short value = (short) ((i + 1) * 0xff);
+ vhShort.set(array, i, value);
+ assertEquals(value, (short) vhShort.get(array, i));
+ assertEquals(value, toHost(order, array[i], array[i + 1]));
+ for (int j = 0; j < array.length; ++j) {
+ if (j < i || j > i + 1) {
+ assertEquals((byte) 0, array[j]);
+ }
+ }
+ if (isAligned) {
+ vhShort.getAcquire(array, i);
+ vhShort.setRelease(array, i, (short) 0);
+ } else {
+ final int fi = i;
+ assertThrowsISE(() -> vhShort.getAcquire(array, fi));
+ assertThrowsISE(() -> vhShort.setRelease(array, fi, (short) 0));
+ }
+ vhShort.set(array, i, (short) 0);
+ }
+ }
+ {
+ final VarHandle vhInt =
+ MethodHandles.byteArrayViewVarHandle(int[].class, order);
+ assertThrowsIOOBE(() -> vhInt.get(array, -1));
+ assertThrowsIOOBE(() -> vhInt.get(array, Integer.MIN_VALUE));
+ assertThrowsIOOBE(() -> vhInt.get(array, array.length));
+ assertThrowsIOOBE(() -> vhInt.get(array, array.length - 1));
+ assertThrowsIOOBE(() -> vhInt.get(array, array.length - 2));
+ assertThrowsIOOBE(() -> vhInt.get(array, array.length - 3));
+ assertThrowsIOOBE(() -> vhInt.get(array, Integer.MAX_VALUE));
+ for (int i = 0; i < array.length - 3; ++i) {
+ final boolean isAligned = (i % 4) == 0;
+ final int value = (i + 1) * 0x11223344;
+ vhInt.set(array, i, value);
+ assertEquals(value, vhInt.get(array, i));
+ assertEquals(
+ value,
+ toHost(order, array[i], array[i + 1], array[i + 2], array[i + 3]));
+ for (int j = 0; j < array.length; ++j) {
+ if (j < i || j > i + 3) {
+ assertEquals((byte) 0, array[j]);
+ }
+ }
+ if (isAligned) {
+ vhInt.getAcquire(array, i);
+ vhInt.setRelease(array, i, (int) 0);
+ } else {
+ final int fi = i;
+ assertThrowsISE(() -> vhInt.getAcquire(array, fi));
+ assertThrowsISE(() -> vhInt.setRelease(array, fi, (int) 0));
+ }
+ vhInt.set(array, i, 0);
+ }
+ }
+ }
+ }
+
+ private void testByteBufferVarHandle() {
+ final ByteOrder[] byteOrders =
+ new ByteOrder[] {ByteOrder.LITTLE_ENDIAN, ByteOrder.BIG_ENDIAN};
+
+ for (final ByteOrder byteOrder : byteOrders) {
+ final ByteBuffer heapBuffer = ByteBuffer.allocate(32);
+ final ByteBuffer directBuffer = ByteBuffer.allocateDirect(32);
+ final ByteBuffer arrayBuffer = ByteBuffer.wrap(new byte[32]);
+ final ByteBuffer anotherArrayBuffer = ByteBuffer.wrap(new byte[32], 3, 23);
+ final ByteBuffer[] buffers = {
+ heapBuffer,
+ ((ByteBuffer) heapBuffer.duplicate().position(1)).slice(),
+ directBuffer,
+ ((ByteBuffer) directBuffer.duplicate().position(1)).slice(),
+ arrayBuffer,
+ ((ByteBuffer) arrayBuffer.duplicate().position(1)).slice(),
+ anotherArrayBuffer,
+ ((ByteBuffer) anotherArrayBuffer.duplicate().position(1)).slice()
+ };
+ for (final ByteBuffer buffer : buffers) {
+ {
+ final VarHandle vhShort =
+ MethodHandles.byteBufferViewVarHandle(short[].class, byteOrder);
+ assertThrowsIOOBE(() -> vhShort.get(buffer, -1));
+ assertThrowsIOOBE(() -> vhShort.get(buffer, Integer.MIN_VALUE));
+ assertThrowsIOOBE(() -> vhShort.get(buffer, Integer.MAX_VALUE));
+ assertThrowsIOOBE(() -> vhShort.get(buffer, buffer.limit()));
+ assertThrowsIOOBE(() -> vhShort.get(buffer, buffer.limit() - 1));
+ final int zeroAlignment = buffer.alignmentOffset(0, Short.BYTES);
+ for (int i = 0; i < buffer.limit() - 1; ++i) {
+ boolean isAligned = (zeroAlignment + i) % Short.BYTES == 0;
+ final short value = (short) ((i + 1) * 0xff);
+ vhShort.set(buffer, i, value);
+ assertEquals(value, (short) vhShort.get(buffer, i));
+ assertEquals(
+ value, toHost(byteOrder, buffer.get(i), buffer.get(i + 1)));
+ for (int j = 0; j < buffer.limit(); ++j) {
+ if (j < i || j > i + 1) {
+ assertEquals((byte) 0, buffer.get(j));
+ }
+ }
+ if (isAligned) {
+ vhShort.getAcquire(buffer, i);
+ vhShort.setRelease(buffer, i, (short) 0);
+ } else {
+ final int fi = i;
+ assertThrowsISE(() -> vhShort.getAcquire(buffer, fi));
+ assertThrowsISE(() -> vhShort.setRelease(buffer, fi, (short) 0));
+ }
+ vhShort.set(buffer, i, (short) 0);
+ }
+ }
+ {
+ final VarHandle vhInt =
+ MethodHandles.byteBufferViewVarHandle(int[].class, byteOrder);
+ assertThrowsIOOBE(() -> vhInt.get(buffer, -1));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, Integer.MIN_VALUE));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, Integer.MAX_VALUE));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, buffer.limit()));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, buffer.limit() - 1));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, buffer.limit() - 2));
+ assertThrowsIOOBE(() -> vhInt.get(buffer, buffer.limit() - 3));
+ final int zeroAlignment = buffer.alignmentOffset(0, Integer.BYTES);
+ for (int i = 0; i < buffer.limit() - 3; ++i) {
+ boolean isAligned = (zeroAlignment + i) % Integer.BYTES == 0;
+ final int value = (i + 1) * 0x11223344;
+ vhInt.set(buffer, i, value);
+ assertEquals(value, vhInt.get(buffer, i));
+ assertEquals(
+ value,
+ toHost(
+ byteOrder,
+ buffer.get(i),
+ buffer.get(i + 1),
+ buffer.get(i + 2),
+ buffer.get(i + 3)));
+ for (int j = 0; j < buffer.limit(); ++j) {
+ if (j < i || j > i + 3) {
+ assertEquals((byte) 0, buffer.get(j));
+ }
+ }
+ if (isAligned) {
+ vhInt.getAcquire(buffer, i);
+ vhInt.setRelease(buffer, i, (int) 0);
+ } else {
+ final int fi = i;
+ assertThrowsISE(() -> vhInt.getAcquire(buffer, fi));
+ assertThrowsISE(() -> vhInt.setRelease(buffer, fi, (int) 0));
+ }
+ vhInt.set(buffer, i, 0);
+ }
+ }
+ }
+ }
+ }
+
@Override
protected void doTest() throws Exception {
testIntegerArrayVarHandle();
testObjectArrayVarHandle();
+ testByteArrayViewVarHandle();
+ testByteBufferVarHandle();
}
public static void main(String[] args) {
diff --git a/test/712-varhandle-invocations/src/VarHandleBadCoordinateTests.java b/test/712-varhandle-invocations/src/VarHandleBadCoordinateTests.java
index 31a004e..e7ec1ae 100644
--- a/test/712-varhandle-invocations/src/VarHandleBadCoordinateTests.java
+++ b/test/712-varhandle-invocations/src/VarHandleBadCoordinateTests.java
@@ -564,6 +564,8 @@
}
public static class ByteBufferViewOutOfBoundsIndexTest extends VarHandleUnitTest {
+ private final static int BYTES_PER_FLOAT = Float.SIZE / Byte.SIZE;
+
private static final VarHandle vh;
static {
@@ -583,27 +585,12 @@
ByteBuffer.wrap(new byte[27], 3, 27 - 3)
};
for (ByteBuffer buffer : buffers) {
- try {
- vh.get(buffer, -1);
- failUnreachable();
- } catch (IndexOutOfBoundsException ex) {
- }
- try {
- vh.get(buffer, buffer.limit());
- failUnreachable();
- } catch (IndexOutOfBoundsException ex) {
- }
- try {
- vh.get(buffer, Integer.MAX_VALUE - 1);
- failUnreachable();
- } catch (IndexOutOfBoundsException ex) {
- }
- try {
- vh.get(buffer, buffer.limit() - Integer.SIZE / 8 + 1);
- failUnreachable();
- } catch (IndexOutOfBoundsException ex) {
- }
- vh.get(buffer, buffer.limit() - Integer.SIZE / 8);
+ assertThrowsIOOBE(() -> vh.get(buffer, -1));
+ assertThrowsIOOBE(() -> vh.get(buffer, Integer.MIN_VALUE));
+ assertThrowsIOOBE(() -> vh.get(buffer, Integer.MAX_VALUE));
+ assertThrowsIOOBE(() -> vh.get(buffer, buffer.limit()));
+ assertThrowsIOOBE(() -> vh.get(buffer, buffer.limit() - BYTES_PER_FLOAT + 1));
+ vh.get(buffer, buffer.limit() - BYTES_PER_FLOAT);
}
}
diff --git a/test/712-varhandle-invocations/src/VarHandleUnitTest.java b/test/712-varhandle-invocations/src/VarHandleUnitTest.java
index 7891fa8..a7861bd 100644
--- a/test/712-varhandle-invocations/src/VarHandleUnitTest.java
+++ b/test/712-varhandle-invocations/src/VarHandleUnitTest.java
@@ -118,6 +118,14 @@
assertThrows(ArrayStoreException.class, access);
}
+ public final void assertThrowsISE(AccessorAccess access) {
+ assertThrows(IllegalStateException.class, access);
+ }
+
+ public final void assertThrowsIOOBE(AccessorAccess access) {
+ assertThrows(IndexOutOfBoundsException.class, access);
+ }
+
public final void assertThrowsCCE(AccessorAccess access) {
assertThrows(ClassCastException.class, access);
}
@@ -145,6 +153,7 @@
doTest();
} catch (Exception e) {
fail("Unexpected exception", e);
+ e.printStackTrace();
} finally {
if (lazyErrorLog == null) {
collector.success();