Fix underflow in MethodHandlePlugin

christianaistleitner · Feb 2, 2024 · b51e7bc · b51e7bc
1 parent 85d0a8e
commit b51e7bc
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 16 deletions.
diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/BytecodeParser.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/BytecodeParser.java
@@ -2948,6 +2948,11 @@ private <T extends Node> void updateLastInstruction(T v) {
         }
     }
 
+    @Override
+    public boolean hasParseTerminated() {
+        return lastInstr == null;
+    }
+
     private AbstractBeginNode updateWithExceptionNode(WithExceptionNode withExceptionNode) {
         if (withExceptionNode.exceptionEdge() == null) {
             AbstractBeginNode exceptionEdge = handleException(null, bci(), false);

diff --git a/...jdk.graal.compiler/src/jdk/graal/compiler/nodes/graphbuilderconf/GraphBuilderContext.java b/...jdk.graal.compiler/src/jdk/graal/compiler/nodes/graphbuilderconf/GraphBuilderContext.java
@@ -301,6 +301,13 @@ default boolean parsingIntrinsic() {
         return getIntrinsic() != null;
     }
 
+    /**
+     * Returns true if control flow has terminated in some fashion, such as a deoptimization.
+     */
+    default boolean hasParseTerminated() {
+        throw GraalError.unimplementedParent(); // ExcludeFromJacocoGeneratedReport
+    }
+
     /**
      * Determines if a graph builder plugin is enabled under current context.
      */

diff --git a/...ler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java b/...ler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/IntrinsicGraphBuilder.java
@@ -195,6 +195,11 @@ private <T extends Node> void updateLastInstruction(T v) {
         }
     }
 
+    @Override
+    public boolean hasParseTerminated() {
+        return lastInstr == null;
+    }
+
     @Override
     public AbstractBeginNode genExplicitExceptionEdge(BytecodeExceptionNode.BytecodeExceptionKind exceptionKind, ValueNode... exceptionArguments) {
         BytecodeExceptionNode exceptionNode = graph.add(new BytecodeExceptionNode(getMetaAccess(), exceptionKind, exceptionArguments));

diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/MethodHandlePlugin.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/MethodHandlePlugin.java
@@ -42,7 +42,6 @@
 import jdk.graal.compiler.replacements.nodes.MacroNode;
 import jdk.graal.compiler.replacements.nodes.MethodHandleNode;
 import jdk.graal.compiler.replacements.nodes.ResolvedMethodHandleCallTargetNode;
-
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.MethodHandleAccessProvider;
 import jdk.vm.ci.meta.MethodHandleAccessProvider.IntrinsicMethod;
@@ -129,21 +128,23 @@ public <T extends ValueNode> T add(T node) {
                     }
                 }
 
-                /*
-                 * After handleReplacedInvoke, a return type according to the signature of
-                 * targetMethod has been pushed. That can be different than the type expected by the
-                 * method handle invoke. Since there cannot be any implicit type conversion, the
-                 * only safe option actually is that the return type is not used at all. If there is
-                 * any other expected return type, the bytecodes are wrong. The JavaDoc of
-                 * MethodHandle.invokeBasic states that this "could crash the JVM", so bailing out
-                 * of compilation seems like a good idea.
-                 */
-                JavaKind invokeReturnKind = invokeReturnStamp.getTrustedStamp().getStackKind();
-                JavaKind targetMethodReturnKind = targetMethod.getSignature().getReturnKind().getStackKind();
-                if (invokeReturnKind != targetMethodReturnKind) {
-                    b.pop(targetMethodReturnKind);
-                    if (invokeReturnKind != JavaKind.Void) {
-                        throw b.bailout("Cannot do any type conversion when invoking method handle, so return value must remain popped");
+                if (!b.hasParseTerminated()) {
+                    /*
+                     * After handleReplacedInvoke, a return type according to the signature of
+                     * targetMethod has been pushed. That can be different than the type expected by
+                     * the method handle invoke. Since there cannot be any implicit type conversion,
+                     * the only safe option actually is that the return type is not used at all. If
+                     * there is any other expected return type, the bytecodes are wrong. The JavaDoc
+                     * of MethodHandle.invokeBasic states that this "could crash the JVM", so
+                     * bailing out of compilation seems like a good idea.
+                     */
+                    JavaKind invokeReturnKind = invokeReturnStamp.getTrustedStamp().getStackKind();
+                    JavaKind targetMethodReturnKind = targetMethod.getSignature().getReturnKind().getStackKind();
+                    if (invokeReturnKind != targetMethodReturnKind) {
+                        b.pop(targetMethodReturnKind);
+                        if (invokeReturnKind != JavaKind.Void) {
+                            throw b.bailout("Cannot do any type conversion when invoking method handle, so return value must remain popped");
+                        }
                     }
                 }
             }