From ca339b3150f6d8bd0d75b9e2d66fc6dfcdaa77b7 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Wed, 14 May 2025 10:35:26 +0200 Subject: [PATCH 1/5] Introduce IndirectCallTargets for Open Type World Hub Layout. --- .../graal/pointsto/meta/AnalysisMethod.java | 66 --------- .../graal/pointsto/meta/AnalysisType.java | 44 ------ .../OpenTypeWorldDispatchTableSnippets.java | 7 + .../oracle/svm/core/meta/SharedMethod.java | 18 +++ .../GraalGraphObjectReplacer.java | 16 ++- .../oracle/svm/graal/meta/SubstrateField.java | 2 +- .../svm/graal/meta/SubstrateMethod.java | 17 ++- .../SharedLayerSnapshotCapnProtoSchema.capnp | 37 ++--- .../svm/hosted/OpenTypeWorldFeature.java | 125 +++++++++++++++-- .../src/com/oracle/svm/hosted/SVMHost.java | 5 - .../svm/hosted/code/NonBytecodeMethod.java | 2 +- .../imagelayer/SVMImageLayerLoader.java | 126 +++++++++++++----- .../imagelayer/SVMImageLayerWriter.java | 1 + ...redLayerSnapshotCapnProtoSchemaHolder.java | 35 +++-- .../svm/hosted/jni/JNIAccessFeature.java | 3 +- .../oracle/svm/hosted/meta/HostedMethod.java | 61 ++++++++- .../svm/hosted/meta/UniverseBuilder.java | 4 + .../oracle/svm/hosted/meta/VTableBuilder.java | 13 +- .../OpenTypeWorldConvertCallTargetPhase.java | 62 --------- .../svm/hosted/reflect/ReflectionFeature.java | 8 +- 20 files changed, 382 insertions(+), 270 deletions(-) delete mode 100644 substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/OpenTypeWorldConvertCallTargetPhase.java diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 47c9dd63ecdc..7bb6b4f2deb6 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -126,9 +126,6 @@ public record Signature(String name, AnalysisType[] parameterTypes) { public final ResolvedJavaMethod wrapped; - private AnalysisMethod indirectCallTarget = null; - public boolean invalidIndirectCallTarget = false; - private final int id; /** Marks a method loaded from a base layer. */ private final boolean isInBaseLayer; @@ -347,69 +344,6 @@ public AnalysisUniverse getUniverse() { return declaringClass.getUniverse(); } - private static boolean matchingSignature(AnalysisMethod o1, AnalysisMethod o2) { - if (o1.equals(o2)) { - return true; - } - - if (!o1.getName().equals(o2.getName())) { - return false; - } - - return o1.getSignature().equals(o2.getSignature()); - } - - private AnalysisMethod setIndirectCallTarget(AnalysisMethod method, boolean foundMatch) { - indirectCallTarget = method; - invalidIndirectCallTarget = !foundMatch; - return indirectCallTarget; - } - - /** - * For methods where its {@link #getDeclaringClass()} does not explicitly declare the method, - * find an alternative explicit declaration for the method which can be used as an indirect call - * target. This logic is currently used for deciding the target of virtual/interface calls when - * using the open type world. - */ - public AnalysisMethod getIndirectCallTarget() { - if (indirectCallTarget != null) { - return indirectCallTarget; - } - if (isStatic() || isConstructor()) { - /* - * Static methods and constructors must always be explicitly declared. - */ - return setIndirectCallTarget(this, true); - } - - var dispatchTableMethods = declaringClass.getOrCalculateOpenTypeWorldDispatchTableMethods(); - - if (dispatchTableMethods.contains(this)) { - return setIndirectCallTarget(this, true); - } - - for (AnalysisType interfaceType : declaringClass.getAllInterfaces()) { - if (interfaceType.equals(declaringClass)) { - // already checked - continue; - } - dispatchTableMethods = interfaceType.getOrCalculateOpenTypeWorldDispatchTableMethods(); - for (AnalysisMethod candidate : dispatchTableMethods) { - if (matchingSignature(candidate, this)) { - return setIndirectCallTarget(candidate, true); - } - } - } - - /* - * For some methods (e.g., methods labeled as @PolymorphicSignature or @Delete), we - * currently do not find matches. However, these methods will not be indirect calls within - * our generated code, so it is not necessary to determine an accurate virtual/interface - * call target. - */ - return setIndirectCallTarget(this, false); - } - /** * @see PointsToAnalysis#validateFixedPointState */ diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 3993bdabdc04..e2291dab8545 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -175,8 +175,6 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav private AnalysisMethod[] declaredMethods; private Set dispatchTableMethods; - private AnalysisType[] allInterfaces; - /* isArray is an expensive operation so we eagerly compute it */ private final boolean isArray; private final boolean isJavaLangObject; @@ -377,36 +375,6 @@ public int getArrayDimension() { return dimension; } - /** - * @return All interfaces this type inherits (including itself if it is an interface). - */ - public AnalysisType[] getAllInterfaces() { - if (allInterfaces != null) { - return allInterfaces; - } - - Set allInterfaceSet = new HashSet<>(); - - if (isInterface()) { - allInterfaceSet.add(this); - } - - if (this.superClass != null) { - allInterfaceSet.addAll(Arrays.asList(this.superClass.getAllInterfaces())); - } - - for (AnalysisType i : interfaces) { - allInterfaceSet.addAll(Arrays.asList(i.getAllInterfaces())); - } - - var result = allInterfaceSet.toArray(AnalysisType[]::new); - - // ensure result is fully visible across threads - VarHandle.storeStoreFence(); - allInterfaces = result; - return allInterfaces; - } - public void cleanupAfterAnalysis() { instantiatedTypes = null; instantiatedTypesNonNull = null; @@ -1356,18 +1324,6 @@ public AnalysisMethod[] getDeclaredConstructors(boolean forceLink) { return universe.lookup(wrapped.getDeclaredConstructors(forceLink)); } - public AnalysisMethod findConstructor(Signature signature) { - if (wrapped instanceof BaseLayerType) { - return null; - } - for (AnalysisMethod ctor : getDeclaredConstructors(false)) { - if (ctor.getSignature().equals(signature)) { - return ctor; - } - } - return null; - } - public boolean isOpenTypeWorldDispatchTableMethodsCalculated() { return dispatchTableMethods != null; } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/OpenTypeWorldDispatchTableSnippets.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/OpenTypeWorldDispatchTableSnippets.java index 1b8add5de3fc..4573939dc2db 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/OpenTypeWorldDispatchTableSnippets.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/OpenTypeWorldDispatchTableSnippets.java @@ -122,6 +122,13 @@ public void lower(LoadOpenTypeWorldDispatchTableStartingOffset node, LoweringToo SharedMethod target = node.getTarget(); int vtableStartingOffset = KnownOffsets.singleton().getVTableBaseOffset(); if (target != null) { + /* + * Update target to point to indirect call target. The indirect call target is + * different than the original target when the original target is a method we have + * not placed in any virtual/interface table. See SharedMethod#getIndirectCallTarget + * and HostedMethod#indirectCallVTableIndex for more information. + */ + target = target.getIndirectCallTarget(); /* * If the target is known, then we know whether to use the class dispatch table or * an interface dispatch table. diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedMethod.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedMethod.java index d3975e747a78..28af26a1f4a9 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedMethod.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/meta/SharedMethod.java @@ -65,6 +65,24 @@ public interface SharedMethod extends ResolvedJavaMethod { int getVTableIndex(); + /** + * In the open type world, our virtual/interface tables will only contain declared methods. + * However, sometimes JVMCI will expose special methods HotSpot introduces into vtables, such as + * miranda and overpass methods. When these special methods serve as call targets for indirect + * calls, we must switch the call target to an alternative method (with the same resolution) + * that will be present in the open type world virtual/interface tables. + * + *

+ * Note normally in the open type world {@code indirectCallTarget == this}. Only for special + * HotSpot-specific methods such as miranda and overpass methods will the indirectCallTarget be + * a different method. The logic for setting the indirectCallTarget can be found in + * {@code OpenTypeWorldFeature#calculateIndirectCallTarget}. + * + *

+ * In the closed type world, this method will always return {@code this}. + */ + SharedMethod getIndirectCallTarget(); + /** * Returns the deopt stub type for the stub methods in {@link Deoptimizer}. Only used when * compiling the deopt stubs during image generation. diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java index af1702a2dbb3..45dcb98224f4 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java @@ -476,8 +476,19 @@ public void updateSubstrateDataAfterCompilation(HostedUniverse hUniverse, Provid ? providers.getConstantReflection().readFieldValue(hField, null) : null; constantValue = SubstrateGraalUtils.hostedToRuntime(constantValue, providers.getConstantReflection()); - sField.setSubstrateData(hField.getLocation(), hField.isAccessed(), hField.isWritten() || !hField.isValueAvailable(), constantValue); + sField.setSubstrateDataAfterCompilation(hField.getLocation(), hField.isAccessed(), hField.isWritten() || !hField.isValueAvailable(), constantValue); } + + methods.forEach((aMethod, sMethod) -> { + HostedMethod hMethod = hUniverse.lookup(aMethod); + SubstrateMethod indirectCallTarget = sMethod; + if (!hMethod.getIndirectCallTarget().equals(hMethod)) { + indirectCallTarget = methods.get(hMethod.getIndirectCallTarget().getWrapped()); + } + int vTableIndex = (hMethod.hasVTableIndex() ? hMethod.getVTableIndex() : HostedMethod.MISSING_VTABLE_IDX); + sMethod.setSubstrateDataAfterCompilation(indirectCallTarget, vTableIndex); + }); + } public void updateSubstrateDataAfterHeapLayout(HostedUniverse hUniverse) { @@ -485,14 +496,13 @@ public void updateSubstrateDataAfterHeapLayout(HostedUniverse hUniverse) { AnalysisMethod aMethod = entry.getKey(); SubstrateMethod sMethod = entry.getValue(); HostedMethod hMethod = hUniverse.lookup(aMethod); - int vTableIndex = (hMethod.hasVTableIndex() ? hMethod.getVTableIndex() : -1); /* * We access the offset of methods in the image code section here. Therefore, this code * can only run after the heap and code cache layout was done. */ int imageCodeOffset = hMethod.isCodeAddressOffsetValid() ? hMethod.getCodeAddressOffset() : 0; - sMethod.setSubstrateData(vTableIndex, imageCodeOffset, hMethod.getImageCodeDeoptOffset()); + sMethod.setSubstrateDataAfterHeapLayout(imageCodeOffset, hMethod.getImageCodeDeoptOffset()); } } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateField.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateField.java index 588aa959bca7..8df6916bc09f 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateField.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateField.java @@ -89,7 +89,7 @@ public void setLinks(SubstrateType type, SubstrateType declaringClass) { this.declaringClass = declaringClass; } - public void setSubstrateData(int location, boolean isAccessed, boolean isWritten, JavaConstant constantValue) { + public void setSubstrateDataAfterCompilation(int location, boolean isAccessed, boolean isWritten, JavaConstant constantValue) { this.location = location; this.isAccessed = isAccessed; this.isWritten = isWritten; diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java index c2ca2d57ea3b..f4556878bdba 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java @@ -37,6 +37,7 @@ import org.graalvm.nativeimage.c.function.CEntryPoint; import com.oracle.graal.pointsto.meta.AnalysisMethod; +import com.oracle.svm.core.BuildPhaseProvider.AfterCompilation; import com.oracle.svm.core.BuildPhaseProvider.AfterHeapLayout; import com.oracle.svm.core.BuildPhaseProvider.ReadyForCompilation; import com.oracle.svm.core.Uninterruptible; @@ -50,6 +51,7 @@ import com.oracle.svm.core.graal.phases.SubstrateSafepointInsertionPhase; import com.oracle.svm.core.heap.UnknownObjectField; import com.oracle.svm.core.heap.UnknownPrimitiveField; +import com.oracle.svm.core.meta.SharedMethod; import com.oracle.svm.core.snippets.SubstrateForeignCallTarget; import com.oracle.svm.core.util.HostedStringDeduplication; import com.oracle.svm.core.util.VMError; @@ -88,7 +90,8 @@ public class SubstrateMethod implements SharedRuntimeMethod { private final int hashCode; private SubstrateType declaringClass; @UnknownPrimitiveField(availability = ReadyForCompilation.class) private int encodedGraphStartOffset; - @UnknownPrimitiveField(availability = AfterHeapLayout.class) private int vTableIndex; + @UnknownPrimitiveField(availability = AfterCompilation.class) private int vTableIndex; + @UnknownObjectField(availability = AfterCompilation.class) private SubstrateMethod indirectCallTarget; /** * A metadata object describing the image code that contains the compiled code of this method. @@ -195,8 +198,13 @@ public Object getRawImplementations() { return implementations; } - public void setSubstrateData(int vTableIndex, int imageCodeOffset, int imageCodeDeoptOffset) { + public void setSubstrateDataAfterCompilation(SubstrateMethod indirectCallTarget, int vTableIndex) { + this.indirectCallTarget = indirectCallTarget; this.vTableIndex = vTableIndex; + + } + + public void setSubstrateDataAfterHeapLayout(int imageCodeOffset, int imageCodeDeoptOffset) { this.imageCodeOffset = imageCodeOffset; this.imageCodeDeoptOffset = imageCodeDeoptOffset; } @@ -302,6 +310,11 @@ public int getVTableIndex() { return vTableIndex; } + @Override + public SharedMethod getIndirectCallTarget() { + return indirectCallTarget; + } + @Override public Deoptimizer.StubType getDeoptStubType() { return Deoptimizer.StubType.NoDeoptStub; diff --git a/substratevm/src/com.oracle.svm.hosted/resources/SharedLayerSnapshotCapnProtoSchema.capnp b/substratevm/src/com.oracle.svm.hosted/resources/SharedLayerSnapshotCapnProtoSchema.capnp index ad16c757f401..9b8dc779efc1 100644 --- a/substratevm/src/com.oracle.svm.hosted/resources/SharedLayerSnapshotCapnProtoSchema.capnp +++ b/substratevm/src/com.oracle.svm.hosted/resources/SharedLayerSnapshotCapnProtoSchema.capnp @@ -96,36 +96,37 @@ struct PersistedAnalysisMethod { annotationList @20 :List(Annotation); isVarArgs @21 :Bool; isBridge @22 :Bool; - analysisGraphLocation @23 :Text; - analysisGraphIsIntrinsic @24 :Bool; - strengthenedGraphLocation @25 :Text; - hostedMethodIndex @26 :HostedMethodIndex; + isDeclared @23 :Bool; + analysisGraphLocation @24 :Text; + analysisGraphIsIntrinsic @25 :Bool; + strengthenedGraphLocation @26 :Text; + hostedMethodIndex @27 :HostedMethodIndex; wrappedMethod :union { - none @27 :Void; # default + none @28 :Void; # default factoryMethod :group { - targetConstructorId @28 :MethodId; - throwAllocatedObject @29 :Bool; - instantiatedTypeId @30 :TypeId; + targetConstructorId @29 :MethodId; + throwAllocatedObject @30 :Bool; + instantiatedTypeId @31 :TypeId; } outlinedSB :group { - methodTypeReturn @31 :Text; - methodTypeParameters @32 :List(Text); + methodTypeReturn @32 :Text; + methodTypeParameters @33 :List(Text); } cEntryPointCallStub :group { - originalMethodId @33 :MethodId; - notPublished @34 :Bool; + originalMethodId @34 :MethodId; + notPublished @35 :Bool; } wrappedMember :group { union { - reflectionExpandSignature @35 :Void; - javaCallVariantWrapper @36 :Void; + reflectionExpandSignature @36 :Void; + javaCallVariantWrapper @37 :Void; } - name @37 :Text; - declaringClassName @38 :Text; - argumentTypeNames @39 :List(Text); + name @38 :Text; + declaringClassName @39 :Text; + argumentTypeNames @40 :List(Text); } polymorphicSignature :group { - callers @40 :List(MethodId); + callers @41 :List(MethodId); } } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java index 308652eef020..fafd10015783 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java @@ -27,7 +27,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.EnumSet; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.graalvm.nativeimage.ImageSingletons; @@ -45,9 +47,12 @@ import com.oracle.svm.core.layeredimagesingleton.ImageSingletonWriter; import com.oracle.svm.core.layeredimagesingleton.LayeredImageSingleton; import com.oracle.svm.core.layeredimagesingleton.LayeredImageSingletonBuilderFlags; +import com.oracle.svm.core.meta.SharedMethod; import com.oracle.svm.hosted.imagelayer.HostedImageLayerBuildingSupport; import com.oracle.svm.hosted.imagelayer.SVMImageLayerLoader; +import com.oracle.svm.hosted.meta.HostedMethod; import com.oracle.svm.hosted.meta.HostedType; +import com.oracle.svm.hosted.meta.HostedUniverse; import jdk.graal.compiler.debug.Assertions; @@ -67,7 +72,6 @@ public void beforeUniverseBuilding(BeforeUniverseBuildingAccess access) { } private final Set triggeredTypes = new HashSet<>(); - private final Set triggeredMethods = new HashSet<>(); @Override public void duringAnalysis(DuringAnalysisAccess access) { @@ -78,14 +82,6 @@ public void duringAnalysis(DuringAnalysisAccess access) { config.requireAnalysisIteration(); } } - for (AnalysisMethod aMethod : config.getUniverse().getMethods()) { - if (triggeredMethods.add(aMethod)) { - if (!aMethod.isStatic()) { - aMethod.getIndirectCallTarget(); - config.requireAnalysisIteration(); - } - } - } } @Override @@ -97,6 +93,117 @@ public void beforeCompilation(BeforeCompilationAccess access) { } } + /** + * see {@link SharedMethod#getIndirectCallTarget}. + */ + public static void computeIndirectCallTargets(HostedUniverse hUniverse, Map methods) { + Map allInterfacesMap = new HashMap<>(); + methods.forEach((aMethod, hMethod) -> { + if (!aMethod.isOriginalMethod()) { + // We don't need to set this; only original methods will be call targets + return; + } + + var aAlias = calculateIndirectCallTarget(allInterfacesMap, hMethod); + HostedMethod hAlias; + if (aAlias.equals(aMethod)) { + hAlias = hMethod; + } else { + hAlias = hUniverse.lookup(aAlias); + assert hAlias != null; + } + + hMethod.setIndirectCallTarget(hAlias); + }); + } + + /** + * For methods where its {@link AnalysisMethod#getDeclaringClass()} does not explicitly declare + * the method, find an alternative explicit declaration for the method which can be used as an + * indirect call target. This logic is currently used for deciding the target of + * virtual/interface calls when using the open type world. + */ + private static AnalysisMethod calculateIndirectCallTarget(Map allInterfacesMap, HostedMethod hOriginal) { + AnalysisMethod aOriginal = hOriginal.getWrapped(); + if (hOriginal.isStatic() || hOriginal.isConstructor()) { + /* + * Static methods and constructors must always be explicitly declared. + */ + return aOriginal; + } + + var declaringClass = hOriginal.getDeclaringClass(); + var dispatchTableMethods = declaringClass.getWrapped().getOpenTypeWorldDispatchTableMethods(); + + if (dispatchTableMethods.contains(aOriginal)) { + return aOriginal; + } + + for (var interfaceType : getAllInterfaces(allInterfacesMap, declaringClass)) { + if (interfaceType.equals(declaringClass)) { + // already checked + continue; + } + dispatchTableMethods = interfaceType.getWrapped().getOpenTypeWorldDispatchTableMethods(); + for (AnalysisMethod candidate : dispatchTableMethods) { + if (matchingSignature(candidate, aOriginal)) { + return candidate; + } + } + } + + /* + * For some methods (e.g., methods labeled as @PolymorphicSignature or @Delete), we + * currently do not find matches. However, these methods will not be indirect calls within + * our generated code, so it is not necessary to determine an accurate virtual/interface + * call target. + */ + return aOriginal; + } + + /** + * @return All interfaces this type inherits (including itself if it is an interface). + */ + private static HostedType[] getAllInterfaces(Map allInterfacesMap, HostedType type) { + var result = allInterfacesMap.get(type); + if (result != null) { + return result; + } + + Set allInterfaceSet = new HashSet<>(); + + if (type.isInterface()) { + allInterfaceSet.add(type); + } + + if (type.getSuperclass() != null) { + allInterfaceSet.addAll(Arrays.asList(getAllInterfaces(allInterfacesMap, type.getSuperclass()))); + } + + for (var i : type.getInterfaces()) { + allInterfaceSet.addAll(Arrays.asList(getAllInterfaces(allInterfacesMap, i))); + } + + result = allInterfaceSet.toArray(HostedType[]::new); + // sort so that we have a consistent order + Arrays.sort(result, HostedUniverse.TYPE_COMPARATOR); + + allInterfacesMap.put(type, result); + return result; + } + + private static boolean matchingSignature(AnalysisMethod o1, AnalysisMethod o2) { + if (o1.equals(o2)) { + return true; + } + + if (!o1.getName().equals(o2.getName())) { + return false; + } + + return o1.getSignature().equals(o2.getSignature()); + } + public static int loadTypeInfo(Collection types) { if (ImageLayerBuildingSupport.buildingExtensionLayer()) { /* diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java index 3a5f6dd7371d..afca15548259 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java @@ -133,7 +133,6 @@ import com.oracle.svm.hosted.phases.InlineBeforeAnalysisGraphDecoderImpl; import com.oracle.svm.hosted.phases.InlineBeforeAnalysisPolicyImpl; import com.oracle.svm.hosted.phases.InlineBeforeAnalysisPolicyUtils; -import com.oracle.svm.hosted.phases.OpenTypeWorldConvertCallTargetPhase; import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor; import com.oracle.svm.hosted.substitute.AutomaticUnsafeTransformationSupport; import com.oracle.svm.hosted.util.IdentityHashCodeUtil; @@ -737,10 +736,6 @@ public void methodAfterParsingHook(BigBang bb, AnalysisMethod method, Structured } protected void optimizeAfterParsing(BigBang bb, AnalysisMethod method, StructuredGraph graph) { - if (!SubstrateOptions.useClosedTypeWorldHubLayout()) { - new OpenTypeWorldConvertCallTargetPhase().apply(graph, getProviders(method.getMultiMethodKey())); - } - if (PointstoOptions.EscapeAnalysisBeforeAnalysis.getValue(bb.getOptions())) { if (method.isOriginalMethod()) { /* diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/NonBytecodeMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/NonBytecodeMethod.java index 9316b94ba121..a40cf1fa98cc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/NonBytecodeMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/NonBytecodeMethod.java @@ -151,7 +151,7 @@ public boolean isDefault() { @Override public boolean isDeclared() { - throw VMError.intentionallyUnimplemented(); // ExcludeFromJacocoGeneratedReport + return false; } @Override diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java index 01426cdb98f2..dff806bb72f6 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java @@ -43,6 +43,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; @@ -58,6 +59,7 @@ import com.oracle.graal.pointsto.BigBang; import com.oracle.graal.pointsto.api.ImageLayerLoader; +import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException; import com.oracle.graal.pointsto.flow.AnalysisParsedGraph; import com.oracle.graal.pointsto.heap.HostedValuesProvider; import com.oracle.graal.pointsto.heap.ImageHeapConstant; @@ -767,50 +769,114 @@ private void loadMethod(PersistedAnalysisMethod.Reader methodData) { AnalysisType returnType = getAnalysisTypeForBaseLayerId(methodData.getReturnTypeId()); + /* + * First try to retrieve host method via reflection. + * + * Because we are using reflection to access the hosted universe, for substitution methods + * we must use the Class values of the substitution class (i.e. class with + * the @TargetClass) and not the target of the substitution itself. + */ String name = methodData.getName().toString(); - if (methodData.hasClassName()) { - String className = methodData.getClassName().toString(); - - Executable method = null; - Class clazz = lookupBaseLayerTypeInHostVM(className); - if (clazz != null) { - Class[] argumentClasses = CapnProtoAdapters.toArray(methodData.getArgumentClassNames(), this::lookupBaseLayerTypeInHostVM, Class[]::new); - method = lookupMethodByReflection(name, clazz, argumentClasses); + boolean maybeReachableViaReflection = methodData.getIsConstructor() || methodData.getIsDeclared(); + if (maybeReachableViaReflection) { + Class clazz = null; + if (methodData.hasClassName()) { + String className = methodData.getClassName().toString(); + clazz = lookupBaseLayerTypeInHostVM(className); } - - if (method != null) { - metaAccess.lookupJavaMethod(method); - if (methods.containsKey(mid)) { - return; + if (clazz == null && !(type.getWrapped() instanceof BaseLayerType)) { + /* + * BaseLayerTypes will always return java.lang.Object, which is not correct for + * reflective lookup. + */ + clazz = type.getJavaClass(); + } + if (clazz != null) { + Class[] argumentClasses; + if (methodData.hasArgumentClassNames()) { + argumentClasses = CapnProtoAdapters.toArray(methodData.getArgumentClassNames(), this::lookupBaseLayerTypeInHostVM, Class[]::new); + } else { + argumentClasses = Arrays.stream(parameterTypes).map(AnalysisType::getJavaClass).toArray(Class[]::new); + } + if (Arrays.stream(argumentClasses).noneMatch(Objects::isNull)) { + var result = lookupMethodByReflection(name, clazz, argumentClasses); + if (result != null) { + metaAccess.lookupJavaMethod(result); + /* + * Note even if we found a method via reflection, it is not guaranteed it is + * the matching method. This is because, in a given class, reflection will + * not find all methods; one example it will not find is bridge methods + * inserted for covariant overrides. + */ + if (methods.containsKey(mid)) { + return; + } + } + } else { + LogUtils.warning("Arguments reflectively loading %s. %s could not be found: %s", methodData.getClassName().toString(), methodData.getName().toString(), + Arrays.toString(parameterTypes)); } } } - Class[] argumentClasses = Arrays.stream(parameterTypes).map(AnalysisType::getJavaClass).toArray(Class[]::new); - Executable method = lookupMethodByReflection(name, type.getJavaClass(), argumentClasses); - - if (method != null) { - metaAccess.lookupJavaMethod(method); - if (methods.containsKey(mid)) { - return; + /* + * Either the method cannot be looked up via reflection or looking up the method via + * reflection failed. Now try to find the matching method. + */ + if (!(type.getWrapped() instanceof BaseLayerType)) { + if (name.equals(CLASS_INIT_NAME)) { + type.getClassInitializer(); + } else { + ResolvedSignature signature = ResolvedSignature.fromArray(parameterTypes, returnType); + tryFindMethod(type, name, signature); } } - ResolvedSignature signature = ResolvedSignature.fromArray(parameterTypes, returnType); - - if (name.equals(CONSTRUCTOR_NAME)) { - type.findConstructor(signature); - } else if (name.equals(CLASS_INIT_NAME)) { - type.getClassInitializer(); - } else { - type.findMethod(name, signature); - } - if (!methods.containsKey(mid)) { createBaseLayerMethod(methodData, mid, name, parameterTypes, returnType); } } + /** + * Iterate through all methods to try to find and load one with a matching signature. + * + * We need this because sometimes JVMCI will expose to analysis special methods HotSpot + * introduces into vtables, such as miranda and overpass methods, which cannot be accessed via + * reflection. + * + * We also need this because reflection cannot find all declared methods within class; one + * example it will not find is bridge methods inserted for covariant overrides. + */ + private void tryFindMethod(AnalysisType type, String name, ResolvedSignature signature) { + ResolvedJavaType wrapped = type.getWrapped(); + assert !(wrapped instanceof BaseLayerType) : type; + for (ResolvedJavaMethod method : wrapped.getAllMethods(false)) { + /* + * Filter to limit the number of universe lookups needed. + */ + if (method.getName().equals(name)) { + try { + ResolvedSignature m = universe.lookup(method.getSignature(), method.getDeclaringClass()); + if (m.equals(signature)) { + universe.lookup(method); + return; + } + } catch (UnsupportedFeatureException t) { + /* + * Methods which are deleted or not available on this platform will throw an + * error during lookup - ignore and continue execution + * + * Note it is not simple to create a check to determine whether calling + * universe#lookup will trigger an error by creating an analysis object for a + * type not supported on this platform, as creating a method requires, in + * addition to the types of its return type and parameters, all of the super + * types of its return and parameters to be created as well. + */ + } + } + } + } + protected boolean delegateLoadMethod(PersistedAnalysisMethod.Reader methodData) { WrappedMethod.Reader wrappedMethod = methodData.getWrappedMethod(); if (wrappedMethod.isNone()) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java index f9087fd3a319..241f431e576c 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerWriter.java @@ -576,6 +576,7 @@ private void persistMethod(AnalysisMethod method, Supplier v.finalizeVTableIndex(true)); } else { builder.buildOpenTypeWorldDispatchTables(); + hUniverse.methods.forEach((k, v) -> v.finalizeVTableIndex(false)); assert builder.verifyOpenTypeWorldDispatchTables(); } } @@ -213,7 +215,7 @@ private List generateDispatchTable(HostedType type, int startingIn int index = startingIndex; for (HostedMethod typeMethod : table) { assert typeMethod.getDeclaringClass().equals(type) : typeMethod; - assert typeMethod.vtableIndex == -1 : typeMethod.vtableIndex; + assert typeMethod.vtableIndex == HostedMethod.MISSING_VTABLE_IDX : typeMethod.vtableIndex; typeMethod.vtableIndex = index; index++; } @@ -272,15 +274,6 @@ private void generateOpenTypeWorldDispatchTable(HostedInstanceClass type, Map { - @Override - protected void run(StructuredGraph graph, CoreProviders context) { - for (MethodCallTargetNode callTarget : graph.getNodes(MethodCallTargetNode.TYPE).snapshot()) { - if (callTarget.invokeKind().isIndirect()) { - maybeConvertCallTarget(callTarget); - } - } - } - - public void maybeConvertCallTarget(MethodCallTargetNode target) { - if (target.targetMethod() instanceof AnalysisMethod aMethod) { - AnalysisMethod indirectTarget = aMethod.getIndirectCallTarget(); - if (!indirectTarget.equals(aMethod)) { - target.setTargetMethod(indirectTarget); - } - } - } -} diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java index 4fb9c890dda4..4aa1a193a592 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java @@ -529,10 +529,12 @@ public Object transform(Object receiver, Object originalValue) { return SubstrateMethodAccessor.INTERFACE_TYPEID_UNNEEDED; } - HostedMethod member = ImageSingletons.lookup(ReflectionFeature.class).hostedMetaAccess().lookupJavaMethod(accessor.getMember()); - if (member.getDeclaringClass().isInterface()) { - return member.getDeclaringClass().getTypeID(); + HostedMethod method = ImageSingletons.lookup(ReflectionFeature.class).hostedMetaAccess().lookupJavaMethod(accessor.getMember()); + HostedMethod indirectCallTarget = method.getIndirectCallTarget(); + if (indirectCallTarget.getDeclaringClass().isInterface()) { + return indirectCallTarget.getDeclaringClass().getTypeID(); } + VMError.guarantee(method.equals(indirectCallTarget)); return SubstrateMethodAccessor.INTERFACE_TYPEID_CLASS_TABLE; } } From 59c90b2ac0d8bb2bb03a8acaa336c820aa764ae5 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Fri, 4 Jul 2025 19:21:30 +0200 Subject: [PATCH 2/5] fixup: address reviewer comments --- .../com/oracle/svm/graal/meta/SubstrateMethod.java | 1 - .../svm/hosted/imagelayer/SVMImageLayerLoader.java | 4 ++-- .../oracle/svm/hosted/jni/JNIAccessFeature.java | 5 ++--- .../com/oracle/svm/hosted/meta/HostedMethod.java | 14 +++++++------- .../svm/hosted/reflect/ReflectionFeature.java | 1 - 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java index f4556878bdba..94a118c72514 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateMethod.java @@ -201,7 +201,6 @@ public Object getRawImplementations() { public void setSubstrateDataAfterCompilation(SubstrateMethod indirectCallTarget, int vTableIndex) { this.indirectCallTarget = indirectCallTarget; this.vTableIndex = vTableIndex; - } public void setSubstrateDataAfterHeapLayout(int imageCodeOffset, int imageCodeDeoptOffset) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java index dff806bb72f6..c514d877cc95 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerLoader.java @@ -828,7 +828,7 @@ private void loadMethod(PersistedAnalysisMethod.Reader methodData) { type.getClassInitializer(); } else { ResolvedSignature signature = ResolvedSignature.fromArray(parameterTypes, returnType); - tryFindMethod(type, name, signature); + tryLoadMethod(type, name, signature); } } @@ -847,7 +847,7 @@ private void loadMethod(PersistedAnalysisMethod.Reader methodData) { * We also need this because reflection cannot find all declared methods within class; one * example it will not find is bridge methods inserted for covariant overrides. */ - private void tryFindMethod(AnalysisType type, String name, ResolvedSignature signature) { + private void tryLoadMethod(AnalysisType type, String name, ResolvedSignature signature) { ResolvedJavaType wrapped = type.getWrapped(); assert !(wrapped instanceof BaseLayerType) : type; for (ResolvedJavaMethod method : wrapped.getAllMethods(false)) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java index 6dbea9d7b51f..8e8d811df42a 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java @@ -616,9 +616,8 @@ private static void finishMethodBeforeCompilation(JNICallableJavaMethod method, if (SubstrateOptions.useClosedTypeWorldHubLayout()) { interfaceTypeID = JNIAccessibleMethod.INTERFACE_TYPEID_UNNEEDED; } else { - HostedType declaringClass = hTarget.getIndirectCallTarget().getDeclaringClass(); - interfaceTypeID = declaringClass.isInterface() ? declaringClass.getTypeID() : JNIAccessibleMethod.INTERFACE_TYPEID_CLASS_TABLE; - VMError.guarantee(!(interfaceTypeID == JNIAccessibleMethod.INTERFACE_TYPEID_CLASS_TABLE && !declaringClass.equals(hTarget.getDeclaringClass()))); + HostedType indirectCallTargetClass = hTarget.getIndirectCallTarget().getDeclaringClass(); + interfaceTypeID = indirectCallTargetClass.isInterface() ? indirectCallTargetClass.getTypeID() : JNIAccessibleMethod.INTERFACE_TYPEID_CLASS_TABLE; } } CodePointer nonvirtualTarget = new MethodPointer(hTarget); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java index fcdce4d546ac..e66458032dd9 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java @@ -107,7 +107,7 @@ public final class HostedMethod extends HostedElement implements SharedMethod, W * When using the open type world we must differentiate between this method's vtable index and * the vtable index used for virtual calls. This is because sometimes JVMCI will expose to * analysis special methods HotSpot introduces into vtables, such as miranda and overpass - * methods. In the open type word we only include declared methods in vtables and hence must + * methods. In the open type world we only include declared methods in vtables and hence must * adjust indirect call targets accordingly. * * Note normally {@code indirectCallTarget == this}. Only for special HotSpot methods such as @@ -117,8 +117,8 @@ public final class HostedMethod extends HostedElement implements SharedMethod, W * * For additional information, see {@link SharedMethod#getIndirectCallTarget}. */ - int indirectCallVTableIndex = MISSING_VTABLE_IDX; - HostedMethod indirectCallTarget = null; + private int indirectCallVTableIndex = MISSING_VTABLE_IDX; + private HostedMethod indirectCallTarget = null; /** * The address offset of the compiled code relative to the code of the first method in the @@ -383,9 +383,9 @@ public void setIndirectCallTarget(HostedMethod alias) { /* * When there is an indirectCallTarget installed which is not the original method, we * currently expect the target method to either have an interface as its declaring class - * or for the declaring class to be unchanged. If we change this in the future, then we - * will need to adjust the JNI (JNIAccessFeature) and reflection (ReflectionFeature) - * code as well. + * or for the declaring class to be unchanged. If the declaring class is different, then + * we must ensure that the layout of the vtable matches for all relevant indexes between + * the original and alias methods' declaring classes. */ VMError.guarantee(alias.getDeclaringClass().isInterface() || alias.getDeclaringClass().equals(getDeclaringClass()), "Invalid indirect call target for %s: %s", this, alias); } @@ -398,7 +398,7 @@ public HostedMethod getIndirectCallTarget() { return indirectCallTarget; } - public void finalizeVTableIndex(boolean closedTypeWorld) { + void finalizeVTableIndex(boolean closedTypeWorld) { if (closedTypeWorld) { /* * In the closed type word we do not have a different indirectCallTarget, so the diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java index 4aa1a193a592..1e6368999111 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionFeature.java @@ -534,7 +534,6 @@ public Object transform(Object receiver, Object originalValue) { if (indirectCallTarget.getDeclaringClass().isInterface()) { return indirectCallTarget.getDeclaringClass().getTypeID(); } - VMError.guarantee(method.equals(indirectCallTarget)); return SubstrateMethodAccessor.INTERFACE_TYPEID_CLASS_TABLE; } } From 25d78eeb64b554286cca013fd93e19c0da827500 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Fri, 4 Jul 2025 19:29:57 +0200 Subject: [PATCH 3/5] fixup: computedVTableIndex --- .../src/com/oracle/svm/hosted/meta/HostedMethod.java | 9 ++++----- .../src/com/oracle/svm/hosted/meta/VTableBuilder.java | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java index e66458032dd9..a27ca8df71fc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java @@ -95,13 +95,13 @@ public final class HostedMethod extends HostedElement implements SharedMethod, W private final ConstantPool constantPool; private final ExceptionHandler[] handlers; /** - * Contains the index of the method within the appropriate table. + * Contains the index of the method computed by {@link VTableBuilder}. * * Within the closed type world, there exists a single table which describes all methods. * However, within the open type world, each type and interface has a unique table, so this * index is relative to the start of the appropriate table. */ - int vtableIndex = MISSING_VTABLE_IDX; + int computedVTableIndex = MISSING_VTABLE_IDX; /** * When using the open type world we must differentiate between this method's vtable index and @@ -404,12 +404,12 @@ void finalizeVTableIndex(boolean closedTypeWorld) { * In the closed type word we do not have a different indirectCallTarget, so the * vtableIndex is always the original vtable index. */ - indirectCallVTableIndex = vtableIndex; + indirectCallVTableIndex = computedVTableIndex; } else { /* * In the open type word we must use the vtable index from the indirect call target. */ - indirectCallVTableIndex = indirectCallTarget.vtableIndex; + indirectCallVTableIndex = indirectCallTarget.computedVTableIndex; } } @@ -676,7 +676,6 @@ public HostedMethod getOrCreateMultiMethod(MultiMethodKey key) { return (HostedMethod) multiMethodMap.computeIfAbsent(key, (k) -> { HostedMethod newMultiMethod = create0(wrapped, holder, signature, constantPool, handlers, k, multiMethodMap, localVariableTable); newMultiMethod.implementations = implementations; - newMultiMethod.vtableIndex = vtableIndex; return newMultiMethod; }); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java index 233f39a883c6..33989bb3e9f1 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java @@ -215,8 +215,8 @@ private List generateDispatchTable(HostedType type, int startingIn int index = startingIndex; for (HostedMethod typeMethod : table) { assert typeMethod.getDeclaringClass().equals(type) : typeMethod; - assert typeMethod.vtableIndex == HostedMethod.MISSING_VTABLE_IDX : typeMethod.vtableIndex; - typeMethod.vtableIndex = index; + assert typeMethod.computedVTableIndex == HostedMethod.MISSING_VTABLE_IDX : typeMethod.computedVTableIndex; + typeMethod.computedVTableIndex = index; index++; } @@ -502,7 +502,7 @@ private void assignImplementations(HostedType type, Map Date: Fri, 4 Jul 2025 19:34:55 +0200 Subject: [PATCH 4/5] fixup: comment update --- .../src/com/oracle/svm/hosted/meta/HostedMethod.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java index a27ca8df71fc..4be7248db3ad 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java @@ -104,11 +104,8 @@ public final class HostedMethod extends HostedElement implements SharedMethod, W int computedVTableIndex = MISSING_VTABLE_IDX; /** - * When using the open type world we must differentiate between this method's vtable index and - * the vtable index used for virtual calls. This is because sometimes JVMCI will expose to - * analysis special methods HotSpot introduces into vtables, such as miranda and overpass - * methods. In the open type world we only include declared methods in vtables and hence must - * adjust indirect call targets accordingly. + * When using the open type world we must differentiate between the vtable index computed by + * {@link VTableBuilder} for this method and the vtable index used for virtual calls. * * Note normally {@code indirectCallTarget == this}. Only for special HotSpot methods such as * miranda and overpass methods will the indirectCallTarget be a different method. The logic for From 6e25248b011271921175261bff61471c0da0b373 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Fri, 4 Jul 2025 20:24:42 +0200 Subject: [PATCH 5/5] bug --- .../svm/hosted/OpenTypeWorldFeature.java | 5 +---- .../oracle/svm/hosted/meta/HostedMethod.java | 18 +++++------------- .../svm/hosted/meta/UniverseBuilder.java | 7 +++++++ .../oracle/svm/hosted/meta/VTableBuilder.java | 4 ++-- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java index fafd10015783..0447a199fb38 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/OpenTypeWorldFeature.java @@ -99,10 +99,7 @@ public void beforeCompilation(BeforeCompilationAccess access) { public static void computeIndirectCallTargets(HostedUniverse hUniverse, Map methods) { Map allInterfacesMap = new HashMap<>(); methods.forEach((aMethod, hMethod) -> { - if (!aMethod.isOriginalMethod()) { - // We don't need to set this; only original methods will be call targets - return; - } + assert aMethod.isOriginalMethod(); var aAlias = calculateIndirectCallTarget(allInterfacesMap, hMethod); HostedMethod hAlias; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java index 4be7248db3ad..d61cc7e407bf 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedMethod.java @@ -395,19 +395,8 @@ public HostedMethod getIndirectCallTarget() { return indirectCallTarget; } - void finalizeVTableIndex(boolean closedTypeWorld) { - if (closedTypeWorld) { - /* - * In the closed type word we do not have a different indirectCallTarget, so the - * vtableIndex is always the original vtable index. - */ - indirectCallVTableIndex = computedVTableIndex; - } else { - /* - * In the open type word we must use the vtable index from the indirect call target. - */ - indirectCallVTableIndex = indirectCallTarget.computedVTableIndex; - } + void finalizeIndirectCallVTableIndex() { + indirectCallVTableIndex = indirectCallTarget.computedVTableIndex; } @Override @@ -673,6 +662,9 @@ public HostedMethod getOrCreateMultiMethod(MultiMethodKey key) { return (HostedMethod) multiMethodMap.computeIfAbsent(key, (k) -> { HostedMethod newMultiMethod = create0(wrapped, holder, signature, constantPool, handlers, k, multiMethodMap, localVariableTable); newMultiMethod.implementations = implementations; + newMultiMethod.computedVTableIndex = computedVTableIndex; + newMultiMethod.indirectCallTarget = indirectCallTarget; + newMultiMethod.indirectCallVTableIndex = indirectCallVTableIndex; return newMultiMethod; }); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java index d52b2cba6f84..8aa9a1936787 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java @@ -182,8 +182,15 @@ public void build(DebugContext debug) { HostedMethod previous = hUniverse.methods.put(aMethod, origHMethod); assert previous == null : "Overwriting analysis key"; } + + // see SharedMethod#getIndirectCallTarget for more information if (!SubstrateOptions.useClosedTypeWorldHubLayout()) { OpenTypeWorldFeature.computeIndirectCallTargets(hUniverse, hUniverse.methods); + } else { + hUniverse.methods.forEach((aMethod, hMethod) -> { + assert aMethod.isOriginalMethod(); + hMethod.setIndirectCallTarget(hMethod); + }); } HostedConfiguration.initializeDynamicHubLayout(hMetaAccess); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java index 33989bb3e9f1..f6a1f9fac409 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/VTableBuilder.java @@ -60,10 +60,10 @@ public static void buildTables(HostedUniverse hUniverse, HostedMetaAccess hMetaA VTableBuilder builder = new VTableBuilder(hUniverse, hMetaAccess); if (SubstrateOptions.useClosedTypeWorldHubLayout()) { builder.buildClosedTypeWorldVTables(); - hUniverse.methods.forEach((k, v) -> v.finalizeVTableIndex(true)); + hUniverse.methods.forEach((k, v) -> v.finalizeIndirectCallVTableIndex()); } else { builder.buildOpenTypeWorldDispatchTables(); - hUniverse.methods.forEach((k, v) -> v.finalizeVTableIndex(false)); + hUniverse.methods.forEach((k, v) -> v.finalizeIndirectCallVTableIndex()); assert builder.verifyOpenTypeWorldDispatchTables(); } }