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 47beeaccdabf..0ffad7697992 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 @@ -51,6 +51,7 @@ import java.util.function.Supplier; import java.util.stream.IntStream; +import com.oracle.svm.hosted.substitute.SubstitutionMethod; import org.graalvm.collections.EconomicMap; import org.graalvm.nativeimage.AnnotationAccess; import org.graalvm.nativeimage.ImageSingletons; @@ -453,6 +454,18 @@ protected boolean delegateLoadType(PersistedAnalysisType.Reader typeData) { return false; } + /** + * The {@link SubstitutionMethod} contains less information than the original + * {@link ResolvedJavaMethod} and trying to access it can result in an exception. + */ + private static ResolvedJavaMethod getOriginalWrapped(AnalysisMethod method) { + ResolvedJavaMethod wrapped = method.getWrapped(); + if (wrapped instanceof SubstitutionMethod subst) { + return subst.getAnnotated(); + } + return wrapped; + } + /** * Load all lambda types of the given capturing class. Each method of the capturing class is * parsed (see {@link LambdaParser#createMethodGraph(ResolvedJavaMethod, OptionValues)}). The @@ -460,9 +473,13 @@ protected boolean delegateLoadType(PersistedAnalysisType.Reader typeData) { */ private void loadLambdaTypes(Class capturingClass) { capturingClasses.computeIfAbsent(capturingClass, key -> { + /* + * Getting the original wrapped method is important to avoid getting exceptions that + * would be ignored otherwise. + */ LambdaParser.allExecutablesDeclaredInClass(universe.getBigbang().getMetaAccess().lookupJavaType(capturingClass)) .filter(m -> m.getCode() != null) - .forEach(m -> loadLambdaTypes(((AnalysisMethod) m).getWrapped(), universe.getBigbang())); + .forEach(m -> loadLambdaTypes(getOriginalWrapped((AnalysisMethod) m), universe.getBigbang())); return true; }); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java index a35b6ae03477..76f717ade706 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java @@ -299,7 +299,13 @@ public String getMethodDescriptor(AnalysisMethod method) { if (originalMethod != null) { return addModuleName(originalMethod.toString(), moduleName); } - return addModuleName(getQualifiedName(method), moduleName); + /* + * The wrapped qualified method is needed here as the AnalysisMethod replaces unresolved + * parameter or return types with java.lang.Object, potentially causing method descriptor + * duplication. The wrapped method signature preserves the original type information, + * preventing this issue. + */ + return addModuleName(getWrappedQualifiedName(method), moduleName); } /* @@ -326,6 +332,10 @@ private static String getQualifiedName(AnalysisMethod method) { return method.getSignature().getReturnType().toJavaName(true) + " " + method.getQualifiedName(); } + private static String getWrappedQualifiedName(AnalysisMethod method) { + return method.wrapped.format("%R %H.%n(%P)"); + } + public static void forcePersistConstant(ImageHeapConstant imageHeapConstant) { AnalysisUniverse universe = imageHeapConstant.getType().getUniverse(); universe.getHeapScanner().markReachable(imageHeapConstant, ObjectScanner.OtherReason.PERSISTED); 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 934ca301d089..23530443260f 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 @@ -204,7 +204,13 @@ public class SVMImageLayerWriter extends ImageLayerWriter { private final MessageBuilder snapshotFileBuilder = new MessageBuilder(); private final SharedLayerSnapshot.Builder snapshotBuilder = this.snapshotFileBuilder.initRoot(SharedLayerSnapshot.factory); private Map constantsMap; - private final Map methodsMap = new ConcurrentHashMap<>(); + private final Map methodsMap = new ConcurrentHashMap<>(); + /** + * This map is only used for validation, to ensure that all method descriptors are unique. A + * duplicate method descriptor would cause methods to be incorrectly matched across layers, + * which is really hard to debug and can have unexpected consequences. + */ + private final Map methodDescriptors = new HashMap<>(); private final Map> polymorphicSignatureCallers = new ConcurrentHashMap<>(); private final GraphsOutput graphsOutput; private final boolean useSharedLayerGraphs; @@ -231,13 +237,13 @@ private record MethodGraphsInfo(String analysisGraphLocation, boolean analysisGr static final MethodGraphsInfo NO_GRAPHS = new MethodGraphsInfo(null, false, null); - MethodGraphsInfo withAnalysisGraph(String location, boolean isIntrinsic) { - assert analysisGraphLocation == null && !analysisGraphIsIntrinsic; + MethodGraphsInfo withAnalysisGraph(AnalysisMethod method, String location, boolean isIntrinsic) { + assert analysisGraphLocation == null && !analysisGraphIsIntrinsic : "Only one analysis graph can be persisted for a given method: " + method; return new MethodGraphsInfo(location, isIntrinsic, strengthenedGraphLocation); } - MethodGraphsInfo withStrengthenedGraph(String location) { - assert strengthenedGraphLocation == null; + MethodGraphsInfo withStrengthenedGraph(AnalysisMethod method, String location) { + assert strengthenedGraphLocation == null : "Only one strengthened graph can be persisted for a given method: " + method; return new MethodGraphsInfo(analysisGraphLocation, analysisGraphIsIntrinsic, location); } } @@ -379,6 +385,7 @@ public void persistAnalysisInfo() { AnalysisMethod[] methodsToPersist = aUniverse.getMethods().stream().filter(AnalysisMethod::isTrackedAcrossLayers).sorted(Comparator.comparingInt(AnalysisMethod::getId)) .toArray(AnalysisMethod[]::new); initSortedArray(snapshotBuilder::initMethods, methodsToPersist, this::persistMethod); + methodDescriptors.clear(); AnalysisField[] fieldsToPersist = aUniverse.getFields().stream().filter(AnalysisField::isTrackedAcrossLayers).sorted(Comparator.comparingInt(AnalysisField::getId)) .toArray(AnalysisField[]::new); @@ -582,18 +589,20 @@ private static void handleNameConflict(String message) { private void persistMethod(AnalysisMethod method, Supplier builderSupplier) { PersistedAnalysisMethod.Builder builder = builderSupplier.get(); - MethodGraphsInfo graphsInfo = methodsMap.putIfAbsent(imageLayerSnapshotUtil.getMethodDescriptor(method), MethodGraphsInfo.NO_GRAPHS); + MethodGraphsInfo graphsInfo = methodsMap.putIfAbsent(method, MethodGraphsInfo.NO_GRAPHS); Executable executable = method.getJavaMethod(); - if (builder.getId() != 0) { - throw GraalError.shouldNotReachHere("The method descriptor should be unique, but " + imageLayerSnapshotUtil.getMethodDescriptor(method) + " got added twice."); - } if (executable != null) { initStringList(builder::initArgumentClassNames, Arrays.stream(executable.getParameterTypes()).map(Class::getName)); builder.setClassName(executable.getDeclaringClass().getName()); } - builder.setDescriptor(imageLayerSnapshotUtil.getMethodDescriptor(method)); + String methodDescriptor = imageLayerSnapshotUtil.getMethodDescriptor(method); + if (methodDescriptors.put(methodDescriptor, method) != null) { + throw GraalError.shouldNotReachHere("The method descriptor should be unique, but %s got added twice.\nThe first method is %s and the second is %s." + .formatted(methodDescriptor, methodDescriptors.get(methodDescriptor), method)); + } + builder.setDescriptor(methodDescriptor); builder.setDeclaringTypeId(method.getDeclaringClass().getId()); initInts(builder::initArgumentTypeIds, method.getSignature().toParameterList(null).stream().mapToInt(AnalysisType::getId)); builder.setId(method.getId()); @@ -1053,18 +1062,19 @@ private static AnalysisMethod getRelocatableConstantMethod(MethodRef methodRef) @Override public void persistAnalysisParsedGraph(AnalysisMethod method, AnalysisParsedGraph analysisParsedGraph) { - String name = imageLayerSnapshotUtil.getMethodDescriptor(method); - MethodGraphsInfo graphsInfo = methodsMap.get(name); - if (graphsInfo == null || graphsInfo.analysisGraphLocation == null) { + /* + * A copy of the encoded graph is needed here because the nodeStartOffsets can be + * concurrently updated otherwise, which causes the ObjectCopier to fail. + */ + String location = persistGraph(method, new EncodedGraph(analysisParsedGraph.getEncodedGraph())); + if (location != null) { /* - * A copy of the encoded graph is needed here because the nodeStartOffsets can be - * concurrently updated otherwise, which causes the ObjectCopier to fail. + * This method should only be called once for each method. This check is performed by + * withAnalysisGraph as it will throw if the MethodGraphsInfo already has an analysis + * graph. */ - String location = persistGraph(method, new EncodedGraph(analysisParsedGraph.getEncodedGraph())); - if (location != null) { - methodsMap.compute(name, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS) - .withAnalysisGraph(location, analysisParsedGraph.isIntrinsic())); - } + methodsMap.compute(method, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS) + .withAnalysisGraph(method, location, analysisParsedGraph.isIntrinsic())); } } @@ -1073,14 +1083,14 @@ public void persistMethodStrengthenedGraph(AnalysisMethod method) { return; } - String name = imageLayerSnapshotUtil.getMethodDescriptor(method); - MethodGraphsInfo graphsInfo = methodsMap.get(name); - - if (graphsInfo == null || graphsInfo.strengthenedGraphLocation == null) { - EncodedGraph analyzedGraph = method.getAnalyzedGraph(); - String location = persistGraph(method, analyzedGraph); - methodsMap.compute(name, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS).withStrengthenedGraph(location)); - } + EncodedGraph analyzedGraph = method.getAnalyzedGraph(); + String location = persistGraph(method, analyzedGraph); + /* + * This method should only be called once for each method. This check is performed by + * withStrengthenedGraph as it will throw if the MethodGraphsInfo already has a strengthened + * graph. + */ + methodsMap.compute(method, (n, mgi) -> (mgi != null ? mgi : MethodGraphsInfo.NO_GRAPHS).withStrengthenedGraph(method, location)); } private String persistGraph(AnalysisMethod method, EncodedGraph analyzedGraph) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java index 5da155e86cdd..891424e3f645 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java @@ -259,10 +259,12 @@ public Object transform(Object receiver, Object originalValue) { for (var entry : cache.entrySet()) { SoftReference value = entry.getValue(); Object object = value.get(); - MethodHandle constructor = ReflectionUtil.invokeMethod(constructorGetter, object); - MethodHandle concatenator = ReflectionUtil.invokeMethod(concatenatorGetter, object); - if (constructor != null && concatenator != null && heapScanner.isObjectReachable(constructor) && heapScanner.isObjectReachable(concatenator)) { - result.put(entry.getKey(), value); + if (object != null) { + MethodHandle constructor = ReflectionUtil.invokeMethod(constructorGetter, object); + MethodHandle concatenator = ReflectionUtil.invokeMethod(concatenatorGetter, object); + if (constructor != null && concatenator != null && heapScanner.isObjectReachable(constructor) && heapScanner.isObjectReachable(concatenator)) { + result.put(entry.getKey(), value); + } } }