[GR-32104] Reflection metadata bugfixes

loicottet · loicottet · commit be5cf4d7a286 · 2021-10-11T18:01:24.000Z
PullRequest: graal/9841
diff --git a/espresso/mx.espresso/mx_espresso.py b/espresso/mx.espresso/mx_espresso.py
@@ -129,7 +129,14 @@ def _espresso_gate_runner(args, tasks):
                             break
 
                 with open(generated_header, 'r') as generated_header_file:
-                    generated_header_lines = generated_header_file.readlines()
+                    generated_header_lines = []
+                    for line in generated_header_file.readlines():
+                        # Ignore definitions that are not needed for Espresso
+                        if not line.startswith("typedef") or "(*Espresso_" in line or "__graal" in line or "(*graal_" in line:
+                            generated_header_lines.append(line)
+                        else:
+                            newline = generated_header_lines.pop()  # Remove newline before ignored declaration
+                            assert newline == "\n"
 
                 errors = errors or mx.update_file(committed_header, ''.join(committed_header_copyright + generated_header_lines), showDiff=True)
 
diff --git a/espresso/src/com.oracle.truffle.espresso.mokapot/include/libespresso_dynamic.h b/espresso/src/com.oracle.truffle.espresso.mokapot/include/libespresso_dynamic.h
@@ -42,8 +42,6 @@ typedef int (*Espresso_CloseContext_fn_t)(graal_isolatethread_t* thread, struct
 
 typedef void (*Espresso_Exit_fn_t)(graal_isolatethread_t* thread, struct JavaVM_* javaVM);
 
-typedef void (*vmLocatorSymbol_fn_t)(graal_isolatethread_t* thread);
-
 #if defined(__cplusplus)
 }
 #endif
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoDecoder.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/CodeInfoDecoder.java
@@ -568,9 +568,9 @@ public static Executable[] getMethodMetadata(int typeID) {
 
             int modifiers = dataReader.getUVInt();
 
-            int paramCount = dataReader.getUVInt();
-            Class<?>[] paramTypes = new Class<?>[paramCount];
-            for (int j = 0; j < paramCount; ++j) {
+            int paramTypeCount = dataReader.getUVInt();
+            Class<?>[] paramTypes = new Class<?>[paramTypeCount];
+            for (int j = 0; j < paramTypeCount; ++j) {
                 int paramTypeIndex = dataReader.getSVInt();
                 paramTypes[j] = NonmovableArrays.getObject(CodeInfoAccess.getFrameInfoSourceClasses(info), paramTypeIndex);
             }
@@ -606,27 +606,27 @@ public static Executable[] getMethodMetadata(int typeID) {
                 typeAnnotations[j] = (byte) dataReader.getS1();
             }
 
-            boolean parameterDataPresent = dataReader.getU1() == 1;
-            String[] parameterNames = null;
-            int[] parameterModifiers = null;
-            if (parameterDataPresent) {
-                int parameterCount = dataReader.getUVInt();
-                parameterNames = new String[parameterCount];
-                parameterModifiers = new int[parameterCount];
-                for (int j = 0; j < paramCount; ++j) {
-                    int parameterNameIndex = dataReader.getSVInt();
-                    parameterNames[j] = NonmovableArrays.getObject(CodeInfoAccess.getFrameInfoSourceMethodNames(info), parameterNameIndex);
-                    parameterModifiers[j] = dataReader.getS4();
+            boolean reflectParameterDataPresent = dataReader.getU1() == 1;
+            String[] reflectParameterNames = null;
+            int[] reflectParameterModifiers = null;
+            if (reflectParameterDataPresent) {
+                int reflectParameterCount = dataReader.getUVInt();
+                reflectParameterNames = new String[reflectParameterCount];
+                reflectParameterModifiers = new int[reflectParameterCount];
+                for (int j = 0; j < reflectParameterCount; ++j) {
+                    int reflectParameterNameIndex = dataReader.getSVInt();
+                    reflectParameterNames[j] = NonmovableArrays.getObject(CodeInfoAccess.getFrameInfoSourceMethodNames(info), reflectParameterNameIndex);
+                    reflectParameterModifiers[j] = dataReader.getS4();
                 }
             }
 
             if (name.equals("<init>")) {
                 assert returnType == void.class;
                 methods[i] = ImageSingletons.lookup(RuntimeReflectionConstructors.class).newConstructor(declaringClass, paramTypes, exceptionTypes, modifiers, signature,
-                                annotations, parameterAnnotations, typeAnnotations, parameterNames, parameterModifiers);
+                                annotations, parameterAnnotations, typeAnnotations, reflectParameterNames, reflectParameterModifiers);
             } else {
                 methods[i] = ImageSingletons.lookup(RuntimeReflectionConstructors.class).newMethod(declaringClass, name, paramTypes, returnType, exceptionTypes, modifiers, signature,
-                                annotations, parameterAnnotations, null, typeAnnotations, parameterNames, parameterModifiers);
+                                annotations, parameterAnnotations, null, typeAnnotations, reflectParameterNames, reflectParameterModifiers);
             }
         }
         return methods;
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/MethodMetadataEncoder.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/code/MethodMetadataEncoder.java
@@ -44,6 +44,7 @@
 import java.util.Set;
 import java.util.TreeMap;
 
+import org.graalvm.collections.Pair;
 import org.graalvm.compiler.core.common.util.TypeConversion;
 import org.graalvm.compiler.core.common.util.UnsafeArrayTypeWriter;
 import org.graalvm.nativeimage.ImageSingletons;
@@ -57,6 +58,7 @@
 import com.oracle.svm.core.util.ByteArrayReader;
 import com.oracle.svm.util.ReflectionUtil;
 
+import jdk.vm.ci.meta.JavaType;
 import sun.invoke.util.Wrapper;
 import sun.reflect.annotation.AnnotationType;
 import sun.reflect.annotation.TypeAnnotation;
@@ -67,7 +69,7 @@ public class MethodMetadataEncoder {
     public static final int NO_METHOD_METADATA = -1;
 
     private CodeInfoEncoder.Encoders encoders;
-    private TreeMap<SharedType, Set<Executable>> methodData;
+    private TreeMap<SharedType, Set<Pair<SharedMethod, Executable>>> methodData;
 
     private byte[] methodDataEncoding;
     private byte[] methodDataIndexEncoding;
@@ -111,7 +113,7 @@ public void prepareMetadataForMethod(SharedMethod method, Executable reflectMeth
         }
 
         /* Register string values in annotations */
-        registerStrings(GuardedAnnotationAccess.getDeclaredAnnotations(reflectMethod));
+        registerStrings(GuardedAnnotationAccess.getDeclaredAnnotations(method));
         for (Annotation[] annotations : reflectMethod.getParameterAnnotations()) {
             registerStrings(annotations);
         }
@@ -126,7 +128,7 @@ public void prepareMetadataForMethod(SharedMethod method, Executable reflectMeth
         }
 
         SharedType declaringType = (SharedType) method.getDeclaringClass();
-        methodData.computeIfAbsent(declaringType, t -> new HashSet<>()).add(reflectMethod);
+        methodData.computeIfAbsent(declaringType, t -> new HashSet<>()).add(Pair.create(method, reflectMethod));
     }
 
     private static final Method hasRealParameterData = ReflectionUtil.lookupMethod(Executable.class, "hasRealParameterData");
@@ -135,9 +137,9 @@ private void encodeMethodMetadata() {
         UnsafeArrayTypeWriter dataEncodingBuffer = UnsafeArrayTypeWriter.create(ByteArrayReader.supportsUnalignedMemoryAccess());
         UnsafeArrayTypeWriter indexEncodingBuffer = UnsafeArrayTypeWriter.create(ByteArrayReader.supportsUnalignedMemoryAccess());
         long lastTypeID = -1;
-        for (Map.Entry<SharedType, Set<Executable>> entry : methodData.entrySet()) {
+        for (Map.Entry<SharedType, Set<Pair<SharedMethod, Executable>>> entry : methodData.entrySet()) {
             SharedType declaringType = entry.getKey();
-            Set<Executable> methods = entry.getValue();
+            Set<Pair<SharedMethod, Executable>> methods = entry.getValue();
             long typeID = declaringType.getHub().getTypeID();
             assert typeID > lastTypeID;
             lastTypeID++;
@@ -148,62 +150,70 @@ private void encodeMethodMetadata() {
             long index = dataEncodingBuffer.getBytesWritten();
             indexEncodingBuffer.putS4(index);
             dataEncodingBuffer.putUV(methods.size());
-            for (Executable method : methods) {
-                Class<?> declaringClass = method.getDeclaringClass();
+            for (Pair<SharedMethod, Executable> method : methods) {
+                SharedMethod hostedMethod = method.getLeft();
+                Executable reflectMethod = method.getRight();
+
+                Class<?> declaringClass = getJavaClass((SharedType) hostedMethod.getDeclaringClass());
                 final int classIndex = encoders.sourceClasses.getIndex(declaringClass);
                 dataEncodingBuffer.putSV(classIndex);
 
-                String name = method instanceof Constructor<?> ? "<init>" : ((Method) method).getName();
+                String name = hostedMethod.isConstructor() ? "<init>" : hostedMethod.getName();
                 final int nameIndex = encoders.sourceMethodNames.getIndex(name);
                 dataEncodingBuffer.putSV(nameIndex);
 
-                dataEncodingBuffer.putUV(method.getModifiers());
+                dataEncodingBuffer.putUV(reflectMethod.getModifiers());
 
-                Class<?>[] parameterTypes = method.getParameterTypes();
+                /* Parameter types do not include the receiver */
+                JavaType[] parameterTypes = hostedMethod.getSignature().toParameterTypes(null);
                 dataEncodingBuffer.putUV(parameterTypes.length);
-                for (Class<?> parameterType : parameterTypes) {
-                    final int paramClassIndex = encoders.sourceClasses.getIndex(parameterType);
+                for (JavaType parameterType : parameterTypes) {
+                    Class<?> parameterClass = getJavaClass((SharedType) parameterType);
+                    final int paramClassIndex = encoders.sourceClasses.getIndex(parameterClass);
                     dataEncodingBuffer.putSV(paramClassIndex);
                 }
 
-                Class<?> returnType = method instanceof Constructor<?> ? void.class : ((Method) method).getReturnType();
+                Class<?> returnType = void.class;
+                if (!hostedMethod.isConstructor()) {
+                    returnType = getJavaClass((SharedType) hostedMethod.getSignature().getReturnType(null));
+                }
                 final int returnTypeIndex = encoders.sourceClasses.getIndex(returnType);
                 dataEncodingBuffer.putSV(returnTypeIndex);
 
                 /* Only include types that are in the image (i.e. that can actually be thrown) */
-                Class<?>[] exceptionTypes = filterTypes(method.getExceptionTypes());
+                Class<?>[] exceptionTypes = filterTypes(reflectMethod.getExceptionTypes());
                 dataEncodingBuffer.putUV(exceptionTypes.length);
                 for (Class<?> exceptionClazz : exceptionTypes) {
                     final int exceptionClassIndex = encoders.sourceClasses.getIndex(exceptionClazz);
                     dataEncodingBuffer.putSV(exceptionClassIndex);
                 }
 
-                final int signatureIndex = encoders.sourceMethodNames.getIndex(getSignature(method));
+                final int signatureIndex = encoders.sourceMethodNames.getIndex(getSignature(reflectMethod));
                 dataEncodingBuffer.putSV(signatureIndex);
 
                 try {
-                    byte[] annotations = encodeAnnotations(GuardedAnnotationAccess.getDeclaredAnnotations(method));
+                    byte[] annotations = encodeAnnotations(GuardedAnnotationAccess.getDeclaredAnnotations(hostedMethod));
                     dataEncodingBuffer.putUV(annotations.length);
                     for (byte b : annotations) {
                         dataEncodingBuffer.putS1(b);
                     }
 
-                    byte[] parameterAnnotations = encodeParameterAnnotations(method.getParameterAnnotations());
+                    byte[] parameterAnnotations = encodeParameterAnnotations(reflectMethod.getParameterAnnotations());
                     dataEncodingBuffer.putUV(parameterAnnotations.length);
                     for (byte b : parameterAnnotations) {
                         dataEncodingBuffer.putS1(b);
                     }
 
-                    byte[] typeAnnotations = encodeTypeAnnotations((TypeAnnotation[]) parseAllTypeAnnotations.invoke(null, method));
+                    byte[] typeAnnotations = encodeTypeAnnotations((TypeAnnotation[]) parseAllTypeAnnotations.invoke(null, reflectMethod));
                     dataEncodingBuffer.putUV(typeAnnotations.length);
                     for (byte b : typeAnnotations) {
                         dataEncodingBuffer.putS1(b);
                     }
 
-                    boolean parameterDataPresent = (boolean) hasRealParameterData.invoke(method);
+                    boolean parameterDataPresent = (boolean) hasRealParameterData.invoke(reflectMethod);
                     dataEncodingBuffer.putU1(parameterDataPresent ? 1 : 0);
                     if (parameterDataPresent) {
-                        Parameter[] parameters = method.getParameters();
+                        Parameter[] parameters = reflectMethod.getParameters();
                         dataEncodingBuffer.putUV(parameters.length);
                         for (Parameter parameter : parameters) {
                             final int parameterNameIndex = encoders.sourceMethodNames.getIndex(parameter.getName());
@@ -226,6 +236,10 @@ private void encodeMethodMetadata() {
         indexEncodingBuffer.toArray(methodDataIndexEncoding);
     }
 
+    private static Class<?> getJavaClass(SharedType sharedType) {
+        return sharedType.getHub().getHostedJavaClass();
+    }
+
     private Class<?>[] filterTypes(Class<?>[] types) {
         List<Class<?>> filteredTypes = new ArrayList<>();
         for (Class<?> type : types) {
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/RuntimeReflectionConstructors.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/RuntimeReflectionConstructors.java
@@ -41,8 +41,8 @@ Method newMethod(Class<?> declaringClass,
                     byte[] parameterAnnotations,
                     byte[] annotationDefault,
                     byte[] typeAnnotations,
-                    String[] parameterNames,
-                    int[] parameterModifiers);
+                    String[] reflectParameterNames,
+                    int[] reflectParameterModifiers);
 
     Constructor<?> newConstructor(Class<?> declaringClass,
                     Class<?>[] parameterTypes,
@@ -52,6 +52,6 @@ Constructor<?> newConstructor(Class<?> declaringClass,
                     byte[] annotations,
                     byte[] parameterAnnotations,
                     byte[] typeAnnotations,
-                    String[] parameterNames,
-                    int[] parameterModifiers);
+                    String[] reflectParameterNames,
+                    int[] reflectParameterModifiers);
 }
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java
@@ -231,12 +231,10 @@ private void processReachableTypes(DuringAnalysisAccessImpl access) {
 
     protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
         if (SubstrateOptions.ConfigureReflectionMetadata.getValue()) {
-            /* Trigger creation of the AnalysisMethod objects needed to store metadata */
             Set<Executable> newQueriedMethods = new HashSet<>();
             for (Executable method : queriedMethods) {
                 if (!SubstitutionReflectivityFilter.shouldExclude(method, access.getMetaAccess(), access.getUniverse())) {
-                    access.getMetaAccess().lookupJavaMethod(method);
-                    registerMetadataForReflection(access, method);
+                    registerMetadataForReflection(access, access.getMetaAccess().lookupJavaMethod(method), method);
                     newQueriedMethods.add(method);
                 }
             }
@@ -248,26 +246,26 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
                  * method.
                  */
                 if (method.isReachable() && method.hasJavaMethod()) {
-                    registerMetadataForReflection(access, method.getJavaMethod());
+                    registerMetadataForReflection(access, method, method.getJavaMethod());
                 }
             }
         }
     }
 
     private static final Method parseAllTypeAnnotations = ReflectionUtil.lookupMethod(TypeAnnotationParser.class, "parseAllTypeAnnotations", AnnotatedElement.class);
 
-    private void registerMetadataForReflection(DuringAnalysisAccessImpl access, Executable reflectMethod) {
+    private void registerMetadataForReflection(DuringAnalysisAccessImpl access, AnalysisMethod analysisMethod, Executable reflectMethod) {
         /*
          * Reflection signature parsing will try to instantiate classes via Class.forName().
          */
-        makeTypeReachable(access, reflectMethod.getDeclaringClass(), false);
+        makeTypeReachable(access, analysisMethod.getDeclaringClass().getJavaClass(), false);
         for (TypeVariable<?> type : reflectMethod.getTypeParameters()) {
             makeTypeReachable(access, type, true);
         }
-        for (Type paramType : reflectMethod.getGenericParameterTypes()) {
+        for (Type paramType : analysisMethod.getGenericParameterTypes()) {
             makeTypeReachable(access, paramType, true);
         }
-        if (reflectMethod instanceof Method) {
+        if (!analysisMethod.isConstructor()) {
             makeTypeReachable(access, ((Method) reflectMethod).getGenericReturnType(), true);
         }
         for (Type exceptionType : reflectMethod.getGenericExceptionTypes()) {
@@ -277,7 +275,7 @@ private void registerMetadataForReflection(DuringAnalysisAccessImpl access, Exec
         /*
          * Enable runtime parsing of annotations
          */
-        for (Annotation annotation : GuardedAnnotationAccess.getDeclaredAnnotations(reflectMethod)) {
+        for (Annotation annotation : GuardedAnnotationAccess.getDeclaredAnnotations(analysisMethod)) {
             makeTypeReachable(access, annotation.annotationType(), false);
         }
         for (Annotation[] parameterAnnotations : reflectMethod.getParameterAnnotations()) {
@@ -304,7 +302,7 @@ private void makeTypeReachable(DuringAnalysisAccessImpl access, Type type, boole
             return;
         }
         seenTypes.add(type);
-        if (type instanceof Class<?>) {
+        if (type instanceof Class<?> && !SubstitutionReflectivityFilter.shouldExclude((Class<?>) type, access.getMetaAccess(), access.getUniverse())) {
             Class<?> clazz = (Class<?>) type;
             if (access.getMetaAccess().lookupJavaType(clazz).registerAsReachable()) {
                 access.requireAnalysisIteration();
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/target/RuntimeReflectionConstructorsImpl.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/target/RuntimeReflectionConstructorsImpl.java

Original file line number	Diff line number	Diff line change
`@@ -42,8 +42,6 @@ typedef int (Espresso_CloseContext_fn_t)(graal_isolatethread_t thread, struct`
`42`	`42`
`43`	`43`	`typedef void (Espresso_Exit_fn_t)(graal_isolatethread_t thread, struct JavaVM_* javaVM);`
`44`	`44`
`45`		`-typedef void (vmLocatorSymbol_fn_t)(graal_isolatethread_t thread);`
`46`		`-`
`47`	`45`	`#if defined(__cplusplus)`
`48`	`46`	`}`
`49`	`47`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -231,12 +231,10 @@ private void processReachableTypes(DuringAnalysisAccessImpl access) {`
`231`	`231`
`232`	`232`	`protected void processMethodMetadata(DuringAnalysisAccessImpl access) {`
`233`	`233`	`if (SubstrateOptions.ConfigureReflectionMetadata.getValue()) {`
`234`		`- /* Trigger creation of the AnalysisMethod objects needed to store metadata */`
`235`	`234`	`Set<Executable> newQueriedMethods = new HashSet<>();`
`236`	`235`	`for (Executable method : queriedMethods) {`
`237`	`236`	`if (!SubstitutionReflectivityFilter.shouldExclude(method, access.getMetaAccess(), access.getUniverse())) {`
`238`		`- access.getMetaAccess().lookupJavaMethod(method);`
`239`		`- registerMetadataForReflection(access, method);`
	`237`	`+ registerMetadataForReflection(access, access.getMetaAccess().lookupJavaMethod(method), method);`
`240`	`238`	`newQueriedMethods.add(method);`
`241`	`239`	`}`
`242`	`240`	`}`
`@@ -248,26 +246,26 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {`
`248`	`246`	`* method.`
`249`	`247`	`*/`
`250`	`248`	`if (method.isReachable() && method.hasJavaMethod()) {`
`251`		`- registerMetadataForReflection(access, method.getJavaMethod());`
	`249`	`+ registerMetadataForReflection(access, method, method.getJavaMethod());`
`252`	`250`	`}`
`253`	`251`	`}`
`254`	`252`	`}`
`255`	`253`	`}`
`256`	`254`
`257`	`255`	`private static final Method parseAllTypeAnnotations = ReflectionUtil.lookupMethod(TypeAnnotationParser.class, "parseAllTypeAnnotations", AnnotatedElement.class);`
`258`	`256`
`259`		`- private void registerMetadataForReflection(DuringAnalysisAccessImpl access, Executable reflectMethod) {`
	`257`	`+ private void registerMetadataForReflection(DuringAnalysisAccessImpl access, AnalysisMethod analysisMethod, Executable reflectMethod) {`
`260`	`258`	`/*`
`261`	`259`	`* Reflection signature parsing will try to instantiate classes via Class.forName().`
`262`	`260`	`*/`
`263`		`- makeTypeReachable(access, reflectMethod.getDeclaringClass(), false);`
	`261`	`+ makeTypeReachable(access, analysisMethod.getDeclaringClass().getJavaClass(), false);`
`264`	`262`	`for (TypeVariable<?> type : reflectMethod.getTypeParameters()) {`
`265`	`263`	`makeTypeReachable(access, type, true);`
`266`	`264`	`}`
`267`		`- for (Type paramType : reflectMethod.getGenericParameterTypes()) {`
	`265`	`+ for (Type paramType : analysisMethod.getGenericParameterTypes()) {`
`268`	`266`	`makeTypeReachable(access, paramType, true);`
`269`	`267`	`}`
`270`		`- if (reflectMethod instanceof Method) {`
	`268`	`+ if (!analysisMethod.isConstructor()) {`
`271`	`269`	`makeTypeReachable(access, ((Method) reflectMethod).getGenericReturnType(), true);`
`272`	`270`	`}`
`273`	`271`	`for (Type exceptionType : reflectMethod.getGenericExceptionTypes()) {`
`@@ -277,7 +275,7 @@ private void registerMetadataForReflection(DuringAnalysisAccessImpl access, Exec`
`277`	`275`	`/*`
`278`	`276`	`* Enable runtime parsing of annotations`
`279`	`277`	`*/`
`280`		`- for (Annotation annotation : GuardedAnnotationAccess.getDeclaredAnnotations(reflectMethod)) {`
	`278`	`+ for (Annotation annotation : GuardedAnnotationAccess.getDeclaredAnnotations(analysisMethod)) {`
`281`	`279`	`makeTypeReachable(access, annotation.annotationType(), false);`
`282`	`280`	`}`
`283`	`281`	`for (Annotation[] parameterAnnotations : reflectMethod.getParameterAnnotations()) {`
`@@ -304,7 +302,7 @@ private void makeTypeReachable(DuringAnalysisAccessImpl access, Type type, boole`
`304`	`302`	`return;`
`305`	`303`	`}`
`306`	`304`	`seenTypes.add(type);`
`307`		`- if (type instanceof Class<?>) {`
	`305`	`+ if (type instanceof Class<?> && !SubstitutionReflectivityFilter.shouldExclude((Class<?>) type, access.getMetaAccess(), access.getUniverse())) {`
`308`	`306`	`Class<?> clazz = (Class<?>) type;`
`309`	`307`	`if (access.getMetaAccess().lookupJavaType(clazz).registerAsReachable()) {`
`310`	`308`	`access.requireAnalysisIteration();`