From f73180430dfe888698e6cfbd16f242a0320536d5 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Wed, 11 Oct 2023 12:32:30 +0200 Subject: [PATCH] handle unlinked types in runtime compilation. --- .../com/oracle/svm/core/hub/DynamicHub.java | 14 ++++- .../hosted/GraalGraphObjectReplacer.java | 14 ++++- .../LegacyRuntimeCompilationFeature.java | 18 ++----- .../hosted/ParseOnceDeoptTestFeature.java | 15 ++---- .../ParseOnceRuntimeCompilationFeature.java | 35 +++---------- .../oracle/svm/graal/meta/SubstrateType.java | 6 ++- .../src/com/oracle/svm/hosted/SVMHost.java | 3 +- .../svm/hosted/code/DeoptimizationUtils.java | 52 ++++++++++++++----- 8 files changed, 86 insertions(+), 71 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java index fc0871a2fab6..cfc609d5fc2c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java @@ -245,6 +245,11 @@ public final class DynamicHub implements AnnotatedElement, java.lang.reflect.Typ * circumstances. */ private static final int IS_LAMBDA_FORM_HIDDEN_BIT = 9; + /** + * Indicates this Class was linked during build time. Accessing an unlinked class during run + * time will throw an error. + */ + private static final int IS_LINKED_BIT = 10; /** Similar to {@link #flags}, but non-final because {@link #setData} sets the value. */ @UnknownPrimitiveField(availability = AfterHostedUniverse.class)// @@ -385,7 +390,7 @@ public final class DynamicHub implements AnnotatedElement, java.lang.reflect.Typ @Platforms(Platform.HOSTED_ONLY.class) public DynamicHub(Class hostedJavaClass, String name, int hubType, ReferenceType referenceType, DynamicHub superType, DynamicHub componentHub, String sourceFileName, int modifiers, ClassLoader classLoader, boolean isHidden, boolean isRecord, Class nestHost, boolean assertionStatus, boolean hasDefaultMethods, boolean declaresDefaultMethods, - boolean isSealed, boolean isVMInternal, boolean isLambdaFormHidden, String simpleBinaryName, Object declaringClass) { + boolean isSealed, boolean isVMInternal, boolean isLambdaFormHidden, boolean isLinked, String simpleBinaryName, Object declaringClass) { this.hostedJavaClass = hostedJavaClass; this.module = hostedJavaClass.getModule(); this.name = name; @@ -408,7 +413,8 @@ public DynamicHub(Class hostedJavaClass, String name, int hubType, ReferenceT makeFlag(DECLARES_DEFAULT_METHODS_FLAG_BIT, declaresDefaultMethods) | makeFlag(IS_SEALED_FLAG_BIT, isSealed) | makeFlag(IS_VM_INTERNAL_FLAG_BIT, isVMInternal) | - makeFlag(IS_LAMBDA_FORM_HIDDEN_BIT, isLambdaFormHidden)); + makeFlag(IS_LAMBDA_FORM_HIDDEN_BIT, isLambdaFormHidden) | + makeFlag(IS_LINKED_BIT, isLinked)); this.companion = new DynamicHubCompanion(hostedJavaClass, classLoader); } @@ -887,6 +893,10 @@ public boolean isLambdaFormHidden() { return isFlagSet(flags, IS_LAMBDA_FORM_HIDDEN_BIT); } + public boolean isLinked() { + return isFlagSet(flags, IS_LINKED_BIT); + } + public boolean isRegisteredForSerialization() { return isFlagSet(additionalFlags, IS_REGISTERED_FOR_SERIALIZATION); } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalGraphObjectReplacer.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalGraphObjectReplacer.java index c79525081653..c039f6c75651 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalGraphObjectReplacer.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalGraphObjectReplacer.java @@ -40,6 +40,7 @@ import org.graalvm.compiler.hotspot.SnippetResolvedJavaMethod; import org.graalvm.compiler.hotspot.SnippetResolvedJavaType; import org.graalvm.compiler.nodes.FieldLocationIdentity; +import org.graalvm.compiler.options.Option; import org.graalvm.compiler.phases.util.Providers; import org.graalvm.nativeimage.c.function.RelocatedPointer; import org.graalvm.nativeimage.hosted.Feature.CompilationAccess; @@ -53,6 +54,7 @@ import com.oracle.svm.core.SubstrateUtil; import com.oracle.svm.core.graal.nodes.SubstrateFieldLocationIdentity; import com.oracle.svm.core.hub.DynamicHub; +import com.oracle.svm.core.option.HostedOptionKey; import com.oracle.svm.core.util.HostedStringDeduplication; import com.oracle.svm.core.util.ObservableImageHeapMapProvider; import com.oracle.svm.core.util.VMError; @@ -94,6 +96,10 @@ * It is mainly used to replace the Hosted* meta data with the Substrate* meta data. */ public class GraalGraphObjectReplacer implements Function { + public static class Options { + @Option(help = "Ensure all created SubstrateTypes are linked")// + public static final HostedOptionKey GuaranteeSubstrateTypesLinked = new HostedOptionKey<>(false); + } private final AnalysisUniverse aUniverse; private final ConcurrentMap methods = ObservableImageHeapMapProvider.create(); @@ -327,7 +333,6 @@ public synchronized SubstrateType createType(JavaType original) { } AnalysisType aType = toAnalysisType(original); - VMError.guarantee(aType.isLinked(), "Types reachable for JIT compilation must not have linkage errors"); SubstrateType sType = types.get(aType); if (sType == null) { VMError.guarantee(!(forbidNewTypes || (original instanceof HostedType)), "Too late to create a new type: %s", aType); @@ -422,6 +427,13 @@ public void setMethodsImplementations() { @SuppressWarnings("try") public void updateSubstrateDataAfterCompilation(HostedUniverse hUniverse, Providers providers) { + if (Options.GuaranteeSubstrateTypesLinked.getValue()) { + var unlinkedTypes = types.values().stream().filter(sType -> !sType.isLinked()).map(SubstrateType::toString).toList(); + if (!unlinkedTypes.isEmpty()) { + throw VMError.shouldNotReachHere(String.format("Unlinked SubstrateTypes have been created:%n%s", String.join(System.lineSeparator(), unlinkedTypes))); + } + } + for (Map.Entry entry : types.entrySet()) { AnalysisType aType = entry.getKey(); SubstrateType sType = entry.getValue(); diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/LegacyRuntimeCompilationFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/LegacyRuntimeCompilationFeature.java index 465cd01c2d10..83af30f70f75 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/LegacyRuntimeCompilationFeature.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/LegacyRuntimeCompilationFeature.java @@ -76,7 +76,6 @@ import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod; import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.config.ConfigurationValues; -import com.oracle.svm.core.graal.stackvalue.StackValueNode; import com.oracle.svm.core.util.VMError; import com.oracle.svm.graal.GraalSupport; import com.oracle.svm.graal.meta.SubstrateMethod; @@ -281,19 +280,6 @@ private void processMethod(CallTreeNode node, Deque worklist, BigB builderPhase.apply(graph); } - if (graph.getNodes(StackValueNode.TYPE).isNotEmpty()) { - /* - * Stack allocated memory is not seen by the deoptimization code, i.e., it is - * not copied in case of deoptimization. Also, pointers to it can be used for - * arbitrary address arithmetic, so we would not know how to update derived - * pointers into stack memory during deoptimization. Therefore, we cannot allow - * methods that allocate stack memory for runtime compilation. To remove this - * limitation, we would need to change how we handle stack allocated memory in - * Graal. - */ - return; - } - CanonicalizerPhase canonicalizer = CanonicalizerPhase.create(); canonicalizer.apply(graph, hostedProviders); if (deoptimizeOnExceptionPredicate != null) { @@ -301,6 +287,10 @@ private void processMethod(CallTreeNode node, Deque worklist, BigB } new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graph, hostedProviders); + if (DeoptimizationUtils.createGraphInvalidator(graph).get()) { + return; + } + graphEncoder.prepare(graph); node.graph = graph; assert RuntimeCompilationFeature.verifyNodes(graph); diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java index 6855cee13cd7..acd16457e3c3 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java @@ -50,7 +50,6 @@ import com.oracle.svm.common.meta.MultiMethod; import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.graal.snippets.DeoptTester; -import com.oracle.svm.core.graal.stackvalue.StackValueNode; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.SVMHost; import com.oracle.svm.hosted.analysis.SVMParsingSupport; @@ -105,20 +104,12 @@ public GraphBuilderConfiguration updateGraphBuilderConfiguration(GraphBuilderCon @Override public boolean validateGraph(PointsToAnalysis bb, StructuredGraph graph) { PointsToAnalysisMethod aMethod = (PointsToAnalysisMethod) graph.method(); - Supplier hasStackValues = () -> graph.getNodes(StackValueNode.TYPE).isNotEmpty(); + Supplier graphInvalidator = DeoptimizationUtils.createGraphInvalidator(graph); if (aMethod.isDeoptTarget()) { - /* - * Stack allocated memory is not seen by the deoptimization code, i.e., it is not - * copied in case of deoptimization. Also, pointers to it can be used for arbitrary - * address arithmetic, so we would not know how to update derived pointers into - * stack memory during deoptimization. Therefore, we cannot allow methods that - * allocate stack memory for runtime compilation. To remove this limitation, we - * would need to change how we handle stack allocated memory in Graal. - */ - return !hasStackValues.get(); + return !graphInvalidator.get(); } else { boolean canDeoptForTesting = aMethod.isOriginalMethod() && - DeoptimizationUtils.canDeoptForTesting(aMethod, DeoptTester.enabled(), hasStackValues); + DeoptimizationUtils.canDeoptForTesting(aMethod, DeoptTester.enabled(), graphInvalidator); if (canDeoptForTesting) { DeoptimizationUtils.registerDeoptEntriesForDeoptTesting(bb, graph, aMethod); } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java index 1ac638f0f059..3ef422596357 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java @@ -106,7 +106,6 @@ import com.oracle.svm.core.graal.nodes.DeoptEntrySupport; import com.oracle.svm.core.graal.nodes.DeoptProxyAnchorNode; import com.oracle.svm.core.graal.nodes.InlinedInvokeArgumentsNode; -import com.oracle.svm.core.graal.stackvalue.StackValueNode; import com.oracle.svm.core.option.HostedOptionKey; import com.oracle.svm.core.option.HostedOptionValues; import com.oracle.svm.core.util.VMError; @@ -831,20 +830,6 @@ private Object parseRuntimeCompiledMethod(BigBang bb, DebugContext debug, Analys } } - if (graph.getNodes(StackValueNode.TYPE).isNotEmpty()) { - /* - * Stack allocated memory is not seen by the deoptimization code, i.e., it is - * not copied in case of deoptimization. Also, pointers to it can be used for - * arbitrary address arithmetic, so we would not know how to update derived - * pointers into stack memory during deoptimization. Therefore, we cannot allow - * methods that allocate stack memory for runtime compilation. To remove this - * limitation, we would need to change how we handle stack allocated memory in - * Graal. - */ - recordFailed(method); - return HostVM.PARSING_FAILED; - } - CanonicalizerPhase canonicalizer = CanonicalizerPhase.create(); canonicalizer.apply(graph, analysisProviders); if (deoptimizeOnExceptionPredicate != null) { @@ -852,6 +837,11 @@ private Object parseRuntimeCompiledMethod(BigBang bb, DebugContext debug, Analys } new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graph, analysisProviders); + if (DeoptimizationUtils.createGraphInvalidator(graph).get()) { + recordFailed(method); + return HostVM.PARSING_FAILED; + } + } catch (Throwable ex) { debug.handle(ex); } @@ -868,22 +858,13 @@ private void recordFailed(AnalysisMethod method) { public boolean validateGraph(PointsToAnalysis bb, StructuredGraph graph) { PointsToAnalysisMethod aMethod = (PointsToAnalysisMethod) graph.method(); MultiMethod.MultiMethodKey multiMethodKey = aMethod.getMultiMethodKey(); - Supplier hasStackValues = () -> graph.getNodes(StackValueNode.TYPE).isNotEmpty(); - if (aMethod.isOriginalMethod() && DeoptimizationUtils.canDeoptForTesting(aMethod, false, hasStackValues)) { + Supplier graphInvalidator = DeoptimizationUtils.createGraphInvalidator(graph); + if (aMethod.isOriginalMethod() && DeoptimizationUtils.canDeoptForTesting(aMethod, false, graphInvalidator)) { DeoptimizationUtils.registerDeoptEntriesForDeoptTesting(bb, graph, aMethod); return true; } if (multiMethodKey != ORIGINAL_METHOD) { - if (hasStackValues.get()) { - /* - * Stack allocated memory is not seen by the deoptimization code, i.e., it is - * not copied in case of deoptimization. Also, pointers to it can be used for - * arbitrary address arithmetic, so we would not know how to update derived - * pointers into stack memory during deoptimization. Therefore, we cannot allow - * methods that allocate stack memory for runtime compilation. To remove this - * limitation, we would need to change how we handle stack allocated memory in - * Graal. - */ + if (graphInvalidator.get()) { recordFailed(aMethod); return false; } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java index 85569dbd119e..97ed0ba205cc 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java @@ -421,12 +421,14 @@ public ResolvedJavaMethod getClassInitializer() { @Override public boolean isLinked() { - return true; // types are always linked + return hub.isLinked(); } @Override public void link() { - // do nothing + if (!isLinked()) { + throw new LinkageError(String.format("Cannot link new type at run time: %s", this)); + } } @Override 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 17325a678ca7..f5e9099fd4e4 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 @@ -418,9 +418,10 @@ private DynamicHub createHub(AnalysisType type) { boolean isSealed = javaClass.isSealed(); boolean isVMInternal = type.isAnnotationPresent(InternalVMMethod.class); boolean isLambdaFormHidden = type.isAnnotationPresent(LambdaFormHiddenMethod.class); + boolean isLinked = type.isLinked(); return new DynamicHub(javaClass, className, computeHubType(type), computeReferenceType(type), superHub, componentHub, sourceFileName, modifiers, hubClassLoader, - isHidden, isRecord, nestHost, assertionStatus, type.hasDefaultMethods(), type.declaresDefaultMethods(), isSealed, isVMInternal, isLambdaFormHidden, simpleBinaryName, + isHidden, isRecord, nestHost, assertionStatus, type.hasDefaultMethods(), type.declaresDefaultMethods(), isSealed, isVMInternal, isLambdaFormHidden, isLinked, simpleBinaryName, getDeclaringClass(javaClass)); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/DeoptimizationUtils.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/DeoptimizationUtils.java index 2e593a567870..c8da2490dad0 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/DeoptimizationUtils.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/DeoptimizationUtils.java @@ -37,6 +37,8 @@ import org.graalvm.compiler.code.CompilationResult; import org.graalvm.compiler.graph.Node; +import org.graalvm.compiler.graph.iterators.NodePredicate; +import org.graalvm.compiler.graph.iterators.NodePredicates; import org.graalvm.compiler.lir.RedundantMoveElimination; import org.graalvm.compiler.lir.alloc.RegisterAllocationPhase; import org.graalvm.compiler.lir.phases.LIRPhase; @@ -71,6 +73,7 @@ import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod; import com.oracle.svm.common.meta.MultiMethod; import com.oracle.svm.core.Uninterruptible; +import com.oracle.svm.core.classinitialization.EnsureClassInitializedNode; import com.oracle.svm.core.code.FrameInfoEncoder; import com.oracle.svm.core.deopt.DeoptEntryInfopoint; import com.oracle.svm.core.deopt.DeoptTest; @@ -124,7 +127,7 @@ static void insertDeoptTests(HostedMethod method, StructuredGraph graph) { * * Note this should only be called within CompileQueue#parseAheadOfTimeCompiledMethods */ - public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimizeAll, Supplier containsStackValueNodes) { + public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimizeAll, Supplier graphInvalidator) { if (SubstrateCompilationDirectives.singleton().isRegisteredForDeoptTesting(method)) { return true; } @@ -143,16 +146,7 @@ public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimi return false; } - if (containsStackValueNodes.get()) { - /* - * Stack allocated memory is not seen by the deoptimization code, i.e., it is not copied - * in case of deoptimization. Also, pointers to it can be used for arbitrary address - * arithmetic, so we would not know how to update derived pointers into stack memory - * during deoptimization. Therefore, we cannot allow methods that allocate stack memory - * for runtime compilation. To remove this limitation, we would need to change how we - * handle stack allocated memory in Graal. - */ - + if (graphInvalidator.get()) { return false; } @@ -297,7 +291,7 @@ static boolean verifyDeoptTarget(HostedMethod method, StructuredGraph graph, Com /* * No deopt targets can have a StackValueNode in the graph. */ - assert graph.getNodes(StackValueNode.TYPE).isEmpty() : "No stack value nodes must be present in deopt target."; + assert !createGraphInvalidator(graph).get() : "Invalid nodes in deopt target: " + graph; for (Infopoint infopoint : result.getInfopoints()) { if (infopoint.debugInfo != null) { @@ -486,4 +480,38 @@ public static Collection registerDeoptEntries(StructuredGrap return changedMethods; } + + /* + * Stack allocated memory is not seen by the deoptimization code, i.e., it is not copied in case + * of deoptimization. Also, pointers to it can be used for arbitrary address arithmetic, so we + * would not know how to update derived pointers into stack memory during deoptimization. + * Therefore, we cannot allow methods that allocate stack memory for runtime compilation. To + * remove this limitation, we would need to change how we handle stack allocated memory in + * Graal. + * + * We also do not allow class initialization at run time to ensure the partial evaluator does + * not constant fold uninitialized fields. + */ + private static final NodePredicate invalidNodes = n -> NodePredicates.isA(StackValueNode.class).or(NodePredicates.isA(EnsureClassInitializedNode.class)).test(n); + + /** + * @return Supplier which returns true if the graph contains invalid nodes. + */ + public static Supplier createGraphInvalidator(StructuredGraph graph) { + return () -> { + if (!graph.method().getDeclaringClass().isInitialized()) { + /* + * All types which are used at run time should build-time initialized. This ensures + * the partial evaluator does not constant fold uninitialized fields. + */ + return true; + } + + if (graph.getNodes().filter(invalidNodes).isNotEmpty()) { + return true; + } + + return false; + }; + } }