svm: Always use SubstrateConstructorAccessor for serialization (adopt…

… removal of SerializationConstructorAccessorGenerator in JDK-8327624) [GR-59282]

substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java

@@ -1300,7 +1300,8 @@ private static void onClassFileLoadHook(@SuppressWarnings("unused") JvmtiEnv jvm
                 return;
             }
         }
-        if (jniFunctions().getIsInstanceOf().invoke(jni, loader, agent.handles().jdkInternalReflectDelegatingClassLoader)) {
+        JNIObjectHandle jdkInternalReflectDelegatingClassLoader = agent.handles().jdkInternalReflectDelegatingClassLoader;
+        if (!jdkInternalReflectDelegatingClassLoader.equal(nullHandle()) && jniFunctions().getIsInstanceOf().invoke(jni, loader, jdkInternalReflectDelegatingClassLoader)) {
             return;
         }
         byte[] data = new byte[classDataLen];

substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/NativeImageAgentJNIHandleSet.java

@@ -114,11 +114,8 @@ public class NativeImageAgentJNIHandleSet extends JNIHandleSet {
         javaLangClassLoader = newClassGlobalRef(env, "java/lang/ClassLoader");
         javaLangClassLoaderGetResource = getMethodId(env, javaLangClassLoader, "getResource", "(Ljava/lang/String;)Ljava/net/URL;", false);
 
-        JNIObjectHandle reflectLoader = findClassOptional(env, "jdk/internal/reflect/DelegatingClassLoader"); // JDK11+
-        if (reflectLoader.equal(nullHandle())) {
-            reflectLoader = findClass(env, "sun/reflect/DelegatingClassLoader"); // JDK 8
-        }
-        jdkInternalReflectDelegatingClassLoader = newTrackedGlobalRef(env, reflectLoader);
+        JNIObjectHandle reflectLoader = findClassOptional(env, "jdk/internal/reflect/DelegatingClassLoader"); // JDK11-23
+        jdkInternalReflectDelegatingClassLoader = reflectLoader.equal(nullHandle()) ? nullHandle() : newTrackedGlobalRef(env, reflectLoader);
 
         JNIObjectHandle javaLangObject = findClass(env, "java/lang/Object");
         javaLangObjectGetClass = getMethodId(env, javaLangObject, "getClass", "()Ljava/lang/Class;", false);

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

@@ -124,10 +124,12 @@
 import jdk.graal.compiler.core.common.NumUtil;
 import jdk.graal.compiler.core.common.SuppressFBWarnings;
 import jdk.graal.compiler.replacements.ReplacementsUtil;
+import jdk.graal.compiler.serviceprovider.JavaVersionUtil;
 import jdk.internal.access.JavaLangReflectAccess;
 import jdk.internal.misc.Unsafe;
 import jdk.internal.reflect.CallerSensitive;
 import jdk.internal.reflect.CallerSensitiveAdapter;
+import jdk.internal.reflect.ConstructorAccessor;
 import jdk.internal.reflect.FieldAccessor;
 import jdk.internal.reflect.Reflection;
 import jdk.internal.reflect.ReflectionFactory;
@@ -2114,7 +2116,7 @@ final class Target_jdk_internal_reflect_ReflectionFactory {
     private static ReflectionFactory soleInstance;
 
     @Alias //
-    private JavaLangReflectAccess langReflectAccess;
+    JavaLangReflectAccess langReflectAccess;
 
     /**
      * This substitution eliminates the SecurityManager check in the original method, which would
@@ -2145,6 +2147,36 @@ public FieldAccessor newFieldAccessor(Field field0, boolean override) {
         boolean isReadOnly = isFinal && (!override || langReflectAccess.isTrustedFinalField(field));
         return UnsafeFieldAccessorFactory.newFieldAccessor(field, isReadOnly);
     }
+
+    @Substitute
+    @TargetElement(onlyWith = JDKLatest.class)
+    private Constructor<?> generateConstructor(Class<?> cl, Constructor<?> constructorToCall) {
+        SerializationRegistry serializationRegistry = ImageSingletons.lookup(SerializationRegistry.class);
+        ConstructorAccessor acc = (ConstructorAccessor) serializationRegistry.getSerializationConstructorAccessor(cl, constructorToCall.getDeclaringClass());
+        /*
+         * Unlike other root constructors, this constructor is not copied for mutation but directly
+         * mutated, as it is not cached. To cache this constructor, setAccessible call must be done
+         * on a copy and return that copy instead.
+         */
+        Constructor<?> ctor = Helper_jdk_internal_reflect_ReflectionFactory.newConstructorWithAccessor(this, constructorToCall, acc);
+        ctor.setAccessible(true);
+        return ctor;
+    }
+
+}
+
+/**
+ * Reflectively access {@code JavaLangReflectAccess.newConstructorWithAccessor}. Once we drop JDK
+ * 21, this can be replaced by a direct call to the method. (GR-55515)
+ */
+final class Helper_jdk_internal_reflect_ReflectionFactory {
+    private static final Method NEW_CONSTRUCTOR_WITH_ACCESSOR = JavaVersionUtil.JAVA_SPEC > 21
+                    ? ReflectionUtil.lookupMethod(JavaLangReflectAccess.class, "newConstructorWithAccessor", Constructor.class, ConstructorAccessor.class)
+                    : null;
+
+    static Constructor<?> newConstructorWithAccessor(Target_jdk_internal_reflect_ReflectionFactory reflectionFactory, Constructor<?> constructorToCall, ConstructorAccessor acc) {
+        return ReflectionUtil.invokeMethod(NEW_CONSTRUCTOR_WITH_ACCESSOR, reflectionFactory.langReflectAccess, constructorToCall, acc);
+    }
 }
 
 /**

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/serialize/SerializationSupport.java

@@ -41,10 +41,12 @@
 import org.graalvm.nativeimage.impl.ConfigurationCondition;
 
 import com.oracle.svm.core.configure.RuntimeConditionSet;
+import com.oracle.svm.core.reflect.SubstrateConstructorAccessor;
 import com.oracle.svm.core.util.ImageHeapMap;
 import com.oracle.svm.core.util.VMError;
 
 import jdk.graal.compiler.java.LambdaUtils;
+import jdk.graal.compiler.serviceprovider.JavaVersionUtil;
 
 public class SerializationSupport implements SerializationRegistry {
 
@@ -92,7 +94,7 @@ private StubForAbstractClass() {
         }
     }
 
-    private final Constructor<?> stubConstructor;
+    private Constructor<?> stubConstructor;
 
     public static final class SerializationLookupKey {
         private final Class<?> declaringClass;
@@ -133,13 +135,20 @@ public int hashCode() {
     private final EconomicMap<SerializationLookupKey, Object> constructorAccessors;
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public SerializationSupport(Constructor<?> stubConstructor) {
+    public SerializationSupport() {
         constructorAccessors = ImageHeapMap.create();
+    }
+
+    public void setStubConstructor(Constructor<?> stubConstructor) {
+        VMError.guarantee(this.stubConstructor == null, "Cannot reset stubConstructor");
         this.stubConstructor = stubConstructor;
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
     public Object addConstructorAccessor(Class<?> declaringClass, Class<?> targetConstructorClass, Object constructorAccessor) {
+        if (JavaVersionUtil.JAVA_SPEC > 21) {
+            VMError.guarantee(constructorAccessor instanceof SubstrateConstructorAccessor, "Not a SubstrateConstructorAccessor: %s", constructorAccessor);
+        }
         SerializationLookupKey key = new SerializationLookupKey(declaringClass, targetConstructorClass);
         return constructorAccessors.putIfAbsent(key, constructorAccessor);
     }
@@ -214,6 +223,7 @@ public Object getSerializationConstructorAccessor(Class<?> rawDeclaringClass, Cl
             declaringClass = SerializedLambda.class;
         }
 
+        VMError.guarantee(stubConstructor != null, "Called too early, no stub constructor yet.");
         Class<?> targetConstructorClass = Modifier.isAbstract(declaringClass.getModifiers()) ? stubConstructor.getDeclaringClass() : rawTargetConstructorClass;
         Object constructorAccessor = constructorAccessors.get(new SerializationLookupKey(declaringClass, targetConstructorClass));
 

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

@@ -29,6 +29,7 @@
 import org.graalvm.nativeimage.hosted.FieldValueTransformer;
 
 import com.oracle.svm.core.annotate.RecomputeFieldValue;
+import com.oracle.svm.core.reflect.SubstrateAccessor;
 
 /**
  * Computes new values for the accessor fields of {@link Executable} subclasses, to be used instead
@@ -39,6 +40,14 @@
 public final class ExecutableAccessorComputer implements FieldValueTransformer {
     @Override
     public Object transform(Object receiver, Object originalValue) {
+        if (originalValue instanceof SubstrateAccessor) {
+            /*
+             * We do not want to replace existing SubstrateAccessors, since they might be more
+             * specialized (e.g., an explicit target class for a constructor accessor) than what
+             * would be created here.
+             */
+            return originalValue;
+        }
         return ReflectionSubstitutionSupport.singleton().getOrCreateAccessor((Executable) receiver);
     }
 }

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

@@ -40,6 +40,8 @@ static ReflectionSubstitutionSupport singleton() {
 
     SubstrateAccessor getOrCreateAccessor(Executable member);
 
+    SubstrateAccessor getOrCreateConstructorAccessor(Class<?> targetClass, Executable member);
+
     /** Offset of the field or -1 if the field was not registered for unsafe access. */
     int getFieldOffset(Field field, boolean checkUnsafeAccessed);
 

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java

@@ -1173,12 +1173,6 @@ private int completeImageBuild() {
         }
         imageClasspath.addAll(customImageClasspath);
 
-        /*
-         * Work around "JDK-8315810: Reimplement
-         * sun.reflect.ReflectionFactory::newConstructorForSerialization with method handles"
-         * [GR-48901]
-         */
-        imageBuilderJavaArgs.add("-Djdk.reflect.useOldSerializableConstructor=true");
         imageBuilderJavaArgs.add("-Djdk.internal.lambda.disableEagerInitialization=true");
         // The following two are for backwards compatibility reasons. They should be removed.
         imageBuilderJavaArgs.add("-Djdk.internal.lambda.eagerlyInitialize=false");

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/FactoryMethod.java

@@ -27,6 +27,7 @@
 import org.graalvm.nativeimage.ImageSingletons;
 
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
+import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.HostedProviders;
 import com.oracle.svm.core.NeverInlineTrivial;
 import com.oracle.svm.hosted.annotation.AnnotationValue;
@@ -49,11 +50,14 @@
 public final class FactoryMethod extends NonBytecodeMethod {
 
     private final ResolvedJavaMethod targetConstructor;
+    private final ResolvedJavaType instantiatedType;
     private final boolean throwAllocatedObject;
 
-    FactoryMethod(String name, ResolvedJavaMethod targetConstructor, ResolvedJavaType declaringClass, Signature signature, ConstantPool constantPool, boolean throwAllocatedObject) {
+    FactoryMethod(String name, ResolvedJavaMethod targetConstructor, ResolvedJavaType instantiatedType, ResolvedJavaType declaringClass, Signature signature, ConstantPool constantPool,
+                    boolean throwAllocatedObject) {
         super(name, true, declaringClass, signature, constantPool);
         this.targetConstructor = targetConstructor;
+        this.instantiatedType = instantiatedType;
         this.throwAllocatedObject = throwAllocatedObject;
 
         assert targetConstructor.isConstructor() : targetConstructor;
@@ -85,8 +89,9 @@ public StructuredGraph buildGraph(DebugContext debug, AnalysisMethod method, Hos
         FactoryMethodSupport support = ImageSingletons.lookup(FactoryMethodSupport.class);
 
         AnalysisMethod aTargetConstructor = kit.getMetaAccess().getUniverse().lookup(targetConstructor);
+        AnalysisType aInstantiatedType = kit.getMetaAccess().getUniverse().lookup(instantiatedType);
 
-        AbstractNewObjectNode newInstance = support.createNewInstance(kit, aTargetConstructor.getDeclaringClass(), true);
+        AbstractNewObjectNode newInstance = support.createNewInstance(kit, aInstantiatedType, true);
 
         ValueNode[] originalArgs = kit.getInitialArguments().toArray(ValueNode.EMPTY_ARRAY);
         ValueNode[] invokeArgs = new ValueNode[originalArgs.length + 1];

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/FactoryMethodSupport.java

@@ -32,6 +32,7 @@
 import com.oracle.graal.pointsto.infrastructure.ResolvedSignature;
 import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
+import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.code.FactoryMethodHolder;
 import com.oracle.svm.core.code.FactoryThrowMethodHolder;
@@ -54,18 +55,27 @@ public static FactoryMethodSupport singleton() {
         return ImageSingletons.lookup(FactoryMethodSupport.class);
     }
 
-    private final Map<AnalysisMethod, FactoryMethod> factoryMethods = new ConcurrentHashMap<>();
-    private final Map<AnalysisMethod, FactoryMethod> factoryThrowMethods = new ConcurrentHashMap<>();
+    private record ConstructorDescription(AnalysisMethod aConstructor, AnalysisType aInstantiatedType) {
+    }
+
+    private final Map<ConstructorDescription, FactoryMethod> factoryMethods = new ConcurrentHashMap<>();
+    private final Map<ConstructorDescription, FactoryMethod> factoryThrowMethods = new ConcurrentHashMap<>();
 
     public static boolean isFactoryMethod(AnalysisMethod method) {
         var javaClass = method.getDeclaringClass().getJavaClass();
         return javaClass == FactoryMethodHolder.class || javaClass == FactoryThrowMethodHolder.class;
     }
 
     public AnalysisMethod lookup(AnalysisMetaAccess aMetaAccess, AnalysisMethod aConstructor, boolean throwAllocatedObject) {
-        VMError.guarantee(aConstructor.getDeclaringClass().isInstanceClass() && !aConstructor.getDeclaringClass().isAbstract(), "Must be a non-abstract instance class");
-        Map<AnalysisMethod, FactoryMethod> methods = throwAllocatedObject ? factoryThrowMethods : factoryMethods;
-        FactoryMethod factoryMethod = methods.computeIfAbsent(aConstructor, key -> {
+        return lookup(aMetaAccess, aConstructor, aConstructor.getDeclaringClass(), throwAllocatedObject);
+    }
+
+    public AnalysisMethod lookup(AnalysisMetaAccess aMetaAccess, AnalysisMethod aConstructor, AnalysisType aInstantiatedType, boolean throwAllocatedObject) {
+        AnalysisType aInstType = aInstantiatedType == null ? aConstructor.getDeclaringClass() : aInstantiatedType;
+        VMError.guarantee(aConstructor.getDeclaringClass().isAssignableFrom(aInstType), "Must be assignable from");
+        VMError.guarantee(aInstType.isInstanceClass() && !aInstType.isAbstract(), "Must be a non-abstract instance class");
+        Map<ConstructorDescription, FactoryMethod> methods = throwAllocatedObject ? factoryThrowMethods : factoryMethods;
+        FactoryMethod factoryMethod = methods.computeIfAbsent(new ConstructorDescription(aConstructor, aInstType), key -> {
             /*
              * Computing the factory method name via the analysis universe ensures that type name
              * modifications, like to make lambda names unique, are incorporated in the name.
@@ -79,12 +89,13 @@ public AnalysisMethod lookup(AnalysisMetaAccess aMetaAccess, AnalysisMethod aCon
             for (int i = 0; i < unwrappedParameterTypes.length; i++) {
                 unwrappedParameterTypes[i] = aConstructor.getSignature().getParameterType(i).getWrapped();
             }
-            ResolvedJavaType unwrappedReturnType = (throwAllocatedObject ? aMetaAccess.lookupJavaType(void.class) : aConstructor.getDeclaringClass()).getWrapped();
+            ResolvedJavaType unwrappedReturnType = (throwAllocatedObject ? aMetaAccess.lookupJavaType(void.class) : aInstType).getWrapped();
             Signature unwrappedSignature = ResolvedSignature.fromArray(unwrappedParameterTypes, unwrappedReturnType);
             ResolvedJavaMethod unwrappedConstructor = aConstructor.getWrapped();
+            ResolvedJavaType unwrappedInstatiatedType = aInstType.getWrapped();
             ResolvedJavaType unwrappedDeclaringClass = (aMetaAccess.lookupJavaType(throwAllocatedObject ? FactoryThrowMethodHolder.class : FactoryMethodHolder.class)).getWrapped();
             ConstantPool unwrappedConstantPool = unwrappedConstructor.getConstantPool();
-            return new FactoryMethod(name, unwrappedConstructor, unwrappedDeclaringClass, unwrappedSignature, unwrappedConstantPool, throwAllocatedObject);
+            return new FactoryMethod(name, unwrappedConstructor, unwrappedInstatiatedType, unwrappedDeclaringClass, unwrappedSignature, unwrappedConstantPool, throwAllocatedObject);
         });
 
         AnalysisMethod aMethod = aMetaAccess.getUniverse().lookup(factoryMethod);

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

@@ -127,7 +127,10 @@ public class ReflectionFeature implements InternalFeature, ReflectionSubstitutio
     private int loadedConfigurations;
     private UniverseMetaAccess metaAccess;
 
-    final Map<Executable, SubstrateAccessor> accessors = new ConcurrentHashMap<>();
+    private record AccessorKey(Executable member, Class<?> targetClass) {
+    }
+
+    final Map<AccessorKey, SubstrateAccessor> accessors = new ConcurrentHashMap<>();
     private final Map<SignatureKey, MethodPointer> expandSignatureMethods = new ConcurrentHashMap<>();
 
     private static final Method invokePrototype = ReflectionUtil.lookupMethod(ReflectionAccessorHolder.class, "invokePrototype",
@@ -143,15 +146,21 @@ public class ReflectionFeature implements InternalFeature, ReflectionSubstitutio
 
     @Override
     public SubstrateAccessor getOrCreateAccessor(Executable member) {
-        SubstrateAccessor existing = accessors.get(member);
+        return getOrCreateConstructorAccessor(member.getDeclaringClass(), member);
+    }
+
+    @Override
+    public SubstrateAccessor getOrCreateConstructorAccessor(Class<?> targetClass, Executable member) {
+        AccessorKey key = new AccessorKey(member, targetClass);
+        SubstrateAccessor existing = accessors.get(key);
         if (existing != null) {
             return existing;
         }
 
         if (analysisAccess == null) {
             throw VMError.shouldNotReachHere("New Method or Constructor found as reachable after static analysis: " + member);
         }
-        return accessors.computeIfAbsent(member, this::createAccessor);
+        return accessors.computeIfAbsent(key, this::createAccessor);
     }
 
     /**
@@ -171,7 +180,9 @@ public SubstrateAccessor getOrCreateAccessor(Executable member) {
      * {@link ConcurrentHashMap#computeIfAbsent} guarantees that this method is called only once per
      * member, so no further synchronization is necessary.
      */
-    private SubstrateAccessor createAccessor(Executable member) {
+    private SubstrateAccessor createAccessor(AccessorKey key) {
+        Executable member = key.member;
+        Class<?> targetClass = key.targetClass;
         MethodPointer expandSignature;
         MethodPointer directTarget = null;
         AnalysisMethod targetMethod = null;
@@ -219,7 +230,7 @@ private SubstrateAccessor createAccessor(Executable member) {
             return new SubstrateMethodAccessor(member, receiverType, expandSignature, directTarget, targetMethod, vtableOffset, initializeBeforeInvoke, callerSensitiveAdapter);
 
         } else {
-            Class<?> holder = member.getDeclaringClass();
+            Class<?> holder = targetClass;
             CFunctionPointer factoryMethodTarget = null;
             ResolvedJavaMethod factoryMethod = null;
             if (Modifier.isAbstract(holder.getModifiers()) || holder.isInterface() || holder.isPrimitive() || holder.isArray()) {
@@ -233,8 +244,9 @@ private SubstrateAccessor createAccessor(Executable member) {
             } else {
                 expandSignature = createExpandSignatureMethod(member, false);
                 targetMethod = analysisAccess.getMetaAccess().lookupJavaMethod(member);
+                var aTargetClass = analysisAccess.getMetaAccess().lookupJavaType(targetClass);
                 directTarget = asMethodPointer(targetMethod);
-                factoryMethod = FactoryMethodSupport.singleton().lookup(analysisAccess.getMetaAccess(), targetMethod, false);
+                factoryMethod = FactoryMethodSupport.singleton().lookup(analysisAccess.getMetaAccess(), targetMethod, aTargetClass, false);
                 factoryMethodTarget = asMethodPointer(factoryMethod);
                 if (!targetMethod.getDeclaringClass().isInitialized()) {
                     initializeBeforeInvoke = analysisAccess.getHostVM().dynamicHub(targetMethod.getDeclaringClass());