From 11f3306d6010aadefd044825c7e92151dc0a7ab8 Mon Sep 17 00:00:00 2001 From: Loic Ottet Date: Thu, 30 Nov 2023 11:35:47 +0100 Subject: [PATCH] Backport GR-50432: Allow fields to be registered for reflection without being made reachable --- .../MissingReflectionRegistrationUtils.java | 11 ++++ ...t_jdk_internal_misc_Unsafe_Reflection.java | 11 ++-- .../config/ReflectionRegistryAdapter.java | 9 ++-- .../hosted/reflect/ReflectionDataBuilder.java | 50 +++++++++++++------ 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java index 8ca8222d8cc..305ec0c30eb 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java @@ -54,6 +54,17 @@ public static void forField(Class declaringClass, String fieldName) { report(exception); } + public static MissingReflectionRegistrationError errorForQueriedOnlyField(Field field) { + MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("read or write field", field.toString()), + field.getClass(), field.getDeclaringClass(), field.getName(), null); + report(exception); + /* + * If report doesn't throw, we throw the exception anyway since this is a Native + * Image-specific error that is unrecoverable in any case. + */ + return exception; + } + public static void forMethod(Class declaringClass, String methodName, Class[] paramTypes) { StringJoiner paramTypeNames = new StringJoiner(", ", "(", ")"); if (paramTypes != null) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_jdk_internal_misc_Unsafe_Reflection.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_jdk_internal_misc_Unsafe_Reflection.java index 1fc5303ac94..98ed7c9b61e 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_jdk_internal_misc_Unsafe_Reflection.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_jdk_internal_misc_Unsafe_Reflection.java @@ -31,7 +31,7 @@ import com.oracle.svm.core.SubstrateUtil; import com.oracle.svm.core.annotate.Substitute; import com.oracle.svm.core.annotate.TargetClass; -import com.oracle.svm.core.util.VMError; +import com.oracle.svm.core.reflect.MissingReflectionRegistrationUtils; @TargetClass(className = "jdk.internal.misc.Unsafe") @SuppressWarnings({"static-method"}) @@ -80,13 +80,10 @@ static long getFieldOffset(Target_java_lang_reflect_Field field) { throw new NullPointerException(); } int offset = field.root == null ? field.offset : field.root.offset; - if (offset > 0) { - return offset; + if (offset <= 0) { + throw MissingReflectionRegistrationUtils.errorForQueriedOnlyField(SubstrateUtil.cast(field, Field.class)); } - throw VMError.unsupportedFeature("The offset of " + field + " is accessed without the field being first registered as unsafe accessed. " + - "Please register the field as unsafe accessed. You can do so with a reflection configuration that " + - "contains an entry for the field with the attribute \"allowUnsafeAccess\": true. Such a configuration " + - "file can be generated for you. Read BuildConfiguration.md and Reflection.md for details."); + return offset; } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/config/ReflectionRegistryAdapter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/config/ReflectionRegistryAdapter.java index 1d815eb71c0..c7bdb4043af 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/config/ReflectionRegistryAdapter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/config/ReflectionRegistryAdapter.java @@ -34,6 +34,7 @@ import com.oracle.svm.core.TypeResult; import com.oracle.svm.core.configure.ConfigurationTypeDescriptor; import com.oracle.svm.hosted.ImageClassLoader; +import com.oracle.svm.hosted.reflect.ReflectionDataBuilder; public class ReflectionRegistryAdapter extends RegistryAdapter { private final RuntimeReflectionSupport reflectionSupport; @@ -89,16 +90,12 @@ public void registerSigners(ConfigurationCondition condition, Class type) { @Override public void registerPublicFields(ConfigurationCondition condition, boolean queriedOnly, Class type) { - if (!queriedOnly) { - reflectionSupport.registerAllFieldsQuery(condition, type); - } + ((ReflectionDataBuilder) reflectionSupport).registerAllFieldsQuery(condition, queriedOnly, type); } @Override public void registerDeclaredFields(ConfigurationCondition condition, boolean queriedOnly, Class type) { - if (!queriedOnly) { - reflectionSupport.registerAllDeclaredFieldsQuery(condition, type); - } + ((ReflectionDataBuilder) reflectionSupport).registerAllDeclaredFieldsQuery(condition, queriedOnly, type); } @Override diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java index 0c714845534..933700269ee 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java @@ -414,26 +414,30 @@ public void registerConstructorLookup(ConfigurationCondition condition, Class public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) { requireNonNull(fields, "field"); checkNotSealed(); - registerInternal(condition, fields); + registerInternal(condition, false, fields); } - private void registerInternal(ConfigurationCondition condition, Field... fields) { + private void registerInternal(ConfigurationCondition condition, boolean queriedOnly, Field... fields) { register(analysisUniverse -> registerConditionalConfiguration(condition, () -> { for (Field field : fields) { - analysisUniverse.getBigbang().postTask(debug -> registerField(field)); + analysisUniverse.getBigbang().postTask(debug -> registerField(queriedOnly, field)); } })); } @Override public void registerAllFieldsQuery(ConfigurationCondition condition, Class clazz) { + registerAllFieldsQuery(condition, false, clazz); + } + + public void registerAllFieldsQuery(ConfigurationCondition condition, boolean queriedOnly, Class clazz) { checkNotSealed(); for (Class current = clazz; current != null; current = current.getSuperclass()) { final Class currentLambda = current; registerConditionalConfiguration(condition, () -> setQueryFlag(currentLambda, ALL_FIELDS_FLAG)); } try { - registerInternal(condition, clazz.getFields()); + registerInternal(condition, queriedOnly, clazz.getFields()); } catch (LinkageError e) { /* Ignore the error */ } @@ -441,23 +445,27 @@ public void registerAllFieldsQuery(ConfigurationCondition condition, Class cl @Override public void registerAllDeclaredFieldsQuery(ConfigurationCondition condition, Class clazz) { + registerAllDeclaredFieldsQuery(condition, false, clazz); + } + + public void registerAllDeclaredFieldsQuery(ConfigurationCondition condition, boolean queriedOnly, Class clazz) { checkNotSealed(); registerConditionalConfiguration(condition, () -> setQueryFlag(clazz, ALL_DECLARED_FIELDS_FLAG)); try { - registerInternal(condition, clazz.getDeclaredFields()); + registerInternal(condition, queriedOnly, clazz.getDeclaredFields()); } catch (LinkageError e) { /* Ignore the error */ } } - private void registerField(Field reflectField) { + private void registerField(boolean queriedOnly, Field reflectField) { if (SubstitutionReflectivityFilter.shouldExclude(reflectField, metaAccess, universe)) { return; } AnalysisField analysisField = metaAccess.lookupJavaField(reflectField); if (registeredFields.put(analysisField, reflectField) == null) { - registerTypesForField(analysisField, reflectField); + registerTypesForField(analysisField, reflectField, true); AnalysisType declaringClass = analysisField.getDeclaringClass(); /* @@ -472,13 +480,21 @@ private void registerField(Field reflectField) { processAnnotationField(reflectField); } } + + /* + * We need to run this even if the method has already been registered, in case it was only + * registered as queried. + */ + if (!queriedOnly) { + registerTypesForField(analysisField, reflectField, false); + } } @Override public void registerFieldLookup(ConfigurationCondition condition, Class declaringClass, String fieldName) { checkNotSealed(); try { - registerInternal(condition, declaringClass.getDeclaredField(fieldName)); + registerInternal(condition, false, declaringClass.getDeclaredField(fieldName)); } catch (NoSuchFieldException e) { registerConditionalConfiguration(condition, () -> negativeFieldLookups.computeIfAbsent(metaAccess.lookupJavaType(declaringClass), (key) -> ConcurrentHashMap.newKeySet()).add(fieldName)); } @@ -645,13 +661,15 @@ private Object[] getEnclosingMethodInfo(Class clazz) { } } - private void registerTypesForField(AnalysisField analysisField, Field reflectField) { - /* - * Reflection accessors use Unsafe, so ensure that all reflectively accessible fields are - * registered as unsafe-accessible, whether they have been explicitly registered or their - * Field object is reachable in the image heap. - */ - analysisField.registerAsUnsafeAccessed("is registered for reflection."); + private void registerTypesForField(AnalysisField analysisField, Field reflectField, boolean queriedOnly) { + if (!queriedOnly) { + /* + * Reflection accessors use Unsafe, so ensure that all reflectively accessible fields + * are registered as unsafe-accessible, whether they have been explicitly registered or + * their Field object is reachable in the image heap. + */ + analysisField.registerAsUnsafeAccessed("is registered for reflection."); + } /* * The generic signature is parsed at run time, so we need to make all the types necessary @@ -996,7 +1014,7 @@ public void registerHeapReflectionField(Field reflectField, ScanReason reason) { assert !sealed; AnalysisField analysisField = metaAccess.lookupJavaField(reflectField); if (heapFields.put(analysisField, reflectField) == null && !SubstitutionReflectivityFilter.shouldExclude(reflectField, metaAccess, universe)) { - registerTypesForField(analysisField, reflectField); + registerTypesForField(analysisField, reflectField, false); if (analysisField.getDeclaringClass().isAnnotation()) { processAnnotationField(reflectField); }