Skip to content

Commit

Permalink
[GR-49579] Backport to 23.1: Handle unlinked types in runtime compila…
Browse files Browse the repository at this point in the history
…tion.

PullRequest: graal/15843
  • Loading branch information
teshull committed Nov 26, 2023
2 parents b798f3a + f731804 commit d8f3d82
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)//
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -94,6 +96,10 @@
* It is mainly used to replace the Hosted* meta data with the Substrate* meta data.
*/
public class GraalGraphObjectReplacer implements Function<Object, Object> {
public static class Options {
@Option(help = "Ensure all created SubstrateTypes are linked")//
public static final HostedOptionKey<Boolean> GuaranteeSubstrateTypesLinked = new HostedOptionKey<>(false);
}

private final AnalysisUniverse aUniverse;
private final ConcurrentMap<AnalysisMethod, SubstrateMethod> methods = ObservableImageHeapMapProvider.create();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<AnalysisType, SubstrateType> entry : types.entrySet()) {
AnalysisType aType = entry.getKey();
SubstrateType sType = entry.getValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -281,26 +280,17 @@ private void processMethod(CallTreeNode node, Deque<CallTreeNode> 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) {
new DeoptimizeOnExceptionPhase(deoptimizeOnExceptionPredicate).apply(graph);
}
new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graph, hostedProviders);

if (DeoptimizationUtils.createGraphInvalidator(graph).get()) {
return;
}

graphEncoder.prepare(graph);
node.graph = graph;
assert RuntimeCompilationFeature.verifyNodes(graph);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -105,20 +104,12 @@ public GraphBuilderConfiguration updateGraphBuilderConfiguration(GraphBuilderCon
@Override
public boolean validateGraph(PointsToAnalysis bb, StructuredGraph graph) {
PointsToAnalysisMethod aMethod = (PointsToAnalysisMethod) graph.method();
Supplier<Boolean> hasStackValues = () -> graph.getNodes(StackValueNode.TYPE).isNotEmpty();
Supplier<Boolean> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -830,27 +829,18 @@ 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) {
new DeoptimizeOnExceptionPhase(deoptimizeOnExceptionPredicate).apply(graph);
}
new ConvertDeoptimizeToGuardPhase(canonicalizer).apply(graph, analysisProviders);

if (DeoptimizationUtils.createGraphInvalidator(graph).get()) {
recordFailed(method);
return HostVM.PARSING_FAILED;
}

} catch (Throwable ex) {
debug.handle(ex);
}
Expand All @@ -867,22 +857,13 @@ private void recordFailed(AnalysisMethod method) {
public boolean validateGraph(PointsToAnalysis bb, StructuredGraph graph) {
PointsToAnalysisMethod aMethod = (PointsToAnalysisMethod) graph.method();
MultiMethod.MultiMethodKey multiMethodKey = aMethod.getMultiMethodKey();
Supplier<Boolean> hasStackValues = () -> graph.getNodes(StackValueNode.TYPE).isNotEmpty();
if (aMethod.isOriginalMethod() && DeoptimizationUtils.canDeoptForTesting(aMethod, false, hasStackValues)) {
Supplier<Boolean> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<Boolean> containsStackValueNodes) {
public static boolean canDeoptForTesting(AnalysisMethod method, boolean deoptimizeAll, Supplier<Boolean> graphInvalidator) {
if (SubstrateCompilationDirectives.singleton().isRegisteredForDeoptTesting(method)) {
return true;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -486,4 +480,38 @@ public static Collection<ResolvedJavaMethod> 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<Boolean> 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;
};
}
}

0 comments on commit d8f3d82

Please sign in to comment.