Skip to content
Permalink
Browse files
8275874: [JVMCI] only support aligned reads in c2v_readFieldValue
Backport-of: 2448b3f5f96ec4d9ea8fe9dae32a0aab725fb4ad
  • Loading branch information
GoeLin committed Feb 3, 2022
1 parent 0b51fe2 commit e459c8cbaa33ab63bd5d48540852906255f4dd24
Showing 5 changed files with 33 additions and 31 deletions.
@@ -1832,19 +1832,6 @@ C2V_VMENTRY_NULL(jobjectArray, getDeclaredMethods, (JNIEnv* env, jobject, jobjec
return JVMCIENV->get_jobjectArray(methods);
C2V_END

// Enforces volatile semantics for a non-volatile read.
class VolatileRead : public StackObj {
public:
VolatileRead() {
// Ensures a possibly volatile read is not reordered with a prior
// volatile write.
OrderAccess::storeload();
}
~VolatileRead() {
OrderAccess::acquire();
}
};

C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object, jobject expected_type, long displacement, jobject kind_object))
if (object == NULL || kind_object == NULL) {
JVMCI_THROW_0(NullPointerException);
@@ -1885,13 +1872,18 @@ C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object,
ShouldNotReachHere();
}

if (displacement < 0 || ((long) displacement + type2aelembytes(basic_type) > HeapWordSize * obj->size())) {
int basic_type_elemsize = type2aelembytes(basic_type);
if (displacement < 0 || ((long) displacement + basic_type_elemsize > HeapWordSize * obj->size())) {
// Reading outside of the object bounds
JVMCI_THROW_MSG_NULL(IllegalArgumentException, "reading outside object bounds");
}

// Perform basic sanity checks on the read. Primitive reads are permitted to read outside the
// bounds of their fields but object reads must map exactly onto the underlying oop slot.
bool aligned = (displacement % basic_type_elemsize) == 0;
if (!aligned) {
JVMCI_THROW_MSG_NULL(IllegalArgumentException, "read is unaligned");
}
if (basic_type == T_OBJECT) {
if (obj->is_objArray()) {
if (displacement < arrayOopDesc::base_offset_in_bytes(T_OBJECT)) {
@@ -1932,22 +1924,17 @@ C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object,

// Treat all reads as volatile for simplicity as this function can be used
// both for reading Java fields declared as volatile as well as for constant
// folding Unsafe.get* methods with volatile semantics. This is done by
// performing the volatile barrier operations around a call to an
// oopDesc::<kind>_field method. The oopDesc::<kind>_field_acquire method
// cannot be used since it does not support unaligned reads on all platforms
// (e.g., an unaligned ldar on AArch64 causes a SIGBUS).

// folding Unsafe.get* methods with volatile semantics.

switch (basic_type) {
case T_BOOLEAN: { VolatileRead vr; value = obj->bool_field(displacement); } break;
case T_BYTE: { VolatileRead vr; value = obj->byte_field(displacement); } break;
case T_SHORT: { VolatileRead vr; value = obj->short_field(displacement);} break;
case T_CHAR: { VolatileRead vr; value = obj->char_field(displacement); } break;
case T_BOOLEAN: value = obj->bool_field_acquire(displacement); break;
case T_BYTE: value = obj->byte_field_acquire(displacement); break;
case T_SHORT: value = obj->short_field_acquire(displacement); break;
case T_CHAR: value = obj->char_field_acquire(displacement); break;
case T_FLOAT:
case T_INT: { VolatileRead vr; value = obj->int_field(displacement); } break;
case T_INT: value = obj->int_field_acquire(displacement); break;
case T_DOUBLE:
case T_LONG: { VolatileRead vr; value = obj->long_field(displacement); } break;
case T_LONG: value = obj->long_field_acquire(displacement); break;

case T_OBJECT: {
if (displacement == java_lang_Class::component_mirror_offset() && java_lang_Class::is_instance(obj()) &&
@@ -1957,8 +1944,7 @@ C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object,
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_NULL_POINTER());
}

oop value;
{ VolatileRead vr; value = obj->obj_field(displacement); }
oop value = obj->obj_field_acquire(displacement);

if (value == NULL) {
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_NULL_POINTER());
@@ -783,15 +783,19 @@ HotSpotResolvedObjectTypeImpl getResolvedJavaType(long displacement, boolean com

/**
* Reads the current value of a static field. If {@code expectedType} is non-null, then the
* object is exptected to be a subtype of {@code expectedType} and extra sanity checking is
* object is expected to be a subtype of {@code expectedType} and extra sanity checking is
* performed on the offset and kind of the read being performed.
*
* @throws IllegalArgumentException if any of the sanity checks fail
*/
native JavaConstant readFieldValue(HotSpotResolvedObjectTypeImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, JavaKind kind);

/**
* Reads the current value of an instance field. If {@code expectedType} is non-null, then the
* object is exptected to be a subtype of {@code expectedType} and extra sanity checking is
* object is expected to be a subtype of {@code expectedType} and extra sanity checking is
* performed on the offset and kind of the read being performed.
*
* @throws IllegalArgumentException if any of the sanity checks fail
*/
native JavaConstant readFieldValue(HotSpotObjectConstantImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, JavaKind kind);

@@ -37,7 +37,7 @@ public interface MemoryAccessProvider {
* @return the read value encapsulated in a {@link JavaConstant} object of {@link JavaKind} kind
* @throws IllegalArgumentException if the read is out of bounds of the object or {@code kind}
* is {@link JavaKind#Void} or not {@linkplain JavaKind#isPrimitive() primitive}
* kind or {@code bits} is not 8, 16, 32 or 64
* kind or {@code bits} is not 8, 16, 32 or 64, or the read is unaligned
*/
JavaConstant readPrimitiveConstant(JavaKind kind, Constant base, long displacement, int bits) throws IllegalArgumentException;

@@ -77,6 +77,13 @@ public static Object[][] getPositivePrimitiveJavaKinds() {
for (KindData k : PRIMITIVE_KIND_DATA) {
result.add(new Object[] {k.kind, TEST_CONSTANT, k.instanceFieldOffset, k.instanceFieldValue, Math.max(8, k.kind.getBitCount())});
result.add(new Object[] {k.kind, TEST_CLASS_CONSTANT, k.staticFieldOffset, k.staticFieldValue, Math.max(8, k.kind.getBitCount())});
}
return result.toArray(new Object[result.size()][]);
}
@DataProvider(name = "unalignedPrimitive")
public static Object[][] getUnalignedPrimitiveJavaKinds() {
List<Object[]> result = new ArrayList<>();
for (KindData k : PRIMITIVE_KIND_DATA) {
if (k.unalignedInstanceFieldValue != null) {
result.add(new Object[] {k.kind, TEST_CONSTANT, k.instanceFieldOffset - 1, k.unalignedInstanceFieldValue, Math.max(8, k.kind.getBitCount())});
}
@@ -64,6 +64,11 @@ public void testReadPrimitiveConstantNullBase(JavaKind kind, Constant base, Long
Assert.assertNull(PROVIDER.readPrimitiveConstant(kind, null, offset, bitsCount), "Unexpected value for null base");
}

@Test(dataProvider = "unalignedPrimitive", dataProviderClass = MemoryAccessProviderData.class, expectedExceptions = {IllegalArgumentException.class})
public void testReadUnalignedConstantConstant(JavaKind kind, Constant base, Long offset, Object expected, int bitsCount) {
PROVIDER.readPrimitiveConstant(kind, null, offset, bitsCount);
}

@Test(dataProvider = "negative", dataProviderClass = MemoryAccessProviderData.class, expectedExceptions = {IllegalArgumentException.class})
public void testNegativeReadPrimitiveConstant(JavaKind kind, Constant base) {
PROVIDER.readPrimitiveConstant(kind, base, 0L, kind == null ? 0 : kind.getByteCount() / 8);

1 comment on commit e459c8c

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on e459c8c Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.