Skip to content

Commit 4d94f40

Browse files
loicottetloicottet
committed
Fix reflection metadata-related test failures
1 parent dbbe854 commit 4d94f40

File tree

10 files changed

+235
-58
lines changed

10 files changed

+235
-58
lines changed

‎sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/RuntimeReflectionSupport.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ public interface RuntimeReflectionSupport extends ReflectionRegistry {
5656
Object getAccessor(Executable method);
5757

5858
/*
59-
* Returns the methods that shadow a superclass method registered for reflection, to be excluded
60-
* from reflection queries.
59+
* Returns the methods and fields that shadow a superclass element registered for reflection, to
60+
* be excluded from reflection queries.
6161
*/
62+
Set<?> getHidingReflectionFields();
63+
6264
Set<?> getHidingReflectionMethods();
6365

6466
Object[] getRecordComponents(Class<?> type);

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,9 @@ public void setPosition(int newPosition) {
408408
}
409409

410410
public int getPosition() {
411+
if (position == -1) {
412+
System.out.println("Found");
413+
}
411414
assert position != -1 : this;
412415
return position;
413416
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.lang.reflect.Field;
4040
import java.lang.reflect.GenericDeclaration;
4141
import java.lang.reflect.InvocationTargetException;
42+
import java.lang.reflect.Member;
4243
import java.lang.reflect.Method;
4344
import java.lang.reflect.Modifier;
4445
import java.lang.reflect.Type;
@@ -49,6 +50,7 @@
4950
import java.util.Arrays;
5051
import java.util.Collection;
5152
import java.util.List;
53+
import java.util.Objects;
5254
import java.util.Optional;
5355
import java.util.StringJoiner;
5456

@@ -899,8 +901,20 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
899901
@KeepOriginal
900902
private native Constructor<?>[] getConstructors();
901903

902-
@KeepOriginal
903-
private native Field getField(@SuppressWarnings("hiding") String name) throws NoSuchMethodException;
904+
@Substitute
905+
public Field getField(String fieldName) throws NoSuchFieldException, SecurityException {
906+
Objects.requireNonNull(fieldName);
907+
@SuppressWarnings("removal")
908+
SecurityManager sm = System.getSecurityManager();
909+
if (sm != null) {
910+
checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true);
911+
}
912+
Field field = getField0(fieldName);
913+
if (field == null || ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
914+
throw new NoSuchFieldException(fieldName);
915+
}
916+
return getReflectionFactory().copyField(field);
917+
}
904918

905919
@KeepOriginal
906920
private native Method getMethod(@SuppressWarnings("hiding") String name, Class<?>... parameterTypes) throws NoSuchMethodException;
@@ -923,8 +937,20 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
923937
@KeepOriginal
924938
private native Constructor<?>[] getDeclaredConstructors();
925939

926-
@KeepOriginal
927-
private native Field getDeclaredField(@SuppressWarnings("hiding") String name);
940+
@Substitute
941+
public Field getDeclaredField(String fieldName) throws NoSuchFieldException, SecurityException {
942+
Objects.requireNonNull(fieldName);
943+
@SuppressWarnings("removal")
944+
SecurityManager sm = System.getSecurityManager();
945+
if (sm != null) {
946+
checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true);
947+
}
948+
Field field = searchFields(privateGetDeclaredFields(false), fieldName);
949+
if (field == null || ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
950+
throw new NoSuchFieldException(fieldName);
951+
}
952+
return getReflectionFactory().copyField(field);
953+
}
928954

929955
@KeepOriginal
930956
private native Method getDeclaredMethod(@SuppressWarnings("hiding") String name, Class<?>... parameterTypes);
@@ -1009,8 +1035,16 @@ private static Method searchMethods(Method[] allMethods, String name, Class<?>[]
10091035
@KeepOriginal
10101036
private static native boolean arrayContentsEq(Object[] a1, Object[] a2);
10111037

1012-
@KeepOriginal
1013-
private static native Field[] copyFields(Field[] arg);
1038+
@Substitute
1039+
private static Field[] copyFields(Field[] original) {
1040+
Field[] arg = filterHidingFields(original);
1041+
Field[] out = new Field[arg.length];
1042+
ReflectionFactory fact = getReflectionFactory();
1043+
for (int i = 0; i < arg.length; i++) {
1044+
out[i] = fact.copyField(arg[i]);
1045+
}
1046+
return out;
1047+
}
10141048

10151049
/**
10161050
* @see #filterHidingMethods(Method...)
@@ -1463,14 +1497,24 @@ private Class<?>[] getPermittedSubclasses0() {
14631497
private native boolean isDirectSubType(Class<?> c);
14641498

14651499
/*
1466-
* We need to filter out hiding methods at the last moment. This ensures that the JDK internals
1467-
* see them as regular methods and ensure the visibility of methods is correct, but they should
1468-
* not be returned to application code.
1500+
* We need to filter out hiding elements at the last moment. This ensures that the JDK internals
1501+
* see them as regular methods and fields and ensure their visibility is correct, but they
1502+
* should not be returned to application code.
14691503
*/
1504+
private static Field[] filterHidingFields(Field... fields) {
1505+
List<Field> filtered = new ArrayList<>();
1506+
for (Field field : fields) {
1507+
if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
1508+
filtered.add(field);
1509+
}
1510+
}
1511+
return filtered.toArray(new Field[0]);
1512+
}
1513+
14701514
private static Method[] filterHidingMethods(Method... methods) {
14711515
List<Method> filtered = new ArrayList<>();
14721516
for (Method method : methods) {
1473-
if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHidingMethod(method.getModifiers())) {
1517+
if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(method.getModifiers())) {
14741518
filtered.add(method);
14751519
}
14761520
}
@@ -1687,7 +1731,7 @@ Method getMostSpecific() {
16871731
}
16881732
}
16891733
/* Filter out hiding methods after the retursive lookup is done */
1690-
return ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHidingMethod(m.getModifiers()) ? null : m;
1734+
return ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(m.getModifiers()) ? null : m;
16911735
}
16921736
}
16931737

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/ReflectionMetadataDecoder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public interface ReflectionMetadataDecoder {
5151

5252
byte[] parseByteArray(int index);
5353

54-
boolean isHidingMethod(int modifiers);
54+
boolean isHiding(int modifiers);
5555

5656
long getMetadataByteLength();
5757
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.concurrent.ForkJoinPool;
4949
import java.util.stream.Collectors;
5050

51+
import com.oracle.graal.pointsto.meta.AnalysisField;
5152
import org.graalvm.compiler.code.CompilationResult;
5253
import org.graalvm.compiler.code.DataSection;
5354
import org.graalvm.compiler.debug.DebugContext;
@@ -282,6 +283,21 @@ public void buildRuntimeMetadata(CFunctionPointer firstMethod, UnsignedWord code
282283
}
283284
}
284285

286+
for (Object field : reflectionSupport.getHidingReflectionFields()) {
287+
AnalysisField hidingField = (AnalysisField) field;
288+
HostedField hostedField = hUniverse.optionalLookup(hidingField);
289+
if (hostedField == null || !includedFields.contains(hostedField)) {
290+
HostedType declaringType = hUniverse.lookup(hidingField.getDeclaringClass());
291+
String name = hidingField.getName();
292+
HostedType type = hUniverse.lookup(hidingField.getType());
293+
int modifiers = hidingField.getModifiers();
294+
reflectionMetadataEncoder.addHidingFieldMetadata(hidingField, declaringType, name, type, modifiers);
295+
if (hostedField != null) {
296+
includedFields.add(hostedField);
297+
}
298+
}
299+
}
300+
285301
for (Object method : reflectionSupport.getHidingReflectionMethods()) {
286302
AnalysisMethod hidingMethod = (AnalysisMethod) method;
287303
HostedMethod hostedMethod = hUniverse.optionalLookup(hidingMethod);
@@ -624,6 +640,8 @@ public interface ReflectionMetadataEncoder {
624640

625641
void addHeapAccessibleObjectMetadata(MetaAccessProvider metaAccess, AccessibleObject object);
626642

643+
void addHidingFieldMetadata(AnalysisField analysisField, HostedType declType, String name, HostedType type, int modifiers);
644+
627645
void addHidingMethodMetadata(AnalysisMethod analysisMethod, HostedType declType, String name, HostedType[] paramTypes, int modifiers, HostedType returnType);
628646

629647
void addReachableFieldMetadata(HostedField field);

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/lambda/LambdaSubstitutionType.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,10 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
399399

400400
@Override
401401
public <T extends Annotation> T getDeclaredAnnotation(Class<T> annotationClass) {
402-
return original.getDeclaredAnnotation(annotationClass);
402+
if (annotationClass == LambdaFormHiddenMethod.class) {
403+
return annotationClass.cast(LambdaFormHiddenMethod.Holder.INSTANCE);
404+
}
405+
return null;
403406
}
404407

405408
@Override
@@ -409,7 +412,7 @@ public <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotati
409412

410413
@Override
411414
public Annotation[] getDeclaredAnnotations() {
412-
return original.getDeclaredAnnotations();
415+
return LambdaFormHiddenMethod.Holder.ARRAY;
413416
}
414417

415418
public ResolvedJavaType getOriginal() {

‎substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import com.oracle.svm.util.ModuleSupport;
8080

8181
import jdk.vm.ci.meta.JavaType;
82+
import jdk.vm.ci.meta.ResolvedJavaField;
8283
import jdk.vm.ci.meta.ResolvedJavaMethod;
8384
import jdk.vm.ci.meta.ResolvedJavaType;
8485
import sun.reflect.annotation.AnnotationType;
@@ -96,6 +97,7 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
9697
private final Map<Executable, ExecutableAccessibility> reflectionMethods = new ConcurrentHashMap<>();
9798
private final Map<Executable, Object> methodAccessors = new ConcurrentHashMap<>();
9899
private final Set<Field> reflectionFields = Collections.newSetFromMap(new ConcurrentHashMap<>());
100+
private final Set<AnalysisField> hidingFields = ConcurrentHashMap.newKeySet();
99101
private final Set<AnalysisMethod> hidingMethods = ConcurrentHashMap.newKeySet();
100102
private final Set<Executable> registeredMethods = ConcurrentHashMap.newKeySet();
101103
private final Set<Field> registeredFields = ConcurrentHashMap.newKeySet();
@@ -107,6 +109,7 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
107109
private final Set<Class<?>> processedClasses = new HashSet<>();
108110
private final Set<Type> processedTypes = new HashSet<>();
109111
private final Set<DynamicHub> processedDynamicHubs = new HashSet<>();
112+
private final Map<AnalysisField, Set<AnalysisType>> processedHidingFields = new HashMap<>();
110113
private final Map<AnalysisMethod, Set<AnalysisType>> processedHidingMethods = new HashMap<>();
111114
private final Set<AccessibleObject> processedHeapReflectionObjects = new HashSet<>();
112115

@@ -266,6 +269,7 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
266269
if (!registeredFields.contains(reflectField) && !SubstitutionReflectivityFilter.shouldExclude(reflectField, access.getMetaAccess(), access.getUniverse())) {
267270
AnalysisField analysisField = access.getMetaAccess().lookupJavaField(reflectField);
268271
registerTypesForField(access, analysisField, reflectField);
272+
registerHidingSubTypeFields(access, analysisField, analysisField.getDeclaringClass());
269273
registeredFields.add(reflectField);
270274
}
271275
}
@@ -291,13 +295,18 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
291295
if (!processedHeapReflectionObjects.contains(object)) {
292296
if (object instanceof Field) {
293297
Field field = (Field) object;
294-
AnalysisField analysisField = access.getMetaAccess().lookupJavaField(field);
295-
registerTypesForField(access, analysisField, field);
298+
if (!SubstitutionReflectivityFilter.shouldExclude(field, access.getMetaAccess(), access.getUniverse())) {
299+
AnalysisField analysisField = access.getMetaAccess().lookupJavaField(field);
300+
registerTypesForField(access, analysisField, field);
301+
registerHidingSubTypeFields(access, analysisField, analysisField.getDeclaringClass());
302+
}
296303
} else if (object instanceof Executable) {
297304
Executable executable = (Executable) object;
298-
AnalysisMethod analysisMethod = access.getMetaAccess().lookupJavaMethod(executable);
299-
registerTypesForMethod(access, analysisMethod, executable);
300-
registerHidingSubTypeMethods(access, analysisMethod, analysisMethod.getDeclaringClass());
305+
if (!SubstitutionReflectivityFilter.shouldExclude(executable, access.getMetaAccess(), access.getUniverse())) {
306+
AnalysisMethod analysisMethod = access.getMetaAccess().lookupJavaMethod(executable);
307+
registerTypesForMethod(access, analysisMethod, executable);
308+
registerHidingSubTypeMethods(access, analysisMethod, analysisMethod.getDeclaringClass());
309+
}
301310
}
302311
processedHeapReflectionObjects.add(object);
303312
}
@@ -316,6 +325,38 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
316325
}
317326
}
318327

328+
private void registerHidingSubTypeFields(DuringAnalysisAccess access, AnalysisField field, AnalysisType type) {
329+
if (!type.equals(field.getDeclaringClass()) && type.isReachable()) {
330+
if (!processedHidingFields.containsKey(field) || !processedHidingFields.get(field).contains(type)) {
331+
processedHidingFields.computeIfAbsent(field, m -> ConcurrentHashMap.newKeySet()).add(type);
332+
try {
333+
AnalysisField[] subClassFields = field.isStatic() ? type.getStaticFields() : type.getInstanceFields(false);
334+
for (AnalysisField subclassField : subClassFields) {
335+
if (subclassField.getName().equals(field.getName())) {
336+
hidingFields.add(subclassField);
337+
}
338+
}
339+
/*
340+
* Lookup can lead to the creation of new AnalysisField objects, so we need to
341+
* run another analysis iteration.
342+
*/
343+
access.requireAnalysisIteration();
344+
345+
} catch (UnsupportedFeatureException | LinkageError e) {
346+
/*
347+
* A field that is not supposed to end up in the image is considered as being
348+
* absent for reflection purposes.
349+
*/
350+
}
351+
}
352+
}
353+
for (AnalysisType subType : type.getSubTypes()) {
354+
if (!subType.equals(type)) {
355+
registerHidingSubTypeFields(access, field, subType);
356+
}
357+
}
358+
}
359+
319360
private void registerHidingSubTypeMethods(DuringAnalysisAccess access, AnalysisMethod method, AnalysisType type) {
320361
if (!type.equals(method.getDeclaringClass()) && type.isReachable()) {
321362
if (!processedHidingMethods.containsKey(method) || !processedHidingMethods.get(method).contains(type)) {
@@ -470,7 +511,6 @@ private static void registerTypesForReachableMethod(DuringAnalysisAccessImpl acc
470511
for (JavaType paramType : analysisMethod.toParameterTypes()) {
471512
makeAnalysisTypeReachable(access, (AnalysisType) paramType);
472513
}
473-
makeAnalysisTypeReachable(access, (AnalysisType) analysisMethod.getSignature().getReturnType(null));
474514
}
475515

476516
private void makeTypeReachable(DuringAnalysisAccessImpl access, Type type) {
@@ -779,6 +819,12 @@ public Object getAccessor(Executable method) {
779819
return methodAccessors.get(method);
780820
}
781821

822+
@Override
823+
public Set<ResolvedJavaField> getHidingReflectionFields() {
824+
assert sealed;
825+
return Collections.unmodifiableSet(hidingFields);
826+
}
827+
782828
@Override
783829
public Set<ResolvedJavaMethod> getHidingReflectionMethods() {
784830
assert sealed;

0 commit comments

Comments
 (0)