# HG changeset patch # User jcm # Date 1562583714 25200 # Node ID 3595fb8794c59cb72ffd54eed895f25089206ddc # Parent 978b2201984c9fb2cb061ea6c5a1825d9d830a0c 8221577: [Graal] Implement basic type consistency checks for Low level MH intrinsics Reviewed-by: kvn diff -r 978b2201984c -r 3595fb8794c5 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/CustomizedBytecodePatternTest.java --- 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; diff -r 978b2201984c -r 3595fb8794c5 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/FrameStateBuilder.java --- 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; diff -r 978b2201984c -r 3595fb8794c5 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements.test/src/org/graalvm/compiler/replacements/test/InvokerSignatureMismatchTest.java --- /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 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, "", "()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(); + } +} diff -r 978b2201984c -r 3595fb8794c5 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/nodes/MethodHandleNode.java --- 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 + } } diff -r 978b2201984c -r 3595fb8794c5 test/hotspot/jtreg/ProblemList-graal.txt --- 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