[GR-71157] [GR-71158] Fix some exceptions during type resolution

gilles-duboscq · gilles-duboscq · commit a13258378638 · 2025-11-04T17:34:34.000Z
PullRequest: graal/22524
diff --git a/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ClassLoading.java b/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ClassLoading.java
@@ -60,22 +60,48 @@ public static boolean isSupported() {
     }
 
     /**
-     * Opens a scope in which class loading is not constrained by the reflection configuration. Once
-     * the returned {@link AutoCloseable} is closed, the previous state is restored.
+     * Opens a scope in which class loading is not constrained by the reflection configuration for
+     * the current thread. Once the returned {@link AutoCloseable} is closed, the previous state is
+     * restored.
      *
      * @throws IllegalStateException if class loading is not {@linkplain #isSupported() supported}.
      */
     public static ArbitraryClassLoadingScope allowArbitraryClassLoading() {
         if (!isSupported()) {
             throw new IllegalStateException("Runtime class loading is not supported. It must be enabled at build-time with -H:+RuntimeClassLoading.");
         }
-        return new ArbitraryClassLoadingScope();
+        return new RealArbitraryClassLoadingScope();
     }
 
-    public static final class ArbitraryClassLoadingScope implements AutoCloseable {
+    /**
+     * Conditionally opens a scope in which class loading is not constrained by the reflection
+     * configuration for the current thread. If {@code allowArbitraryClassLoading} is true, behaves
+     * like {@link #allowArbitraryClassLoading()}. Otherwise, it has no effect.
+     *
+     * @throws IllegalStateException if class loading is not {@linkplain #isSupported() supported}
+     *             and {@code allowArbitraryClassLoading} is true.
+     *
+     * @see #allowArbitraryClassLoading()
+     */
+    public static ArbitraryClassLoadingScope allowArbitraryClassLoading(boolean allowArbitraryClassLoading) {
+        if (allowArbitraryClassLoading) {
+            return allowArbitraryClassLoading();
+        }
+        return NoopScope.INSTANCE;
+    }
+
+    public abstract static class ArbitraryClassLoadingScope implements AutoCloseable {
+        private ArbitraryClassLoadingScope() {
+        }
+
+        @Override
+        public abstract void close();
+    }
+
+    public static final class RealArbitraryClassLoadingScope extends ArbitraryClassLoadingScope {
         private boolean closed;
 
-        ArbitraryClassLoadingScope() {
+        RealArbitraryClassLoadingScope() {
             if (!ImageInfo.inImageRuntimeCode()) {
                 return;
             }
@@ -94,4 +120,12 @@ public void close() {
             ClassLoadingSupport.singleton().endIgnoreReflectionConfigurationScope();
         }
     }
+
+    private static final class NoopScope extends ArbitraryClassLoadingScope {
+        private static final NoopScope INSTANCE = new NoopScope();
+
+        @Override
+        public void close() {
+        }
+    }
 }
diff --git a/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ClassLoadingSupport.java b/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ClassLoadingSupport.java
@@ -47,11 +47,28 @@ static ClassLoadingSupport singleton() {
         return ImageSingletons.lookup(ClassLoadingSupport.class);
     }
 
+    /**
+     * Returns whether runtime class loading is supported.
+     */
     boolean isSupported();
 
+    /**
+     * Returns false iff the current thread is in a
+     * {@linkplain #startIgnoreReflectionConfigurationScope() scope} where reflection configuration
+     * is ignored.
+     */
     boolean followReflectionConfiguration();
 
+    /**
+     * Starts a scope in which the reflection configuration is ignored for runtime class loading.
+     * <p>
+     * That scope can be closed by calling {@link #endIgnoreReflectionConfigurationScope()}.
+     */
     void startIgnoreReflectionConfigurationScope();
 
+    /**
+     * Ends a scope started by {@link #startIgnoreReflectionConfigurationScope()} for the current
+     * thread.
+     */
     void endIgnoreReflectionConfigurationScope();
 }
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/OWNERS.toml b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/OWNERS.toml
@@ -6,3 +6,8 @@ all = [
 ]
 any = [
 ]
+[[rule]]
+files = "RuntimeClassLoading.java RuntimeDynamicHubMetadata.java RuntimeReflectionMetadata.java"
+all = [
+    "gilles.m.duboscq@oracle.com",
+]
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/crema/CremaSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/crema/CremaSupport.java
@@ -39,10 +39,10 @@
 import com.oracle.svm.espresso.classfile.descriptors.Symbol;
 import com.oracle.svm.espresso.classfile.descriptors.Type;
 
-import jdk.vm.ci.meta.JavaType;
 import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
+import jdk.vm.ci.meta.UnresolvedJavaType;
 
 public interface CremaSupport {
     @Platforms(Platform.HOSTED_ONLY.class)
@@ -88,23 +88,23 @@ interface CremaDispatchTable {
 
     Class<?> toClass(ResolvedJavaType resolvedJavaType);
 
-    default Class<?> resolveOrThrow(JavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
+    default Class<?> resolveOrThrow(UnresolvedJavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
         ByteSequence type = ByteSequence.create(unresolvedJavaType.getName());
         Symbol<Type> symbolicType = SymbolsSupport.getTypes().getOrCreateValidType(type);
         return resolveOrThrow(symbolicType, accessingClass);
     }
 
     Class<?> resolveOrThrow(Symbol<Type> type, ResolvedJavaType accessingClass);
 
-    default Class<?> resolveOrNull(JavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
+    default Class<?> resolveOrNull(UnresolvedJavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
         ByteSequence type = ByteSequence.create(unresolvedJavaType.getName());
         Symbol<Type> symbolicType = SymbolsSupport.getTypes().getOrCreateValidType(type);
         return resolveOrNull(symbolicType, accessingClass);
     }
 
     Class<?> resolveOrNull(Symbol<Type> type, ResolvedJavaType accessingClass);
 
-    default Class<?> findLoadedClass(JavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
+    default Class<?> findLoadedClass(UnresolvedJavaType unresolvedJavaType, ResolvedJavaType accessingClass) {
         ByteSequence type = ByteSequence.create(unresolvedJavaType.getName());
         Symbol<Type> symbolicType = SymbolsSupport.getTypes().getOrCreateValidType(type);
         return findLoadedClass(symbolicType, accessingClass);
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/crema/OWNERS.toml b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/crema/OWNERS.toml
@@ -0,0 +1,5 @@
+[[rule]]
+files = "*"
+all = [
+    "gilles.m.duboscq@oracle.com",
+]
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/registry/OWNERS.toml b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/registry/OWNERS.toml
@@ -0,0 +1,5 @@
+[[rule]]
+files = "*"
+all = [
+    "gilles.m.duboscq@oracle.com",
+]
diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml b/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml
@@ -1,12 +1,6 @@
 [[rule]]
 files = "*"
 all = [
-    "alfonso.peterssen@oracle.com",
     "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
-]
-any = [
-    "alfonso.peterssen@oracle.com",
-    "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
+    "gilles.m.duboscq@oracle.com",
 ]
diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/CremaResolvedJavaFieldImpl.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/CremaResolvedJavaFieldImpl.java
@@ -24,6 +24,8 @@
  */
 package com.oracle.svm.interpreter.metadata;
 
+import org.graalvm.nativeimage.impl.ClassLoading;
+
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.hub.crema.CremaResolvedJavaField;
 import com.oracle.svm.core.hub.crema.CremaSupport;
@@ -78,8 +80,10 @@ public JavaType getType() {
     @Override
     public InterpreterResolvedJavaType getResolvedType() {
         if (resolvedType == null) {
-            Class<?> cls = CremaSupport.singleton().resolveOrThrow(getSymbolicType(), getDeclaringClass());
-            resolvedType = (InterpreterResolvedJavaType) DynamicHub.fromClass(cls).getInterpreterType();
+            try (var _ = ClassLoading.allowArbitraryClassLoading()) {
+                Class<?> cls = CremaSupport.singleton().resolveOrThrow(getSymbolicType(), getDeclaringClass());
+                resolvedType = (InterpreterResolvedJavaType) DynamicHub.fromClass(cls).getInterpreterType();
+            }
         }
         return resolvedType;
     }
diff --git a/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml b/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml
@@ -1,12 +1,6 @@
 [[rule]]
 files = "*"
 all = [
-    "alfonso.peterssen@oracle.com",
     "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
-]
-any = [
-    "alfonso.peterssen@oracle.com",
-    "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
+    "gilles.m.duboscq@oracle.com",
 ]
diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/CremaRuntimeAccess.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/CremaRuntimeAccess.java
@@ -27,6 +27,8 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
 
+import org.graalvm.nativeimage.impl.ClassLoading;
+
 import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.hub.crema.CremaSupport;
 import com.oracle.svm.core.hub.registry.SymbolsSupport;
@@ -94,9 +96,11 @@ public RuntimeException throwError(ErrorType errorType, String messageFormat, Ob
 
     @Override
     public InterpreterResolvedObjectType lookupOrLoadType(Symbol<Type> type, InterpreterResolvedJavaType accessingClass) {
-        Class<?> result = CremaSupport.singleton().resolveOrThrow(type, accessingClass);
-        assert !result.isPrimitive();
-        return (InterpreterResolvedObjectType) DynamicHub.fromClass(result).getInterpreterType();
+        try (var _ = ClassLoading.allowArbitraryClassLoading()) {
+            Class<?> result = CremaSupport.singleton().resolveOrThrow(type, accessingClass);
+            assert !result.isPrimitive();
+            return (InterpreterResolvedObjectType) DynamicHub.fromClass(result).getInterpreterType();
+        }
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java
@@ -1497,9 +1497,8 @@ private static InterpreterResolvedJavaType resolveType(InterpreterResolvedJavaMe
                 missingType = unresolvedJavaType;
             }
             throw noClassDefFoundError(opcode, missingType);
-        } catch (ClassFormatError e) {
-            // Out-of-bounds CPI or mis-matching tag.
-            throw SemanticJavaException.raise(e);
+        } catch (Throwable t) {
+            throw SemanticJavaException.raise(t);
         }
     }
 
@@ -1512,10 +1511,8 @@ private static InterpreterResolvedJavaType resolveTypeOrNullIfUnresolvable(Inter
             return getConstantPool(method).resolvedTypeAt(method.getDeclaringClass(), cpi);
         } catch (UnsupportedResolutionException e) {
             return null;
-        } catch (ClassFormatError e) {
-            // Out-of-bounds CPI or mis-matching tag.
-            // Unrelated to resolution, just propagate the error.
-            throw SemanticJavaException.raise(e);
+        } catch (Throwable t) {
+            throw SemanticJavaException.raise(t);
         }
     }
 
@@ -1533,9 +1530,8 @@ private static InterpreterResolvedJavaMethod resolveMethod(InterpreterResolvedJa
                 missingMethod = unresolvedJavaMethod;
             }
             throw noSuchMethodError(opcode, missingMethod);
-        } catch (ClassFormatError e) {
-            // Out-of-bounds CPI or mis-matching tag.
-            throw SemanticJavaException.raise(e);
+        } catch (Throwable t) {
+            throw SemanticJavaException.raise(t);
         }
     }
 
@@ -1553,9 +1549,8 @@ private static InterpreterResolvedJavaField resolveField(InterpreterResolvedJava
                 missingField = unresolvedJavaField;
             }
             throw noSuchFieldError(opcode, missingField);
-        } catch (ClassFormatError e) {
-            // Out of bounds CPI or mis-matching tag.
-            throw SemanticJavaException.raise(e);
+        } catch (Throwable t) {
+            throw SemanticJavaException.raise(t);
         }
     }
 
diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/RuntimeInterpreterConstantPool.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/RuntimeInterpreterConstantPool.java
@@ -26,6 +26,9 @@
 
 import java.lang.invoke.MethodType;
 
+import org.graalvm.nativeimage.impl.ClassLoading;
+
+import com.oracle.svm.core.hub.DynamicHub;
 import com.oracle.svm.core.hub.crema.CremaSupport;
 import com.oracle.svm.core.hub.registry.SymbolsSupport;
 import com.oracle.svm.core.methodhandles.Target_java_lang_invoke_MethodHandleNatives;
@@ -137,32 +140,41 @@ private String resolveStringConstant(int stringIndex, @SuppressWarnings("unused"
         return string;
     }
 
-    private InterpreterResolvedJavaType resolveClassConstant(int classIndex, InterpreterResolvedJavaType accessingKlass) {
+    private static InterpreterResolvedObjectType resolveObjectType(Symbol<Type> type, InterpreterResolvedObjectType accessingClass) {
+        DynamicHub hub = DynamicHub.fromClass(CremaSupport.singleton().resolveOrThrow(type, accessingClass));
+        assert !hub.isPrimitive();
+        return (InterpreterResolvedObjectType) hub.getInterpreterType();
+    }
+
+    private InterpreterResolvedJavaType resolveClassConstant(int classIndex, InterpreterResolvedObjectType accessingKlass) {
         assert accessingKlass != null;
         assert tagAt(classIndex) == Tag.CLASS;
 
         Object entry = this.cachedEntries[classIndex];
         Symbol<Type> type = null;
 
+        boolean allowArbitraryClassLoading;
         if (entry == null) {
             // CP comes from dynamically loaded .class file.
             Symbol<Name> className = this.className(classIndex);
             type = SymbolsSupport.getTypes().fromClassNameEntry(className);
+            allowArbitraryClassLoading = true;
         } else if (entry instanceof UnresolvedJavaType unresolvedJavaType) {
             Throwable cause = unresolvedJavaType.getCause();
             if (cause != null) {
                 throw uncheckedThrow(cause);
             }
             // CP comes from build-time JVMCI type, derive type from UnresolvedJavaType.
             type = SymbolsSupport.getTypes().getOrCreateValidType(unresolvedJavaType.getName());
+            allowArbitraryClassLoading = false;
         } else {
             throw VMError.shouldNotReachHere("Invalid cached CP entry, expected unresolved type, but got " + entry);
         }
 
         assert type != null;
 
-        try {
-            InterpreterResolvedObjectType result = CremaRuntimeAccess.getInstance().lookupOrLoadType(type, accessingKlass);
+        try (var _ = ClassLoading.allowArbitraryClassLoading(allowArbitraryClassLoading)) {
+            InterpreterResolvedObjectType result = resolveObjectType(type, accessingKlass);
             return result;
         } catch (LinkageError e) {
             // Comment from Hotspot:
@@ -201,7 +213,7 @@ private InterpreterResolvedJavaField resolveFieldRefConstant(int fieldIndex, Int
             Symbol<Type> holderType = SymbolsSupport.getTypes().getOrCreateValidType(unresolvedJavaField.getDeclaringClass().getName());
             assert !TypeSymbols.isPrimitive(holderType) && !TypeSymbols.isArray(holderType);
             // Perf. note: The holder is re-resolved every-time (never cached).
-            holder = CremaRuntimeAccess.getInstance().lookupOrLoadType(holderType, accessingClass);
+            holder = resolveObjectType(holderType, accessingClass);
         } else {
             throw VMError.shouldNotReachHere("Invalid cached CP entry, expected unresolved field, but got " + entry);
         }
@@ -237,7 +249,7 @@ private InterpreterResolvedJavaMethod resolveClassMethodRefConstant(int methodIn
             methodSignature = SymbolsSupport.getSignatures().getOrCreateValidSignature(unresolvedJavaMethod.getSignature().toMethodDescriptor());
             Symbol<Type> holderType = SymbolsSupport.getTypes().getOrCreateValidType(unresolvedJavaMethod.getDeclaringClass().getName());
             // Perf. note: The holder is re-resolved every-time (never cached).
-            holder = CremaRuntimeAccess.getInstance().lookupOrLoadType(holderType, accessingClass);
+            holder = resolveObjectType(holderType, accessingClass);
         } else {
             throw VMError.shouldNotReachHere("Invalid cached CP entry, expected unresolved method, but got " + entry);
         }
@@ -275,7 +287,7 @@ private InterpreterResolvedJavaMethod resolveInterfaceMethodRefConstant(int inte
             methodSignature = SymbolsSupport.getSignatures().getOrCreateValidSignature(unresolvedJavaMethod.getSignature().toMethodDescriptor());
             Symbol<Type> holderType = SymbolsSupport.getTypes().getOrCreateValidType(unresolvedJavaMethod.getDeclaringClass().getName());
             // Perf. note: The holder is re-resolved every-time (never cached).
-            holder = CremaRuntimeAccess.getInstance().lookupOrLoadType(holderType, accessingClass);
+            holder = resolveObjectType(holderType, accessingClass);
         } else {
             throw VMError.shouldNotReachHere("Invalid cached CP entry, expected unresolved method, but got " + entry);
         }
@@ -322,8 +334,10 @@ public static MethodType signatureToMethodType(Symbol<Type>[] signature, Interpr
     }
 
     private static Class<?> resolveSymbolAndAccessCheck(InterpreterResolvedObjectType accessingClass, Symbol<Type> type) {
-        Class<?> clazz = CremaSupport.singleton().resolveOrThrow(type, accessingClass);
-        // GR-62339 check access
-        return clazz;
+        try (var _ = ClassLoading.allowArbitraryClassLoading()) {
+            Class<?> clazz = CremaSupport.singleton().resolveOrThrow(type, accessingClass);
+            // GR-62339 check access
+            return clazz;
+        }
     }
 }
diff --git a/substratevm/src/com.oracle.svm.jdwp.bridge/OWNERS.toml b/substratevm/src/com.oracle.svm.jdwp.bridge/OWNERS.toml
@@ -1,12 +1,5 @@
 [[rule]]
 files = "*"
 all = [
-    "alfonso.peterssen@oracle.com",
     "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
-]
-any = [
-    "alfonso.peterssen@oracle.com",
-    "bernhard.urban-forster@oracle.com",
-    "martin.entlicher@oracle.com",
 ]
diff --git a/substratevm/src/com.oracle.svm.jdwp.resident/OWNERS.toml b/substratevm/src/com.oracle.svm.jdwp.resident/OWNERS.toml
diff --git a/substratevm/src/com.oracle.svm.jdwp.server/OWNERS.toml b/substratevm/src/com.oracle.svm.jdwp.server/OWNERS.toml

Original file line number	Diff line number	Diff line change
`@@ -6,3 +6,8 @@ all = [`
`6`	`6`	`]`
`7`	`7`	`any = [`
`8`	`8`	`]`
	`9`	`+[[rule]]`
	`10`	`+files = "RuntimeClassLoading.java RuntimeDynamicHubMetadata.java RuntimeReflectionMetadata.java"`
	`11`	`+all = [`
	`12`	`+ "gilles.m.duboscq@oracle.com",`
	`13`	`+]`