8275874: [JVMCI] only support aligned reads in c2v_readFieldValue

Reviewed-by: never, shade

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -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());

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);
 

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;
 

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<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())});
             }

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);