Skip to content
Permalink
Browse files
8275874: [JVMCI] only support aligned reads in c2v_readFieldValue
Reviewed-by: never, shade
  • Loading branch information
Doug Simon committed Oct 26, 2021
1 parent f1f5e26 commit 2448b3f5f96ec4d9ea8fe9dae32a0aab725fb4ad
Showing 5 changed files with 33 additions and 31 deletions.
@@ -1891,19 +1891,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);
@@ -1944,13 +1931,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)) {
@@ -1991,22 +1983,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()) &&
@@ -2016,8 +2003,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);

3 comments on commit 2448b3f

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 2448b3f Oct 26, 2021

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 2448b3f Feb 1, 2022

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 2448b3f Feb 1, 2022

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-2448b3f5 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 2448b3f5 from the openjdk/jdk repository.

The commit being backported was authored by Doug Simon on 26 Oct 2021 and was reviewed by Tom Rodriguez and Aleksey Shipilev.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-2448b3f5:GoeLin-backport-2448b3f5
$ git checkout GoeLin-backport-2448b3f5
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-2448b3f5

Please sign in to comment.