Register reflectively-accessed types as unsafe allocated

Author: loicottet

docs/reference-manual/native-image/ReachabilityMetadata.md

@@ -331,11 +331,10 @@ In case the field-value-access metadata is missing, the following methods will t
 
 ### Unsafe Allocation of a Type
 
-For unsafe allocation of a type via `sun.misc.Unsafe#allocateInstance(Class<?>)`, or from native code via `AllocObject(jClass)`, we must provide the following metadata:
+For unsafe allocation of a type via `sun.misc.Unsafe#allocateInstance(Class<?>)`, or from native code via `AllocObject(jClass)`, the type must be registered for reflection. The following metadata is sufficient:
 ```json
 {
-  "type": "FullyQualifiedUnsafeAllocatedType",
-  "unsafeAllocated": true
+  "type": "FullyQualifiedUnsafeAllocatedType"
 }
 ```
 Otherwise, these methods will throw a `MissingReflectionRegistrationError`.
@@ -360,8 +359,7 @@ The overall definition of a type in JSON can have the following values:
   "allDeclaredMethods": true,
   "allPublicMethods": true,
   "allDeclaredFields": true,
-  "allPublicFields": true,
-  "unsafeAllocated": true
+  "allPublicFields": true
 }
 ```
 
@@ -430,13 +428,12 @@ As a convenience, one can allow method invocation for groups of methods by addin
 `allDeclaredConstructors` and `allDeclaredMethods` allow calls invocations of methods declared on a given type.
 `allPublicConstructors` and `allPublicMethods` allow invocations of all public methods defined on a type and all of its supertypes.
 
-To allocate objects of a type with `AllocObject`, the metadata must be stored in the `reflection` section:
+To allocate objects of a type with `AllocObject`, the type must be registered in the `reflection` section:
 ```json
 {
   "reflection": [
     {
-      "type": "jni.accessed.Type",
-      "unsafeAllocated": true
+      "type": "jni.unsafe.allocated.Type"
     }
   ]
 }
@@ -698,8 +695,7 @@ See below is a sample reachability metadata configuration that you can use in _r
       "allDeclaredFields": true,
       "allPublicFields": true,
       "allDeclaredMethods": true,
-      "allPublicMethods": true,
-      "unsafeAllocated": true
+      "allPublicMethods": true
     }
   ],
   "jni": [

sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ReflectionRegistry.java

@@ -46,10 +46,10 @@
 
 public interface ReflectionRegistry {
     default void register(ConfigurationCondition condition, Class<?>... classes) {
-        Arrays.stream(classes).forEach(clazz -> register(condition, false, clazz));
+        Arrays.stream(classes).forEach(clazz -> register(condition, clazz));
     }
 
-    void register(ConfigurationCondition condition, boolean unsafeAllocated, Class<?> clazz);
+    void register(ConfigurationCondition condition, Class<?> clazz);
 
     void register(ConfigurationCondition condition, boolean queriedOnly, Executable... methods);
 

substratevm/CHANGELOG.md

@@ -17,6 +17,7 @@ At runtime, premain runtime options are set along with main class' arguments in
 * (GR-58914) `ActiveProcessorCount` must be set at isolate or VM creation time.
 * (GR-59326) Ensure builder ForkJoin commonPool parallelism always respects NativeImageOptions.NumberOfThreads.
 * (GR-60234) Remove `"customTargetConstructorClass"` field from the serialization JSON metadata. All possible constructors are now registered by default when registering a type for serialization.
+* (GR-60235) Remove `"unsafeAllocated"` field from the reflection JSON metadata. All reflectively-accessible types are now automatically registered for unsafe allocation.
 
 ## GraalVM for JDK 23 (Internal Version 24.1.0)
 * (GR-51520) The old class initialization strategy, which was deprecated in GraalVM for JDK 22, is removed. The option `StrictImageHeap` no longer has any effect.

substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ConfigurationType.java

@@ -100,7 +100,6 @@ static ConfigurationType copyAndMerge(ConfigurationType type, ConfigurationType
     private boolean allPublicClasses;
     private ConfigurationMemberAccessibility allDeclaredFieldsAccess = ConfigurationMemberAccessibility.NONE;
     private ConfigurationMemberAccessibility allPublicFieldsAccess = ConfigurationMemberAccessibility.NONE;
-    private boolean unsafeAllocated;
     private ConfigurationMemberAccessibility allDeclaredMethodsAccess = ConfigurationMemberAccessibility.NONE;
     private ConfigurationMemberAccessibility allPublicMethodsAccess = ConfigurationMemberAccessibility.NONE;
     private ConfigurationMemberAccessibility allDeclaredConstructorsAccess = ConfigurationMemberAccessibility.NONE;
@@ -281,7 +280,6 @@ private void setFlagsFromOther(ConfigurationType other, BiPredicate<Boolean, Boo
         allPublicClasses = flagPredicate.test(allPublicClasses, other.allPublicClasses);
         allDeclaredFieldsAccess = accessCombiner.apply(allDeclaredFieldsAccess, other.allDeclaredFieldsAccess);
         allPublicFieldsAccess = accessCombiner.apply(allPublicFieldsAccess, other.allPublicFieldsAccess);
-        unsafeAllocated = flagPredicate.test(unsafeAllocated, other.unsafeAllocated);
         allDeclaredMethodsAccess = accessCombiner.apply(allDeclaredMethodsAccess, other.allDeclaredMethodsAccess);
         allPublicMethodsAccess = accessCombiner.apply(allPublicMethodsAccess, other.allPublicMethodsAccess);
         allDeclaredConstructorsAccess = accessCombiner.apply(allDeclaredConstructorsAccess, other.allDeclaredConstructorsAccess);
@@ -411,10 +409,6 @@ public synchronized void setAllPublicClasses() {
         allPublicClasses = true;
     }
 
-    public void setUnsafeAllocated() {
-        this.unsafeAllocated = true;
-    }
-
     public synchronized void setAllDeclaredFields(ConfigurationMemberAccessibility accessibility) {
         if (!allDeclaredFieldsAccess.includes(accessibility)) {
             allDeclaredFieldsAccess = accessibility;
@@ -470,7 +464,6 @@ public synchronized void printJson(JsonWriter writer) throws IOException {
         printJsonBooleanIfSet(writer, allPublicMethodsAccess == ConfigurationMemberAccessibility.ACCESSED, "allPublicMethods");
         printJsonBooleanIfSet(writer, allDeclaredConstructorsAccess == ConfigurationMemberAccessibility.ACCESSED, "allDeclaredConstructors");
         printJsonBooleanIfSet(writer, allPublicConstructorsAccess == ConfigurationMemberAccessibility.ACCESSED, "allPublicConstructors");
-        printJsonBooleanIfSet(writer, unsafeAllocated, "unsafeAllocated");
 
         if (fields != null) {
             writer.appendSeparator().quote("fields").appendFieldSeparator();

substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ParserConfigurationAdapter.java

@@ -81,12 +81,6 @@ public boolean registerAllConstructors(UnresolvedConfigurationCondition conditio
         return true;
     }
 
-    @Override
-    public void registerUnsafeAllocated(UnresolvedConfigurationCondition condition, ConfigurationType type) {
-        VMError.guarantee(condition.equals(type.getCondition()), "condition is here part of the type");
-        type.setUnsafeAllocated();
-    }
-
     @Override
     public void registerMethod(UnresolvedConfigurationCondition condition, boolean queriedOnly, ConfigurationType type, String methodName, List<ConfigurationType> methodParameterTypes) {
         VMError.guarantee(condition.equals(type.getCondition()), "condition is already a part of the type");

substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/trace/JniProcessor.java

@@ -89,10 +89,10 @@ void processEntry(EconomicMap<String, ?> entry, ConfigurationSet configurationSe
             case "AllocObject":
                 expectSize(args, 0);
                 /*
-                 * AllocObject is implemented via Unsafe.allocateInstance, so we need to set the
-                 * "unsafe allocated" flag in the reflection configuration file.
+                 * AllocObject is implemented via Unsafe.allocateInstance, so we need to add the
+                 * type to the reflection configuration file.
                  */
-                configurationSet.getReflectionConfiguration().getOrCreateType(condition, clazz).setUnsafeAllocated();
+                configurationSet.getReflectionConfiguration().getOrCreateType(condition, clazz);
                 break;
             case "GetStaticMethodID":
             case "GetMethodID": {

substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/trace/ReflectionProcessor.java

@@ -283,7 +283,8 @@ public void processEntry(EconomicMap<String, ?> entry, ConfigurationSet configur
                 break;
             }
             case "allocateInstance": {
-                configuration.getOrCreateType(condition, clazz).setUnsafeAllocated();
+                /* Reflectively-accessed types are unsafe-allocatable by default */
+                configuration.getOrCreateType(condition, clazz);
                 break;
             }
             default:

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/LegacyReflectionConfigurationParser.java

@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import com.oracle.svm.util.LogUtils;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.MapCursor;
 import org.graalvm.nativeimage.impl.UnresolvedConfigurationCondition;
@@ -56,6 +57,8 @@ public void parseAndRegister(Object json, URI origin) {
         parseClassArray(asList(json, "first level of document must be an array of class descriptors"));
     }
 
+    private boolean unsafeAllocatedWarningTriggered = false;
+
     @Override
     protected void parseClass(EconomicMap<String, Object> data) {
         checkAttributes(data, "reflection class descriptor object", List.of(NAME_KEY), OPTIONAL_REFLECT_CONFIG_OBJECT_ATTRS);
@@ -93,6 +96,11 @@ protected void parseClass(EconomicMap<String, Object> data) {
         T clazz = result.get();
         delegate.registerType(conditionResult.get(), clazz);
 
+        if (!unsafeAllocatedWarningTriggered && data.containsKey("unsafeAllocated")) {
+            unsafeAllocatedWarningTriggered = true;
+            LogUtils.warning("\"unsafeAllocated\" is deprecated in reflection-config.json. All reflectively-accessed classes can be instantiated through unsafe without the use of the flag.");
+        }
+
         registerIfNotDefault(data, false, clazz, "allDeclaredConstructors", () -> delegate.registerDeclaredConstructors(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allPublicConstructors", () -> delegate.registerPublicConstructors(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allDeclaredMethods", () -> delegate.registerDeclaredMethods(condition, false, clazz));
@@ -117,7 +125,6 @@ protected void parseClass(EconomicMap<String, Object> data) {
             delegate.registerDeclaredFields(queryCondition, true, clazz);
             delegate.registerPublicFields(queryCondition, true, clazz);
         }
-        registerIfNotDefault(data, false, clazz, "unsafeAllocated", () -> delegate.registerUnsafeAllocated(condition, clazz));
         MapCursor<String, Object> cursor = data.getEntries();
         while (cursor.advance()) {
             String name = cursor.getKey();

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/ReflectionConfigurationParserDelegate.java

@@ -68,8 +68,6 @@ public interface ReflectionConfigurationParserDelegate<C, T> {
 
     boolean registerAllConstructors(C condition, boolean queriedOnly, T type);
 
-    void registerUnsafeAllocated(C condition, T clazz);
-
     String getTypeName(T type);
 
     String getSimpleName(T type);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/ReflectionMetadataParser.java

@@ -34,6 +34,7 @@
 import org.graalvm.nativeimage.impl.UnresolvedConfigurationCondition;
 
 import com.oracle.svm.core.TypeResult;
+import com.oracle.svm.util.LogUtils;
 
 class ReflectionMetadataParser<C, T> extends ReflectionConfigurationParser<C, T> {
     private static final List<String> OPTIONAL_REFLECT_METADATA_ATTRS = Arrays.asList(CONDITIONAL_KEY,
@@ -56,6 +57,8 @@ public void parseAndRegister(Object json, URI origin) {
         }
     }
 
+    private boolean unsafeAllocatedWarningTriggered = false;
+
     @Override
     protected void parseClass(EconomicMap<String, Object> data) {
         checkAttributes(data, "reflection class descriptor object", List.of(TYPE_KEY), OPTIONAL_REFLECT_METADATA_ATTRS);
@@ -84,7 +87,7 @@ protected void parseClass(EconomicMap<String, Object> data) {
 
         C queryCondition = conditionResolver.alwaysTrue();
         T clazz = result.get();
-        delegate.registerType(conditionResult.get(), clazz);
+        delegate.registerType(condition, clazz);
 
         delegate.registerDeclaredClasses(queryCondition, clazz);
         delegate.registerRecordComponents(queryCondition, clazz);
@@ -98,14 +101,17 @@ protected void parseClass(EconomicMap<String, Object> data) {
         delegate.registerPublicMethods(queryCondition, true, clazz);
         delegate.registerDeclaredFields(queryCondition, true, clazz);
         delegate.registerPublicFields(queryCondition, true, clazz);
+        if (!unsafeAllocatedWarningTriggered && data.containsKey("unsafeAllocated")) {
+            unsafeAllocatedWarningTriggered = true;
+            LogUtils.warning("\"unsafeAllocated\" is deprecated in reachability-metadata.json. All reflectively-accessed classes can be instantiated through unsafe without the use of the flag.");
+        }
 
         registerIfNotDefault(data, false, clazz, "allDeclaredConstructors", () -> delegate.registerDeclaredConstructors(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allPublicConstructors", () -> delegate.registerPublicConstructors(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allDeclaredMethods", () -> delegate.registerDeclaredMethods(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allPublicMethods", () -> delegate.registerPublicMethods(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allDeclaredFields", () -> delegate.registerDeclaredFields(condition, false, clazz));
         registerIfNotDefault(data, false, clazz, "allPublicFields", () -> delegate.registerPublicFields(condition, false, clazz));
-        registerIfNotDefault(data, false, clazz, "unsafeAllocated", () -> delegate.registerUnsafeAllocated(condition, clazz));
 
         MapCursor<String, Object> cursor = data.getEntries();
         while (cursor.advance()) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/config/RegistryAdapter.java

@@ -28,7 +28,6 @@
 
 import java.lang.reflect.Executable;
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -223,17 +222,6 @@ public boolean registerAllConstructors(ConfigurationCondition condition, boolean
         return methods.length > 0;
     }
 
-    @Override
-    public void registerUnsafeAllocated(ConfigurationCondition condition, Class<?> clazz) {
-        if (!clazz.isArray() && !clazz.isInterface() && !Modifier.isAbstract(clazz.getModifiers())) {
-            registry.register(condition, true, clazz);
-            /*
-             * Ignore otherwise as the implementation of allocateInstance will anyhow throw an
-             * exception.
-             */
-        }
-    }
-
     @Override
     public void registerMethod(ConfigurationCondition condition, boolean queriedOnly, Class<?> type, String methodName, List<Class<?>> methodParameterTypes) throws NoSuchMethodException {
         try {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java

@@ -219,8 +219,7 @@ private class JNIRuntimeAccessibilitySupportImpl extends ConditionalConfiguratio
                     implements RuntimeJNIAccessSupport {
 
         @Override
-        public void register(ConfigurationCondition condition, boolean unsafeAllocated, Class<?> clazz) {
-            assert !unsafeAllocated : "unsafeAllocated can be only set via Unsafe.allocateInstance, not via JNI.";
+        public void register(ConfigurationCondition condition, Class<?> clazz) {
             Objects.requireNonNull(clazz, () -> nullErrorMessage("class"));
             abortIfSealed();
             registerConditionalConfiguration(condition, (cnd) -> newClasses.add(clazz));
@@ -252,7 +251,7 @@ private void registerFields(boolean finalIsWritable, Field[] fields) {
         @Override
         public void registerClassLookup(ConfigurationCondition condition, String typeName) {
             try {
-                register(condition, false, Class.forName(typeName));
+                register(condition, Class.forName(typeName));
             } catch (ClassNotFoundException e) {
                 newNegativeClassLookups.add(typeName);
             }

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

@@ -197,9 +197,9 @@ private boolean isQueryFlagSet(Class<?> clazz, int flag) {
     }
 
     @Override
-    public void register(ConfigurationCondition condition, boolean unsafeInstantiated, Class<?> clazz) {
+    public void register(ConfigurationCondition condition, Class<?> clazz) {
         Objects.requireNonNull(clazz, () -> nullErrorMessage("class"));
-        runConditionalInAnalysisTask(condition, (cnd) -> registerClass(cnd, clazz, unsafeInstantiated, true));
+        runConditionalInAnalysisTask(condition, (cnd) -> registerClass(cnd, clazz, true));
     }
 
     @Override
@@ -213,7 +213,7 @@ public void registerAllClassesQuery(ConfigurationCondition condition, Class<?> c
                         /* Malformed inner classes can have no declaring class */
                         innerClasses.computeIfAbsent(innerClass.getDeclaringClass(), c -> ConcurrentHashMap.newKeySet()).add(innerClass);
                     }
-                    registerClass(cnd, innerClass, false, !MissingRegistrationUtils.throwMissingRegistrationErrors());
+                    registerClass(cnd, innerClass, !MissingRegistrationUtils.throwMissingRegistrationErrors());
                 }
             } catch (LinkageError e) {
                 registerLinkageError(clazz, e, classLookupExceptions);
@@ -239,22 +239,22 @@ public void registerAllDeclaredClassesQuery(ConfigurationCondition condition, Cl
             try {
                 for (Class<?> innerClass : clazz.getDeclaredClasses()) {
                     innerClasses.computeIfAbsent(clazz, c -> ConcurrentHashMap.newKeySet()).add(innerClass);
-                    registerClass(cnd, innerClass, false, !MissingRegistrationUtils.throwMissingRegistrationErrors());
+                    registerClass(cnd, innerClass, !MissingRegistrationUtils.throwMissingRegistrationErrors());
                 }
             } catch (LinkageError e) {
                 registerLinkageError(clazz, e, classLookupExceptions);
             }
         });
     }
 
-    private void registerClass(ConfigurationCondition condition, Class<?> clazz, boolean unsafeInstantiated, boolean allowForName) {
+    private void registerClass(ConfigurationCondition condition, Class<?> clazz, boolean allowForName) {
         if (shouldExcludeClass(clazz)) {
             return;
         }
 
         AnalysisType type = metaAccess.lookupJavaType(clazz);
         type.registerAsReachable("Is registered for reflection.");
-        if (unsafeInstantiated) {
+        if (type.isArray() || (type.isInstanceClass() && !type.isAbstract())) {
             type.registerAsUnsafeAllocated("Is registered via reflection metadata.");
             classForNameSupport.registerUnsafeAllocated(condition, clazz);
         }
@@ -291,7 +291,7 @@ public void registerClassLookupException(ConfigurationCondition condition, Strin
     public void registerClassLookup(ConfigurationCondition condition, String typeName) {
         runConditionalInAnalysisTask(condition, (cnd) -> {
             try {
-                registerClass(cnd, Class.forName(typeName, false, ClassLoader.getSystemClassLoader()), false, true);
+                registerClass(cnd, Class.forName(typeName, false, ClassLoader.getSystemClassLoader()), true);
             } catch (ClassNotFoundException e) {
                 classForNameSupport.registerNegativeQuery(cnd, typeName);
             } catch (Throwable t) {
@@ -316,7 +316,7 @@ public void registerAllPermittedSubclassesQuery(ConfigurationCondition condition
             setQueryFlag(clazz, ALL_PERMITTED_SUBCLASSES_FLAG);
             if (clazz.isSealed()) {
                 for (Class<?> permittedSubclass : clazz.getPermittedSubclasses()) {
-                    registerClass(condition, permittedSubclass, false, false);
+                    registerClass(condition, permittedSubclass, false);
                 }
             }
         });
@@ -329,7 +329,7 @@ public void registerAllNestMembersQuery(ConfigurationCondition condition, Class<
             setQueryFlag(clazz, ALL_NEST_MEMBERS_FLAG);
             for (Class<?> nestMember : clazz.getNestMembers()) {
                 if (nestMember != clazz) {
-                    registerClass(condition, nestMember, false, false);
+                    registerClass(condition, nestMember, false);
                 }
             }
         });