[GR-50432] Allow fields to be registered for reflection without being made reachable

PullRequest: graal/16267

Author: loicottet

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/MissingReflectionRegistrationUtils.java

@@ -56,6 +56,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) {
@@ -69,15 +80,15 @@ public static void forMethod(Class<?> declaringClass, String methodName, Class<?
         report(exception);
     }
 
-    public static void forQueriedOnlyExecutable(Executable executable) {
+    public static MissingReflectionRegistrationError errorForQueriedOnlyExecutable(Executable executable) {
         MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("invoke method", executable.toString()),
                         executable.getClass(), executable.getDeclaringClass(), executable.getName(), executable.getParameterTypes());
         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.
          */
-        throw exception;
+        return exception;
     }
 
     public static void forBulkQuery(Class<?> declaringClass, String methodName) {
@@ -87,7 +98,7 @@ public static void forBulkQuery(Class<?> declaringClass, String methodName) {
         report(exception);
     }
 
-    public static void forProxy(Class<?>... interfaces) {
+    public static MissingReflectionRegistrationError errorForProxy(Class<?>... interfaces) {
         MissingReflectionRegistrationError exception = new MissingReflectionRegistrationError(errorMessage("access the proxy class inheriting",
                         Arrays.toString(Arrays.stream(interfaces).map(Class::getTypeName).toArray()),
                         "The order of interfaces used to create proxies matters.", "dynamic-proxy"),
@@ -97,7 +108,7 @@ public static void forProxy(Class<?>... interfaces) {
          * If report doesn't throw, we throw the exception anyway since this is a Native
          * Image-specific error that is unrecoverable in any case.
          */
-        throw exception;
+        return exception;
     }
 
     private static String errorMessage(String failedAction, String elementDescriptor) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/proxy/DynamicProxySupport.java

@@ -31,7 +31,6 @@
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.Equivalence;
-import jdk.graal.compiler.debug.GraalError;
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 import org.graalvm.nativeimage.hosted.RuntimeClassInitialization;
@@ -47,6 +46,8 @@
 import com.oracle.svm.util.LogUtils;
 import com.oracle.svm.util.ReflectionUtil;
 
+import jdk.graal.compiler.debug.GraalError;
+
 public class DynamicProxySupport implements DynamicProxyRegistry {
 
     public static final Pattern PROXY_CLASS_NAME_PATTERN = Pattern.compile(".*\\$Proxy[0-9]+");
@@ -173,7 +174,7 @@ public Class<?> getProxyClass(ClassLoader loader, Class<?>... interfaces) {
         ProxyCacheKey key = new ProxyCacheKey(interfaces);
         Object clazzOrError = proxyCache.get(key);
         if (clazzOrError == null) {
-            MissingReflectionRegistrationUtils.forProxy(interfaces);
+            throw MissingReflectionRegistrationUtils.errorForProxy(interfaces);
         }
         if (clazzOrError instanceof Throwable) {
             throw new GraalError((Throwable) clazzOrError);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Constructor.java

@@ -71,7 +71,7 @@ public native void constructor(Class<?> declaringClass, Class<?>[] parameterType
     @Substitute
     Target_jdk_internal_reflect_ConstructorAccessor acquireConstructorAccessor() {
         if (constructorAccessor == null) {
-            MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class));
+            throw MissingReflectionRegistrationUtils.errorForQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class));
         }
         return constructorAccessor;
     }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/target/Target_java_lang_reflect_Method.java

@@ -74,7 +74,7 @@ public native void constructor(Class<?> declaringClass, String name, Class<?>[]
     @Substitute
     public Target_jdk_internal_reflect_MethodAccessor acquireMethodAccessor() {
         if (methodAccessor == null) {
-            MissingReflectionRegistrationUtils.forQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class));
+            throw MissingReflectionRegistrationUtils.errorForQueriedOnlyExecutable(SubstrateUtil.cast(this, Executable.class));
         }
         return methodAccessor;
     }

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/reflect/ReflectionDataBuilder.java

@@ -446,50 +446,58 @@ 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, (cnd) -> {
             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);
+    }
+
+    private void registerAllFieldsQuery(ConfigurationCondition condition, boolean queriedOnly, Class<?> clazz) {
         checkNotSealed();
         for (Class<?> current = clazz; current != null; current = current.getSuperclass()) {
             final Class<?> currentLambda = current;
             registerConditionalConfiguration(condition, (cnd) -> setQueryFlag(currentLambda, ALL_FIELDS_FLAG));
         }
         try {
-            registerInternal(condition, clazz.getFields());
+            registerInternal(condition, queriedOnly, clazz.getFields());
         } catch (LinkageError e) {
             registerLinkageError(clazz, e, fieldLookupExceptions);
         }
     }
 
     @Override
     public void registerAllDeclaredFieldsQuery(ConfigurationCondition condition, Class<?> clazz) {
+        registerAllDeclaredFieldsQuery(condition, false, clazz);
+    }
+
+    private void registerAllDeclaredFieldsQuery(ConfigurationCondition condition, boolean queriedOnly, Class<?> clazz) {
         checkNotSealed();
         registerConditionalConfiguration(condition, (cnd) -> setQueryFlag(clazz, ALL_DECLARED_FIELDS_FLAG));
         try {
-            registerInternal(condition, clazz.getDeclaredFields());
+            registerInternal(condition, queriedOnly, clazz.getDeclaredFields());
         } catch (LinkageError e) {
             registerLinkageError(clazz, e, fieldLookupExceptions);
         }
     }
 
-    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();
 
             /*
@@ -504,13 +512,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,
                             (cnd) -> negativeFieldLookups.computeIfAbsent(metaAccess.lookupJavaType(declaringClass), (key) -> ConcurrentHashMap.newKeySet()).add(fieldName));
@@ -680,13 +696,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
@@ -1045,7 +1063,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);
             }
@@ -1155,4 +1173,10 @@ private static void requireNonNull(Object[] values, String kind) {
     private static String nullErrorMessage(String kind) {
         return "Cannot register null value as " + kind + " for reflection. Please ensure that all values you register are not null.";
     }
+
+    public static class TestBackdoor {
+        public static void registerField(ReflectionDataBuilder reflectionDataBuilder, boolean queriedOnly, Field field) {
+            reflectionDataBuilder.registerInternal(ConfigurationCondition.alwaysTrue(), queriedOnly, field);
+        }
+    }
 }