8221577: [Graal] Implement basic type consistency checks for Low level MH intrinsics
authorjcm
Mon, 08 Jul 2019 04:01:54 -0700
changeset 55619 3595fb8794c5
parent 55618 978b2201984c
child 55620 eb1fbe4d61b2
8221577: [Graal] Implement basic type consistency checks for Low level MH intrinsics Reviewed-by: kvn
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/CustomizedBytecodePatternTest.java
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/FrameStateBuilder.java
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements.test/src/org/graalvm/compiler/replacements/test/InvokerSignatureMismatchTest.java
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/MethodHandleNode.java
test/hotspot/jtreg/ProblemList-graal.txt
--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/CustomizedBytecodePatternTest.java	Mon Jul 08 10:56:05 2019 +0200
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/CustomizedBytecodePatternTest.java	Mon Jul 08 04:01:54 2019 -0700
@@ -24,14 +24,44 @@
 
 package org.graalvm.compiler.core.test;
 
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Method;
+import java.security.ProtectionDomain;
+
 import org.objectweb.asm.Opcodes;
 
+import sun.misc.Unsafe;
+
 public abstract class CustomizedBytecodePatternTest extends GraalCompilerTest implements Opcodes {
 
     protected Class<?> getClass(String className) throws ClassNotFoundException {
         return new CachedLoader(CustomizedBytecodePatternTest.class.getClassLoader(), className).findClass(className);
     }
 
+    /**
+     * @param className
+     * @param lookUp lookup object with boot class load capability (required for jdk 9 and above)
+     * @return loaded class
+     * @throws ClassNotFoundException
+     */
+    protected Class<?> getClassBL(String className, MethodHandles.Lookup lookUp) throws ClassNotFoundException {
+        byte[] gen = generateClass(className.replace('.', '/'));
+        Method defineClass = null;
+        Class<?> loadedClass = null;
+        try {
+            if (Java8OrEarlier) {
+                defineClass = Unsafe.class.getDeclaredMethod("defineClass", String.class, byte[].class, int.class, int.class, ClassLoader.class, ProtectionDomain.class);
+                loadedClass = (Class<?>) defineClass.invoke(UNSAFE, className, gen, 0, gen.length, null, null);
+            } else {
+                defineClass = MethodHandles.lookup().getClass().getDeclaredMethod("defineClass", byte[].class);
+                loadedClass = (Class<?>) defineClass.invoke(lookUp, gen);
+            }
+        } catch (Exception e) {
+            throw new ClassNotFoundException();
+        }
+        return loadedClass;
+    }
+
     private class CachedLoader extends ClassLoader {
 
         final String className;
--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/FrameStateBuilder.java	Mon Jul 08 10:56:05 2019 +0200
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/FrameStateBuilder.java	Mon Jul 08 04:01:54 2019 -0700
@@ -275,8 +275,6 @@
         clearNonLiveLocals = other.clearNonLiveLocals;
         monitorIds = other.monitorIds.length == 0 ? other.monitorIds : other.monitorIds.clone();
 
-        assert locals.length == code.getMaxLocals();
-        assert stack.length == Math.max(1, code.getMaxStackSize());
         assert lockedObjects.length == monitorIds.length;
     }
 
@@ -791,7 +789,7 @@
     public ValueNode pop(JavaKind slotKind) {
         if (slotKind.needsTwoSlots()) {
             ValueNode s = xpop();
-            assert s == TWO_SLOT_MARKER;
+            assert s == TWO_SLOT_MARKER : s;
         }
         ValueNode x = xpop();
         assert verifyKind(slotKind, x);
@@ -835,7 +833,7 @@
                 /* Ignore second slot of two-slot value. */
                 x = xpop();
             }
-            assert x != null && x != TWO_SLOT_MARKER;
+            assert x != null && x != TWO_SLOT_MARKER : x;
             result[i] = x;
         }
         return result;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements.test/src/org/graalvm/compiler/replacements/test/InvokerSignatureMismatchTest.java	Mon Jul 08 04:01:54 2019 -0700
@@ -0,0 +1,184 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+
+package org.graalvm.compiler.replacements.test;
+
+import static org.graalvm.compiler.test.SubprocessUtil.getVMCommandLine;
+import static org.graalvm.compiler.test.SubprocessUtil.withoutDebuggerArguments;
+
+import org.junit.Test;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Type;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.List;
+
+import org.graalvm.compiler.core.test.CustomizedBytecodePatternTest;
+import org.graalvm.compiler.test.SubprocessUtil;
+import org.graalvm.compiler.test.SubprocessUtil.Subprocess;
+
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+
+public class InvokerSignatureMismatchTest {
+
+    @Test
+    public void test() throws Throwable {
+        List<String> args = withoutDebuggerArguments(getVMCommandLine());
+        String classPath = System.getProperty("java.class.path");
+        classPath = classPath + File.pathSeparator + TestISMBL.class.getProtectionDomain().getCodeSource().getLocation().getPath();
+        args.add("-Xbootclasspath/a:" + classPath);
+        args.add("-XX:-TieredCompilation");
+        args.add("-XX:+EnableJVMCI");
+        args.add("-XX:+UseJVMCICompiler");
+
+        args.add(TestISMBL.class.getName());
+        Subprocess proc = SubprocessUtil.java(args);
+        if (proc.exitCode != 0) {
+            System.out.println(proc);
+        }
+    }
+}
+
+class TestISMBL extends CustomizedBytecodePatternTest {
+
+    public static void main(String[] args) {
+        try {
+            new TestISMBL().test();
+        } catch (Throwable e) {
+            e.printStackTrace();
+            System.exit(1);
+        }
+        System.exit(0);
+    }
+
+    private void test() throws Throwable {
+        getClass("java/lang/invoke/MHHelper");
+        Class<?> testClass = getClass("ISMTest");
+
+        ResolvedJavaMethod mL = getResolvedJavaMethod(testClass, "mainLink");
+        ResolvedJavaMethod mI = getResolvedJavaMethod(testClass, "mainInvoke");
+        executeActual(mL, null, 100);
+        executeActual(mI, null, 100);
+    }
+
+    @Override
+    protected Class<?> getClass(String className) throws ClassNotFoundException {
+        if (className.equals("java/lang/invoke/MHHelper")) {
+            return super.getClassBL(className, MethodHandles.lookup());
+        } else {
+            return super.getClass(className);
+        }
+    }
+
+    @Override
+    protected byte[] generateClass(String className) {
+        String[] exceptions = new String[]{"java/lang/Throwable"};
+        ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
+        cw.visit(52, ACC_SUPER | ACC_PUBLIC, className, null, "java/lang/Object", null);
+
+        if (className.equals("java/lang/invoke/MHHelper")) {
+            MethodVisitor internalMemberName = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "internalMemberName", "(Ljava/lang/invoke/MethodHandle;)Ljava/lang/Object;", null, exceptions);
+            internalMemberName.visitCode();
+            internalMemberName.visitVarInsn(ALOAD, 0);
+            internalMemberName.visitMethodInsn(INVOKEVIRTUAL, "java/lang/invoke/MethodHandle", "internalMemberName", "()Ljava/lang/invoke/MemberName;", false);
+            internalMemberName.visitInsn(ARETURN);
+            internalMemberName.visitMaxs(1, 1);
+            internalMemberName.visitEnd();
+
+            MethodVisitor linkToStatic = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "linkToStatic", "(FLjava/lang/Object;)I", null, exceptions);
+            linkToStatic.visitCode();
+            linkToStatic.visitVarInsn(FLOAD, 0);
+            linkToStatic.visitVarInsn(ALOAD, 1);
+            linkToStatic.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MethodHandle", "linkToStatic", "(FLjava/lang/Object;)I", false);
+            linkToStatic.visitInsn(IRETURN);
+            linkToStatic.visitMaxs(1, 1);
+            linkToStatic.visitEnd();
+
+            MethodVisitor invokeBasicI = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "invokeBasicI", "(Ljava/lang/invoke/MethodHandle;F)I", null, exceptions);
+            invokeBasicI.visitCode();
+            invokeBasicI.visitVarInsn(ALOAD, 0);
+            invokeBasicI.visitVarInsn(FLOAD, 1);
+            invokeBasicI.visitMethodInsn(INVOKEVIRTUAL, "java/lang/invoke/MethodHandle", "invokeBasic", "(F)I", false);
+            invokeBasicI.visitInsn(IRETURN);
+            invokeBasicI.visitMaxs(1, 1);
+            invokeBasicI.visitEnd();
+
+        } else {
+            assert className.equals("ISMTest") : className;
+            cw.visitField(ACC_FINAL | ACC_STATIC, "INT_MH", "Ljava/lang/invoke/MethodHandle;", null, null).visitAnnotation("Ljava/lang/invoke/Stable.class;", true).visitEnd();
+            MethodVisitor clinit = cw.visitMethod(ACC_STATIC, "<clinit>", "()V", null, exceptions);
+            clinit.visitCode();
+            clinit.visitInsn(ACONST_NULL);
+            clinit.visitVarInsn(ASTORE, 0);
+            clinit.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MethodHandles", "lookup", "()Ljava/lang/invoke/MethodHandles$Lookup;", false);
+            clinit.visitLdcInsn(Type.getObjectType(className));
+            clinit.visitLdcInsn("bodyI");
+            clinit.visitFieldInsn(GETSTATIC, "java/lang/Integer", "TYPE", "Ljava/lang/Class;");
+            clinit.visitFieldInsn(GETSTATIC, "java/lang/Integer", "TYPE", "Ljava/lang/Class;");
+            clinit.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MethodType", "methodType", "(Ljava/lang/Class;Ljava/lang/Class;)Ljava/lang/invoke/MethodType;", false);
+            clinit.visitMethodInsn(INVOKEVIRTUAL, "java/lang/invoke/MethodHandles$Lookup", "findStatic",
+                            "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/MethodHandle;", false);
+            clinit.visitFieldInsn(PUTSTATIC, className, "INT_MH", "Ljava/lang/invoke/MethodHandle;");
+            clinit.visitInsn(RETURN);
+            clinit.visitMaxs(1, 1);
+            clinit.visitEnd();
+
+            MethodVisitor mainLink = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "mainLink", "(I)I", null, exceptions);
+            mainLink.visitCode();
+            mainLink.visitFieldInsn(GETSTATIC, className, "INT_MH", "Ljava/lang/invoke/MethodHandle;");
+            mainLink.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MHHelper", "internalMemberName", "(Ljava/lang/invoke/MethodHandle;)Ljava/lang/Object;", false);
+            mainLink.visitVarInsn(ASTORE, 1);
+            mainLink.visitVarInsn(ILOAD, 0);
+            mainLink.visitInsn(I2F);
+            mainLink.visitVarInsn(ALOAD, 1);
+            mainLink.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MHHelper", "linkToStatic", "(FLjava/lang/Object;)I", false);
+            mainLink.visitInsn(IRETURN);
+            mainLink.visitMaxs(1, 1);
+            mainLink.visitEnd();
+
+            MethodVisitor mainInvoke = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "mainInvoke", "(I)I", null, exceptions);
+            mainInvoke.visitCode();
+            mainInvoke.visitFieldInsn(GETSTATIC, className, "INT_MH", "Ljava/lang/invoke/MethodHandle;");
+            mainInvoke.visitVarInsn(ILOAD, 0);
+            mainInvoke.visitInsn(I2F);
+            mainInvoke.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MHHelper", "invokeBasicI", "(Ljava/lang/invoke/MethodHandle;F)I", false);
+            mainInvoke.visitInsn(IRETURN);
+            mainInvoke.visitMaxs(1, 1);
+            mainInvoke.visitEnd();
+
+            MethodVisitor bodyI = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "bodyI", "(I)I", null, null);
+            bodyI.visitCode();
+            bodyI.visitVarInsn(ILOAD, 0);
+            bodyI.visitIntInsn(SIPUSH, 1023);
+            bodyI.visitInsn(IAND);
+            bodyI.visitInsn(IRETURN);
+            bodyI.visitMaxs(1, 1);
+            bodyI.visitEnd();
+        }
+        cw.visitEnd();
+        return cw.toByteArray();
+    }
+}
--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/MethodHandleNode.java	Mon Jul 08 10:56:05 2019 +0200
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/MethodHandleNode.java	Mon Jul 08 04:01:54 2019 -0700
@@ -206,7 +206,8 @@
                     StampPair returnStamp, ValueNode[] arguments) {
         ValueNode methodHandleNode = getReceiver(arguments);
         if (methodHandleNode.isConstant()) {
-            return getTargetInvokeNode(adder, intrinsicMethod, bci, returnStamp, arguments, methodHandleAccess.resolveInvokeBasicTarget(methodHandleNode.asJavaConstant(), true), original);
+            return getTargetInvokeNode(adder, intrinsicMethod, methodHandleAccess, bci, returnStamp, arguments, methodHandleAccess.resolveInvokeBasicTarget(methodHandleNode.asJavaConstant(), true),
+                            original);
         }
         return null;
     }
@@ -227,7 +228,7 @@
                     StampPair returnStamp, ValueNode[] arguments) {
         ValueNode memberNameNode = getMemberName(arguments);
         if (memberNameNode.isConstant()) {
-            return getTargetInvokeNode(adder, intrinsicMethod, bci, returnStamp, arguments, methodHandleAccess.resolveLinkToTarget(memberNameNode.asJavaConstant()), original);
+            return getTargetInvokeNode(adder, intrinsicMethod, methodHandleAccess, bci, returnStamp, arguments, methodHandleAccess.resolveLinkToTarget(memberNameNode.asJavaConstant()), original);
         }
         return null;
     }
@@ -241,9 +242,10 @@
      *
      * @return invoke node for the member name target
      */
-    private static InvokeNode getTargetInvokeNode(GraphAdder adder, IntrinsicMethod intrinsicMethod, int bci, StampPair returnStamp, ValueNode[] originalArguments, ResolvedJavaMethod target,
+    private static InvokeNode getTargetInvokeNode(GraphAdder adder, IntrinsicMethod intrinsicMethod, MethodHandleAccessProvider methodHandleAccess, int bci, StampPair returnStamp,
+                    ValueNode[] originalArguments, ResolvedJavaMethod target,
                     ResolvedJavaMethod original) {
-        if (target == null) {
+        if (target == null || !isConsistentInfo(methodHandleAccess, original, target)) {
             return null;
         }
 
@@ -390,4 +392,84 @@
             return new InvokeNode(callTarget, bci);
         }
     }
+
+    /**
+     * Checks basic type consistency of low level method handle intrinsics.
+     *
+     * @param original declared method
+     * @param target resolved method
+     * @return true if original is type consistent with target
+     */
+    private static boolean isConsistentInfo(MethodHandleAccessProvider methodHandleAccess, ResolvedJavaMethod original, ResolvedJavaMethod target) {
+        IntrinsicMethod originalIntrinsicMethod = methodHandleAccess.lookupMethodHandleIntrinsic(original);
+        assert originalIntrinsicMethod == IntrinsicMethod.INVOKE_BASIC ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_STATIC ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_SPECIAL ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_VIRTUAL ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_INTERFACE;
+        IntrinsicMethod targetIntrinsicMethod = methodHandleAccess.lookupMethodHandleIntrinsic(target);
+        Signature originalSignature = original.getSignature();
+        Signature targetSignature = target.getSignature();
+
+        boolean invokeThroughMHIntrinsic = originalIntrinsicMethod != null && targetIntrinsicMethod == null;
+        if (!invokeThroughMHIntrinsic) {
+            return (original.getName().equals(target.getName())) && (originalSignature.equals(targetSignature));
+        }
+
+        // Linkers have appendix argument which is not passed to callee.
+        int hasAppendix = (originalIntrinsicMethod == IntrinsicMethod.LINK_TO_STATIC ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_SPECIAL ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_VIRTUAL ||
+                        originalIntrinsicMethod == IntrinsicMethod.LINK_TO_INTERFACE) ? 1 : 0;
+        if (originalSignature.getParameterCount(original.hasReceiver()) != (targetSignature.getParameterCount(target.hasReceiver()) + hasAppendix)) {
+            return false; // parameter count mismatch
+        }
+        int senderBase = 0;
+        int receiverBase = 0;
+        switch (originalIntrinsicMethod) {
+            case LINK_TO_VIRTUAL:
+            case LINK_TO_INTERFACE:
+            case LINK_TO_SPECIAL: {
+                if (target.isStatic()) {
+                    return false;
+                }
+                if (originalSignature.getParameterKind(0).isPrimitive()) {
+                    return false; // receiver should be an oop
+                }
+                senderBase = 1; // skip receiver
+                break;
+            }
+            case LINK_TO_STATIC: {
+                if (target.hasReceiver()) {
+                    return false;
+                }
+                break;
+            }
+            case INVOKE_BASIC: {
+                if (target.isStatic()) {
+                    if (targetSignature.getParameterKind(0).isPrimitive()) {
+                        return false; // receiver should be an oop
+                    }
+                    receiverBase = 1; // skip receiver
+                }
+                break;
+            }
+            default:
+                break;
+        }
+        assert (targetSignature.getParameterCount(false) - receiverBase) == (originalSignature.getParameterCount(false) - senderBase - hasAppendix) : "argument count mismatch";
+        int argCount = targetSignature.getParameterCount(false) - receiverBase;
+        for (int i = 0; i < argCount; i++) {
+            if (originalSignature.getParameterKind(senderBase + i).getStackKind() != targetSignature.getParameterKind(receiverBase + i).getStackKind()) {
+                return false;
+            }
+        }
+        // Only check the return type if the symbolic info has non-void return type.
+        // I.e. the return value of the resolved method can be dropped.
+        if (originalSignature.getReturnKind() != JavaKind.Void &&
+                        originalSignature.getReturnKind().getStackKind() != targetSignature.getReturnKind().getStackKind()) {
+            return false;
+        }
+        return true; // no mismatch found
+    }
 }
--- a/test/hotspot/jtreg/ProblemList-graal.txt	Mon Jul 08 10:56:05 2019 +0200
+++ b/test/hotspot/jtreg/ProblemList-graal.txt	Mon Jul 08 04:01:54 2019 -0700
@@ -38,7 +38,6 @@
 compiler/compilercontrol/mixed/RandomValidCommandsTest.java     8181753   generic-all
 compiler/compilercontrol/mixed/RandomCommandsTest.java          8181753   generic-all
 
-compiler/jsr292/InvokerSignatureMismatch.java                   8223807   generic-all
 
 compiler/graalunit/JttThreadsTest.java                          8207757   generic-all