[GR-19088] JNI member hiding subclass search fails on incomplete clas…

…spath.

PullRequest: graal/4844

substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleField.java

@@ -28,7 +28,6 @@
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.util.stream.Stream;
 
 import org.graalvm.nativeimage.Platform.HOSTED_ONLY;
 import org.graalvm.nativeimage.Platforms;
@@ -42,6 +41,7 @@
 import com.oracle.svm.jni.nativeapi.JNIFieldId;
 
 import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.meta.ResolvedJavaField;
 
 /**
  * Information on a field that can be looked up and accessed via JNI.
@@ -98,6 +98,15 @@ void finishBeforeCompilation(CompilationAccessImpl access) {
         } catch (NoSuchFieldException e) {
             throw new RuntimeException(e);
         }
-        setHidingSubclasses(access.getMetaAccess(), sub -> Stream.of(sub.getDeclaredFields()).anyMatch(f -> f.getName().equals(name)));
+        setHidingSubclasses(access.getMetaAccess(), sub -> anyMatchName(sub.getInstanceFields(false)) || anyMatchName(sub.getStaticFields()));
+    }
+
+    private boolean anyMatchName(ResolvedJavaField[] fields) {
+        for (ResolvedJavaField field : fields) {
+            if (field.getName().equals(name)) {
+                return true;
+            }
+        }
+        return false;
     }
 }

substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleMember.java

@@ -28,10 +28,13 @@
 import java.util.Map;
 import java.util.function.Predicate;
 
+import com.oracle.graal.pointsto.infrastructure.WrappedJavaType;
 import com.oracle.svm.core.annotate.UnknownObjectField;
 import com.oracle.svm.hosted.meta.HostedMetaAccess;
 import com.oracle.svm.hosted.meta.HostedType;
 
+import jdk.vm.ci.meta.ResolvedJavaType;
+
 abstract class JNIAccessibleMember {
     private final JNIAccessibleClass declaringClass;
 
@@ -74,22 +77,34 @@ boolean isDiscoverableIn(Class<?> clazz) {
      *
      * @param predicate determines if the given class contains a declaration hiding this member.
      */
-    void setHidingSubclasses(HostedMetaAccess metaAccess, Predicate<Class<?>> predicate) {
+    void setHidingSubclasses(HostedMetaAccess metaAccess, Predicate<ResolvedJavaType> predicate) {
         assert hidingSubclasses == null : "must be set exactly once";
         HostedType declaringType = metaAccess.lookupJavaType(declaringClass.getClassObject());
         hidingSubclasses = findHidingSubclasses(declaringType, predicate, null);
     }
 
-    private Map<Class<?>, Void> findHidingSubclasses(HostedType type, Predicate<Class<?>> predicate, Map<Class<?>, Void> existing) {
+    private Map<Class<?>, Void> findHidingSubclasses(HostedType type, Predicate<ResolvedJavaType> predicate, Map<Class<?>, Void> existing) {
         Map<Class<?>, Void> map = existing;
+        /*
+         * HostedType.getSubTypes() only gives us subtypes that are part of our analyzed closed
+         * world, but this is fine because JNI lookups can only be done on those.
+         */
         for (HostedType subType : type.getSubTypes()) {
             if (subType.isInstantiated() || subType.getWrapped().isInTypeCheck()) {
-                Class<?> subclass = subType.getJavaClass();
-                if (predicate.test(subclass)) {
+                /*
+                 * We must use the unwrapped type to query its members in the predicate: HostedType
+                 * and AnalysisType provide only members that are in our closed world, but members
+                 * which are not part of it can still legitimately hide our member that is, and in
+                 * that case, we must not return our member in a JNI lookup. Note that we have to
+                 * use JVMCI and not reflection here to avoid errors due to unresolved types.
+                 */
+                ResolvedJavaType originalType = subType.getWrapped().getWrapped();
+                assert !(originalType instanceof WrappedJavaType) : "need fully unwrapped type for member lookups";
+                if (predicate.test(originalType)) {
                     if (map == null) {
                         map = new IdentityHashMap<>();
                     }
-                    map.put(subclass, null);
+                    map.put(subType.getJavaClass(), null);
                     // no need to explore further subclasses
                 } else {
                     map = findHidingSubclasses(subType, predicate, map);

substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleMethod.java

@@ -27,7 +27,6 @@
 // Checkstyle: allow reflection
 
 import java.lang.reflect.Modifier;
-import java.util.stream.Stream;
 
 import org.graalvm.nativeimage.Platform.HOSTED_ONLY;
 import org.graalvm.nativeimage.Platforms;
@@ -43,6 +42,8 @@
 
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaField;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.ResolvedJavaType;
 
 /**
  * Information on a method that can be looked up and called via JNI.
@@ -131,6 +132,15 @@ void finishBeforeCompilation(CompilationAccessImpl access) {
             arrayNonvirtualCallWrapper = MethodPointer.factory(hUniverse.lookup(aUniverse.lookup(arrayNonvirtualCallWrapperMethod)));
             valistNonvirtualCallWrapper = MethodPointer.factory(hUniverse.lookup(aUniverse.lookup(valistNonvirtualCallWrapperMethod)));
         }
-        setHidingSubclasses(access.getMetaAccess(), sub -> Stream.of(sub.getDeclaredMethods()).anyMatch(descriptor::matchesIgnoreReturnType));
+        setHidingSubclasses(access.getMetaAccess(), this::anyMatchIgnoreReturnType);
+    }
+
+    private boolean anyMatchIgnoreReturnType(ResolvedJavaType sub) {
+        for (ResolvedJavaMethod method : sub.getDeclaredMethods()) {
+            if (descriptor.matchesIgnoreReturnType(method)) {
+                return true;
+            }
+        }
+        return false;
     }
 }

substratevm/src/com.oracle.svm.jni/src/com/oracle/svm/jni/access/JNIAccessibleMethodDescriptor.java

@@ -33,7 +33,9 @@
 import com.oracle.svm.core.util.VMError;
 
 import jdk.vm.ci.meta.JavaMethod;
+import jdk.vm.ci.meta.JavaType;
 import jdk.vm.ci.meta.MetaUtil;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 /**
  * Method descriptor that is used for lookups of JNI-accessible methods.
@@ -104,13 +106,13 @@ public boolean equals(Object obj) {
         return false;
     }
 
-    boolean matchesIgnoreReturnType(Method method) {
+    boolean matchesIgnoreReturnType(ResolvedJavaMethod method) {
         if (!name.equals(method.getName())) {
             return false;
         }
         int position = 1; // skip '('
-        for (Class<?> parameterType : method.getParameterTypes()) {
-            String paramInternal = MetaUtil.toInternalName(parameterType.getName());
+        for (JavaType parameterType : method.getSignature().toParameterTypes(null)) {
+            String paramInternal = parameterType.getName();
             if (!signature.startsWith(paramInternal, position)) {
                 return false;
             }