From dce3fba36c49bd4b2f07510ad45fa883eb012047 Mon Sep 17 00:00:00 2001 From: duke Date: Tue, 1 Feb 2022 09:15:54 +0000 Subject: [PATCH] Backport 2448b3f5f96ec4d9ea8fe9dae32a0aab725fb4ad --- src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 42 +++++++------------ .../src/jdk/vm/ci/hotspot/CompilerToVM.java | 8 +++- .../jdk/vm/ci/meta/MemoryAccessProvider.java | 2 +- .../test/MemoryAccessProviderData.java | 7 ++++ .../test/MemoryAccessProviderTest.java | 5 +++ 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index 577d2bbd22c..42318fe5bfa 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp @@ -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::_field method. The oopDesc::_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()); diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java index 3aff862dea6..38648df3cb7 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java @@ -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); diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/MemoryAccessProvider.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/MemoryAccessProvider.java index 9eba9d0f8ac..30c4273a97e 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/MemoryAccessProvider.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/MemoryAccessProvider.java @@ -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; diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderData.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderData.java index 86994171407..2f01ed8646e 100644 --- a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderData.java +++ b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderData.java @@ -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 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())}); } diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderTest.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderTest.java index 07001534bf7..64cf8e41127 100644 --- a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderTest.java @@ -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);