8362306: HotSpotJVMCIRuntime.getMirror can crash

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -2277,7 +2277,7 @@ C2V_VMENTRY_NULL(jobjectArray, getDeclaredFieldsInfo, (JNIEnv* env, jobject, ARG
   InstanceKlass* iklass = InstanceKlass::cast(klass);
   int java_fields, injected_fields;
   GrowableArray<FieldInfo>* fields = FieldInfoStream::create_FieldInfoArray(iklass->fieldinfo_stream(), &java_fields, &injected_fields);
-  JVMCIObjectArray array = JVMCIENV->new_FieldInfo_array(fields->length(), JVMCIENV);
+  JVMCIObjectArray array = JVMCIENV->new_FieldInfo_array(fields->length(), JVMCI_CHECK_NULL);
   for (int i = 0; i < fields->length(); i++) {
     JVMCIObject field_info = JVMCIENV->new_FieldInfo(fields->adr_at(i), JVMCI_CHECK_NULL);
     JVMCIENV->put_object_at(array, i, field_info);
@@ -2984,13 +2984,21 @@ C2V_VMENTRY_NULL(jobject, asReflectionExecutable, (JNIEnv* env, jobject, ARGUMEN
   return JNIHandles::make_local(THREAD, executable);
 C2V_END
 
+// Checks that `index` denotes a non-injected field in `klass`
 static InstanceKlass* check_field(Klass* klass, jint index, JVMCI_TRAPS) {
   if (!klass->is_instance_klass()) {
     JVMCI_THROW_MSG_NULL(IllegalArgumentException,
         err_msg("Expected non-primitive type, got %s", klass->external_name()));
   }
   InstanceKlass* iklass = InstanceKlass::cast(klass);
-  if (index < 0 || index > iklass->total_fields_count()) {
+  if (index < 0 || index >= iklass->java_fields_count()) {
+    if (index >= 0 && index < iklass->total_fields_count()) {
+      fieldDescriptor fd(iklass, index);
+      if (fd.is_injected()) {
+        JVMCI_THROW_MSG_NULL(IllegalArgumentException,
+            err_msg("Cannot get Field for injected %s.%s", klass->external_name(), fd.name()->as_C_string()));
+      }
+    }
     JVMCI_THROW_MSG_NULL(IllegalArgumentException,
         err_msg("Field index %d out of bounds for %s", index, klass->external_name()));
   }
@@ -3000,15 +3008,15 @@ static InstanceKlass* check_field(Klass* klass, jint index, JVMCI_TRAPS) {
 C2V_VMENTRY_NULL(jobject, asReflectionField, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index))
   requireInHotSpot("asReflectionField", JVMCI_CHECK_NULL);
   Klass* klass = UNPACK_PAIR(Klass, klass);
-  InstanceKlass* iklass = check_field(klass, index, JVMCIENV);
+  InstanceKlass* iklass = check_field(klass, index, JVMCI_CHECK_NULL);
   fieldDescriptor fd(iklass, index);
   oop reflected = Reflection::new_field(&fd, CHECK_NULL);
   return JNIHandles::make_local(THREAD, reflected);
 C2V_END
 
 static jbyteArray get_encoded_annotation_data(InstanceKlass* holder, AnnotationArray* annotations_array, bool for_class,
                                               jint filter_length, jlong filter_klass_pointers,
-                                              JavaThread* THREAD, JVMCIEnv* JVMCIENV) {
+                                              JavaThread* THREAD, JVMCI_TRAPS) {
   // Get a ConstantPool object for annotation parsing
   Handle jcp = reflect_ConstantPool::create(CHECK_NULL);
   reflect_ConstantPool::set_cp(jcp(), holder->constants());
@@ -3086,7 +3094,7 @@ C2V_END
 C2V_VMENTRY_NULL(jbyteArray, getEncodedFieldAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index,
                  jobject filter, jint filter_length, jlong filter_klass_pointers))
   CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support
-  InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCIENV);
+  InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCI_CHECK_NULL);
   fieldDescriptor fd(holder, index);
   return get_encoded_annotation_data(holder, fd.annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV);
 C2V_END

src/hotspot/share/runtime/fieldDescriptor.hpp

@@ -84,6 +84,7 @@ class fieldDescriptor {
   bool is_static()                const    { return access_flags().is_static(); }
   bool is_final()                 const    { return access_flags().is_final(); }
   bool is_stable()                const    { return field_flags().is_stable(); }
+  bool is_injected()              const    { return field_flags().is_injected(); }
   bool is_volatile()              const    { return access_flags().is_volatile(); }
   bool is_transient()             const    { return access_flags().is_transient(); }
 

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java

@@ -775,8 +775,8 @@ public boolean test(ResolvedJavaType type) {
      *         instances
      */
     public Class<?> getMirror(ResolvedJavaType type) {
-        if (type instanceof HotSpotResolvedJavaType && reflection instanceof HotSpotJDKReflection) {
-            return ((HotSpotJDKReflection) reflection).getMirror((HotSpotResolvedJavaType) type);
+        if (type instanceof HotSpotResolvedJavaType hType && reflection instanceof HotSpotJDKReflection) {
+            return ((HotSpotJDKReflection) reflection).getMirror(hType);
         }
         return null;
     }
@@ -790,8 +790,8 @@ public Class<?> getMirror(ResolvedJavaType type) {
      *         instances to {@link Executable} instances
      */
     public Executable getMirror(ResolvedJavaMethod method) {
-        if (method instanceof HotSpotResolvedJavaMethodImpl && reflection instanceof HotSpotJDKReflection) {
-            return HotSpotJDKReflection.getMethod((HotSpotResolvedJavaMethodImpl) method);
+        if (!method.isClassInitializer() && method instanceof HotSpotResolvedJavaMethodImpl hMethod && reflection instanceof HotSpotJDKReflection) {
+            return HotSpotJDKReflection.getMethod(hMethod);
         }
         return null;
     }
@@ -805,8 +805,8 @@ public Executable getMirror(ResolvedJavaMethod method) {
      *         instances
      */
     public Field getMirror(ResolvedJavaField field) {
-        if (field instanceof HotSpotResolvedJavaFieldImpl && reflection instanceof HotSpotJDKReflection) {
-            return HotSpotJDKReflection.getField((HotSpotResolvedJavaFieldImpl) field);
+        if (!field.isInternal() && field instanceof HotSpotResolvedJavaFieldImpl hField && reflection instanceof HotSpotJDKReflection) {
+            return HotSpotJDKReflection.getField(hField);
         }
         return null;
     }

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java

@@ -297,6 +297,16 @@ public ResolvedJavaMethod[] getDeclaredConstructors() {
         return new ResolvedJavaMethod[0];
     }
 
+    @Override
+    public ResolvedJavaMethod[] getDeclaredConstructors(boolean forceLink) {
+        return new ResolvedJavaMethod[0];
+    }
+
+    @Override
+    public ResolvedJavaMethod[] getDeclaredMethods(boolean forceLink) {
+        return new ResolvedJavaMethod[0];
+    }
+
     @Override
     public ResolvedJavaMethod[] getDeclaredMethods() {
         return new ResolvedJavaMethod[0];

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaField.java

@@ -33,6 +33,7 @@
  * @compile ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberDeleted.java
  *          ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberTypeChanged.java
  * @modules jdk.internal.vm.ci/jdk.vm.ci.meta
+ *          jdk.internal.vm.ci/jdk.vm.ci.hotspot
  *          jdk.internal.vm.ci/jdk.vm.ci.runtime
  *          jdk.internal.vm.ci/jdk.vm.ci.common
  *          java.base/jdk.internal.reflect

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java

@@ -34,6 +34,7 @@
  *          ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberTypeChanged.java
  * @modules java.base/jdk.internal.reflect
  *          jdk.internal.vm.ci/jdk.vm.ci.meta
+ *          jdk.internal.vm.ci/jdk.vm.ci.hotspot
  *          jdk.internal.vm.ci/jdk.vm.ci.runtime
  *          jdk.internal.vm.ci/jdk.vm.ci.common
  *          java.base/jdk.internal.misc
@@ -104,16 +105,25 @@
 import jdk.vm.ci.meta.ResolvedJavaType;
 import jdk.vm.ci.meta.UnresolvedJavaType;
 import sun.reflect.annotation.AnnotationSupport;
+import jdk.vm.ci.hotspot.HotSpotJVMCIRuntime;
 
 /**
  * Tests for {@link ResolvedJavaType}.
  */
 public class TestResolvedJavaType extends TypeUniverse {
     private static final Class<? extends Annotation> SIGNATURE_POLYMORPHIC_CLASS = findPolymorphicSignatureClass();
+    private static final HotSpotJVMCIRuntime runtime = HotSpotJVMCIRuntime.runtime();
 
     public TestResolvedJavaType() {
     }
 
+    @Test
+    public void getMirrorTest() {
+        for (ResolvedJavaType type : javaTypes) {
+            assertEquals(type.toClassName(), runtime.getMirror(type).getName());
+        }
+    }
+
     @Test
     public void equalsTest() {
         for (ResolvedJavaType t : javaTypes) {
@@ -935,6 +945,9 @@ public Field lookupField(Collection<Field> fields, ResolvedJavaField key) {
         return null;
     }
 
+    /**
+     * Replicates the semantics of jdk.internal.reflect.Reflection#fieldFilterMap.
+     */
     private static boolean isHiddenFromReflection(ResolvedJavaField f) {
         if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(ConstantPool.class)) && f.getName().equals("constantPoolOop")) {
             return true;
@@ -971,7 +984,13 @@ public void getInstanceFieldsTest() {
                 int reflectFieldIndex = 0;
                 for (int i = 0; i < fields.length; i++) {
                     ResolvedJavaField field = fields[i];
-                    if (field.isInternal() || isHiddenFromReflection(field)) {
+                    var mirror = runtime.getMirror(field);
+                    if (field.isInternal()) {
+                        assertNull(field.toString(), mirror);
+                        continue;
+                    }
+                    assertNotNull(field.toString(), mirror);
+                    if (isHiddenFromReflection(field)) {
                         continue;
                     }
                     Field reflectField = reflectFields.get(reflectFieldIndex++);
@@ -1002,6 +1021,8 @@ public void getStaticFieldsTest() {
                 assertNotNull(lookupField(actual, f));
             }
             for (ResolvedJavaField rf : actual) {
+                var mirror = runtime.getMirror(rf);
+                assertNotNull(rf.toString(), mirror);
                 if (!isHiddenFromReflection(rf)) {
                     assertEquals(lookupField(expected, rf) != null, !rf.isInternal());
                 }
@@ -1025,6 +1046,28 @@ public void getDeclaredMethodsTest() {
                 expected.add(resolvedMethod);
             }
             Set<ResolvedJavaMethod> actual = new HashSet<>(Arrays.asList(type.getDeclaredMethods()));
+            for (ResolvedJavaMethod method : actual) {
+                assertNotNull(method.toString(), runtime.getMirror(method));
+            }
+            assertEquals(expected, actual);
+        }
+    }
+
+    @Test
+    public void getDeclaredConstructorsTest() {
+        for (Class<?> c : classes) {
+            ResolvedJavaType type = metaAccess.lookupJavaType(c);
+            Constructor<?>[] raw = c.getDeclaredConstructors();
+            Set<ResolvedJavaMethod> expected = new HashSet<>();
+            for (Constructor<?> m : raw) {
+                ResolvedJavaMethod resolvedMethod = metaAccess.lookupJavaMethod(m);
+                assertNotNull(resolvedMethod);
+                expected.add(resolvedMethod);
+            }
+            Set<ResolvedJavaMethod> actual = new HashSet<>(Arrays.asList(type.getDeclaredConstructors()));
+            for (ResolvedJavaMethod method : actual) {
+                assertNotNull(runtime.getMirror(method));
+            }
             assertEquals(expected, actual);
         }
     }
@@ -1067,6 +1110,7 @@ private static ResolvedJavaMethod getClassInitializer(Class<?> c) {
         if (clinit != null) {
             assertEquals(0, clinit.getAnnotations().length);
             assertEquals(0, clinit.getDeclaredAnnotations().length);
+            assertNull(runtime.getMirror(clinit));
         }
         return clinit;
     }
@@ -1238,7 +1282,6 @@ public void getAnnotationDataTest() throws Exception {
         "initialize",
         "isPrimitive",
         "newArray",
-        "getDeclaredConstructors",
         "isInitialized",
         "isLinked",
         "getJavaClass",