Backport GR-50432: Allow fields to be registered for reflection without being made reachable

Author: loicottet

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) {

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;
     }
 
 }

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

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

@@ -409,50 +409,58 @@ public void registerConstructorLookup(ConfigurationCondition condition, Class<?>
     @Override
     public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) {
         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 */
         }
     }
 
     @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();
 
             /*
@@ -467,13 +475,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));
         }
@@ -640,13 +656,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
@@ -998,7 +1016,7 @@ public void registerHeapReflectionField(Field reflectField, ScanReason reason) {
                 throw new UnsupportedFeatureException("Registering new field for reflection when the image heap is already sealed: " + reflectField);
             }
             if (!SubstitutionReflectivityFilter.shouldExclude(reflectField, metaAccess, universe)) {
-                registerTypesForField(analysisField, reflectField);
+                registerTypesForField(analysisField, reflectField, false);
                 if (analysisField.getDeclaringClass().isAnnotation()) {
                     processAnnotationField(reflectField);
                 }