From 0c865a75e658617d40dfa9eb8cf44ccdcea928d9 Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Thu, 7 Sep 2023 18:30:09 +0000 Subject: [PATCH] 8315637: JDK-8314249 broke libgraal Reviewed-by: dnsimon, matsaave --- .../jdk/vm/ci/hotspot/CompilerToVM.java | 41 ++++---- .../vm/ci/hotspot/HotSpotConstantPool.java | 48 ++++------ .../vm/ci/runtime/test/ConstantPoolTest.java | 96 ++++++++++++++++++- 3 files changed, 135 insertions(+), 50 deletions(-) diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java index f50207ac13a..0ca3c731bcf 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java @@ -336,48 +336,55 @@ final class CompilerToVM { /** * Gets the name of the {@code JVM_CONSTANT_NameAndType} entry in {@code constantPool} - * referenced by the {@code rawIndex}. The meaning of {@code rawIndex} is dependent - * on the given {@opcode}. + * referenced by the {@code which}. * * The behavior of this method is undefined if the class holding the {@code constantPool} - * has not yet been rewritten, or {@code rawIndex} is not a valid index for + * has not yet been rewritten, or {@code which} is not a valid index for * this class for the given {@code opcode} + * + * @param which for INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}, must be {@code cpci}. For all other bytecodes, + * must be {@code rawIndex} */ - String lookupNameInPool(HotSpotConstantPool constantPool, int rawIndex, int opcode) { - return lookupNameInPool(constantPool, constantPool.getConstantPoolPointer(), rawIndex, opcode); + String lookupNameInPool(HotSpotConstantPool constantPool, int which, int opcode) { + return lookupNameInPool(constantPool, constantPool.getConstantPoolPointer(), which, opcode); } - private native String lookupNameInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int rawIndex, int opcode); + private native String lookupNameInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int which, int opcode); /** * Gets the signature of the {@code JVM_CONSTANT_NameAndType} entry in {@code constantPool} - * referenced by the {@code rawIndex}. The meaning of {@code rawIndex} is dependent - * on the given {@opcode}. + * referenced by the {@code which}. * * The behavior of this method is undefined if the class holding the {@code constantPool} - * has not yet been rewritten, or {@code rawIndex} is not a valid index for + * has not yet been rewritten, or {@code which} is not a valid index for * this class for the given {@code opcode} + * + * @param which for INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}, must be {@code cpci}. For all other bytecodes, + * must be {@code rawIndex} */ - String lookupSignatureInPool(HotSpotConstantPool constantPool, int rawIndex, int opcode) { - return lookupSignatureInPool(constantPool, constantPool.getConstantPoolPointer(), rawIndex, opcode); + String lookupSignatureInPool(HotSpotConstantPool constantPool, int which, int opcode) { + return lookupSignatureInPool(constantPool, constantPool.getConstantPoolPointer(), which, opcode); } - private native String lookupSignatureInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int rawIndex, int opcode); + private native String lookupSignatureInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int which, int opcode); /** * Gets the {@code JVM_CONSTANT_Class} index from the entry in {@code constantPool} - * referenced by the {@code rawIndex}. The meaning of {@code rawIndex} is dependent + * referenced by the {@code which}. The meaning of {@code which} is dependent * on the given {@opcode}. * * The behavior of this method is undefined if the class holding the {@code constantPool} - * has not yet been rewritten, or {@code rawIndex} is not a valid index for + * has not yet been rewritten, or {@code which} is not a valid index for * this class for the given {@code opcode} + * + * @param which for INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}, must be {@code cpci}. For all other bytecodes, + * must be {@code rawIndex} */ - int lookupKlassRefIndexInPool(HotSpotConstantPool constantPool, int rawIndex, int opcode) { - return lookupKlassRefIndexInPool(constantPool, constantPool.getConstantPoolPointer(), rawIndex, opcode); + int lookupKlassRefIndexInPool(HotSpotConstantPool constantPool, int which, int opcode) { + return lookupKlassRefIndexInPool(constantPool, constantPool.getConstantPoolPointer(), which, opcode); } - private native int lookupKlassRefIndexInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int rawIndex, int opcode); + private native int lookupKlassRefIndexInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int which, int opcode); /** * Looks up a class denoted by the {@code JVM_CONSTANT_Class} entry at index {@code cpi} in diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java index d4bb97f03c4..51917d3fb7f 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java @@ -396,18 +396,6 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO return compilerToVM().lookupNameAndTypeRefIndexInPool(this, rawIndex, opcode); } - /** - * Gets the name of a {@code JVM_CONSTANT_NameAndType} constant pool entry referenced by another - * entry denoted by {@code which}. - * - * @param rawIndex rewritten index in the bytecode stream - * @param opcode the opcode of the instruction for which the lookup is being performed - * @return name as {@link String} - */ - private String getNameOf(int rawIndex, int opcode) { - return compilerToVM().lookupNameInPool(this, rawIndex, opcode); - } - /** * Gets the name reference index of a {@code JVM_CONSTANT_NameAndType} constant pool entry at * index {@code index}. @@ -421,17 +409,6 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO return refIndex & 0xFFFF; } - /** - * Gets the signature of a {@code JVM_CONSTANT_NameAndType} constant pool entry referenced by - * another entry denoted by {@code which}. - * - * @param rawIndex rewritten index in the bytecode stream - * @param opcode the opcode of the instruction for which the lookup is being performed - * @return signature as {@link String} - */ - private String getSignatureOf(int rawIndex, int opcode) { - return compilerToVM().lookupSignatureInPool(this, rawIndex, opcode); - } /** * Gets the signature reference index of a {@code JVM_CONSTANT_NameAndType} constant pool entry @@ -449,12 +426,13 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO /** * Gets the klass reference index constant pool entry at index {@code index}. * - * @param rawIndex rewritten index in the bytecode stream + * @param which for INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}, must be {@code cpci}. For all other bytecodes, + * must be {@code rawIndex} * @param opcode the opcode of the instruction for which the lookup is being performed * @return klass reference index */ - private int getKlassRefIndexAt(int rawIndex, int opcode) { - return compilerToVM().lookupKlassRefIndexInPool(this, rawIndex, opcode); + private int getKlassRefIndexAt(int which, int opcode) { + return compilerToVM().lookupKlassRefIndexInPool(this, which, opcode); } /** @@ -719,18 +697,26 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO @Override public JavaMethod lookupMethod(int rawIndex, int opcode, ResolvedJavaMethod caller) { - final int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode); - final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, cpci, (byte) opcode, (HotSpotResolvedJavaMethodImpl) caller); + int which; // interpretation depends on opcode + if (opcode == Bytecodes.INVOKEDYNAMIC) { + if (!isInvokedynamicIndex(rawIndex)) { + throw new IllegalArgumentException("expected a raw index for INVOKEDYNAMIC but got " + rawIndex); + } + which = rawIndex; + } else { + which = rawIndexToConstantPoolCacheIndex(rawIndex, opcode); + } + final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, which, (byte) opcode, (HotSpotResolvedJavaMethodImpl) caller); if (method != null) { return method; } else { // Get the method's name and signature. - String name = getNameOf(cpci, opcode); - HotSpotSignature signature = new HotSpotSignature(runtime(), getSignatureOf(cpci, opcode)); + String name = compilerToVM().lookupNameInPool(this, which, opcode); + HotSpotSignature signature = new HotSpotSignature(runtime(), compilerToVM().lookupSignatureInPool(this, which, opcode)); if (opcode == Bytecodes.INVOKEDYNAMIC) { return new UnresolvedJavaMethod(name, signature, runtime().getMethodHandleClass()); } else { - final int klassIndex = getKlassRefIndexAt(cpci, opcode); + final int klassIndex = getKlassRefIndexAt(which, opcode); final Object type = compilerToVM().lookupKlassInPool(this, klassIndex); return new UnresolvedJavaMethod(name, signature, getJavaType(type)); } diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java index 26fcedb64d4..b811d8753cb 100644 --- a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java @@ -160,6 +160,11 @@ public class ConstantPoolTest { return a + b; } + // We never call this method, so its indy is never resolved. + static String concatString3_never_call(String a, String b) { + return a + b; + } + static void invokeHandle(MethodHandle mh) throws Throwable { mh.invokeExact(0); } @@ -184,7 +189,7 @@ public class ConstantPoolTest { lookupAppendixTest_virtual(); } - public void lookupAppendixTest_dynamic(String methodName) throws Exception { + private void lookupAppendixTest_dynamic(String methodName) throws Exception { MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess(); ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class); Signature methodSig = metaAccess.parseMethodDescriptor("(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"); @@ -209,7 +214,7 @@ public class ConstantPoolTest { Assert.assertTrue(constant.toString().startsWith("Object["), "wrong appendix: " + constant); } - public void lookupAppendixTest_virtual() throws Exception { + private void lookupAppendixTest_virtual() throws Exception { MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess(); ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class); Signature methodSig = metaAccess.parseMethodDescriptor("(Ljava/lang/invoke/MethodHandle;)V"); @@ -233,4 +238,91 @@ public class ConstantPoolTest { //System.out.println("constant = " + constant); Assert.assertTrue(constant.toString().startsWith("Object["), "wrong appendix: " + constant); } + + static void invokeVirtual(Object o) { + o.hashCode(); + } + + @Test + public void lookupMethodTest_dynamic() throws Exception { + concatString1("aaa", "bbb"); // force the indy to be resolved + + MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess(); + ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class); + Signature methodSig = metaAccess.parseMethodDescriptor("(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"); + ResolvedJavaMethod m = type.findMethod("concatString1", methodSig); + Assert.assertNotNull(m); + + // Expected: + // aload_0; + // aload_1; + // invokedynamic ...StringConcatFactory.makeConcatWithConstants... + byte[] bytecode = m.getCode(); + Assert.assertNotNull(bytecode); + Assert.assertEquals(8, bytecode.length); + Assert.assertEquals(ALOAD_0, beU1(bytecode, 0)); + Assert.assertEquals(ALOAD_1, beU1(bytecode, 1)); + Assert.assertEquals(INVOKEDYNAMIC, beU1(bytecode, 2)); + + // Note: internally HotSpot stores the indy index as a native int32, but m.getCode() byte-swaps all such + // indices so they appear to be big-endian. + int rawIndex = beS4(bytecode, 3); + System.out.println("rawIndex = " + rawIndex); + JavaMethod callee = m.getConstantPool().lookupMethod(rawIndex, INVOKEDYNAMIC, /*caller=*/m); + System.out.println("callee = " + callee); + Assert.assertTrue(callee.toString().equals("HotSpotMethod"), + "wrong method: " + callee); + } + + @Test + public void lookupMethodTest_dynamic_unresolved() throws Exception { + MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess(); + ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class); + Signature methodSig = metaAccess.parseMethodDescriptor("(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"); + ResolvedJavaMethod m = type.findMethod("concatString3_never_call", methodSig); + Assert.assertNotNull(m); + + // Expected: + // aload_0; + // aload_1; + // invokedynamic ...StringConcatFactory.makeConcatWithConstants... + byte[] bytecode = m.getCode(); + Assert.assertNotNull(bytecode); + Assert.assertEquals(8, bytecode.length); + Assert.assertEquals(ALOAD_0, beU1(bytecode, 0)); + Assert.assertEquals(ALOAD_1, beU1(bytecode, 1)); + Assert.assertEquals(INVOKEDYNAMIC, beU1(bytecode, 2)); + + // Note: internally HotSpot stores the indy index as a native int32, but m.getCode() byte-swaps all such + // indices so they appear to be big-endian. + int rawIndex = beS4(bytecode, 3); + System.out.println("rawIndex = " + rawIndex); + JavaMethod callee = m.getConstantPool().lookupMethod(rawIndex, INVOKEDYNAMIC, /*caller=*/m); + System.out.println("callee = " + callee); + Assert.assertTrue(callee.toString().startsWith("jdk.vm.ci.meta.UnresolvedJavaMethod"), + "wrong method: " + callee); + } + + @Test + public void lookupMethodTest_virtual() throws Exception { + MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess(); + ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class); + Signature methodSig = metaAccess.parseMethodDescriptor("(Ljava/lang/Object;)V"); + ResolvedJavaMethod m = type.findMethod("invokeVirtual", methodSig); + Assert.assertNotNull(m); + + // Expected + // 0: aload_0 + // 1: invokevirtual #rawIndex // Method Method java/lang/Object.hashCode:()I + byte[] bytecode = m.getCode(); + Assert.assertNotNull(bytecode); + Assert.assertEquals(ALOAD_0, beU1(bytecode, 0)); + Assert.assertEquals(INVOKEVIRTUAL, beU1(bytecode, 1)); + int rawIndex = beU2(bytecode, 2); + System.out.println("rawIndex = " + rawIndex); + JavaMethod callee = m.getConstantPool().lookupMethod(rawIndex, INVOKEVIRTUAL, /*caller=*/m); + System.out.println("callee = " + callee); + Assert.assertTrue(callee.toString().equals("HotSpotMethod"), + "wrong method: " + callee); + } }