8314249: Refactor handling of invokedynamic in JVMCI ConstantPool

Reviewed-by: dnsimon, coleenp

Author: iklam

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -906,9 +906,9 @@ C2V_VMENTRY_NULL(jobject, lookupKlassInPool, (JNIEnv* env, jobject, ARGUMENT_PAI
   return JVMCIENV->get_jobject(result);
 C2V_END
 
-C2V_VMENTRY_NULL(jobject, lookupAppendixInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index))
+C2V_VMENTRY_NULL(jobject, lookupAppendixInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint which))
   constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
-  oop appendix_oop = ConstantPool::appendix_at_if_loaded(cp, index);
+  oop appendix_oop = ConstantPool::appendix_at_if_loaded(cp, which);
   return JVMCIENV->get_jobject(JVMCIENV->get_object_constant(appendix_oop));
 C2V_END
 

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java

@@ -560,14 +560,17 @@ int constantPoolRemapInstructionOperandFromCache(HotSpotConstantPool constantPoo
     private native int constantPoolRemapInstructionOperandFromCache(HotSpotConstantPool constantPool, long constantPoolPointer, int cpci);
 
     /**
-     * Gets the appendix object (if any) associated with the entry at index {@code cpi} in
-     * {@code constantPool}.
+     * Gets the appendix object (if any) associated with the entry identified by {@code which}.
+     *
+     * @param which if negative, is treated as an encoded indy index for INVOKEDYNAMIC;
+     *              Otherwise, it's treated as a constant pool cache index (returned by HotSpotConstantPool::rawIndexToConstantPoolCacheIndex)
+     *              for INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}.
      */
-    HotSpotObjectConstantImpl lookupAppendixInPool(HotSpotConstantPool constantPool, int cpi) {
-        return lookupAppendixInPool(constantPool, constantPool.getConstantPoolPointer(), cpi);
+    HotSpotObjectConstantImpl lookupAppendixInPool(HotSpotConstantPool constantPool, int which) {
+        return lookupAppendixInPool(constantPool, constantPool.getConstantPoolPointer(), which);
     }
 
-    private native HotSpotObjectConstantImpl lookupAppendixInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi);
+    private native HotSpotObjectConstantImpl lookupAppendixInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int which);
 
     /**
      * Installs the result of a compilation into the code cache.

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java

@@ -47,6 +47,19 @@
 
 /**
  * Implementation of {@link ConstantPool} for HotSpot.
+ *
+ * The following convention is used in the jdk.vm.ci.hotspot package when accessing the ConstantPool with an index:
+ * <ul>
+ * <li>rawIndex - Index in the bytecode stream after the opcode (could be rewritten for some opcodes)</li>
+ * <li>cpi -      The constant pool index (as specified in JVM Spec)</li>
+ * <li>cpci -     The constant pool cache index, used only by the four bytecodes INVOKE{VIRTUAL,SPECIAL,STATIC,INTERFACE}.
+ *                It's the same as {@code rawIndex + HotSpotVMConfig::constantPoolCpCacheIndexTag}. </li>
+ * <li>which -    May be either a {@code rawIndex} or a {@code cpci}.</li>
+ * </ul>
+ *
+ * Note that {@code cpci} and {@code which} are used only in the HotSpot-specific implementation. They
+ * are not used by the public interface in jdk.vm.ci.meta.*.
+ * After JDK-8301993, all uses of {@code cpci} and {@code which} will be replaced with {@code rawIndex}.
  */
 public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleObject {
 
@@ -252,25 +265,15 @@ private HotSpotResolvedObjectType getHolder() {
      * @return constant pool cache index
      */
     private static int rawIndexToConstantPoolCacheIndex(int rawIndex, int opcode) {
-        int index;
-        if (opcode == Bytecodes.INVOKEDYNAMIC) {
-            index = rawIndex;
-            // See: ConstantPool::is_invokedynamic_index
-            if (index >= 0) {
-                throw new IllegalArgumentException("not an invokedynamic constant pool index " + index);
-            }
+        if (opcode == Bytecodes.INVOKEINTERFACE ||
+            opcode == Bytecodes.INVOKEVIRTUAL ||
+            opcode == Bytecodes.INVOKESPECIAL ||
+            opcode == Bytecodes.INVOKESTATIC) {
+            return rawIndex + config().constantPoolCpCacheIndexTag;
         } else {
-            if (opcode == Bytecodes.INVOKEINTERFACE ||
-                opcode == Bytecodes.INVOKEVIRTUAL ||
-                opcode == Bytecodes.INVOKESPECIAL ||
-                opcode == Bytecodes.INVOKESTATIC) {
-                index = rawIndex + config().constantPoolCpCacheIndexTag;
-            } else {
-                throw new IllegalArgumentException("unexpected opcode " + opcode);
-
-            }
+            // Only the above 4 bytecodes use ConstantPoolCacheIndex
+            throw new IllegalArgumentException("unexpected opcode " + opcode);
         }
-        return index;
     }
 
     /**
@@ -581,8 +584,8 @@ private static String argumentAsString(JavaConstant arg) {
     }
 
     @Override
-    public BootstrapMethodInvocation lookupBootstrapMethodInvocation(int rawCpi, int opcode) {
-        int cpi = opcode == -1 ? rawCpi : rawIndexToConstantPoolIndex(rawCpi, opcode);
+    public BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) {
+        int cpi = opcode == -1 ? index : indyIndexConstantPoolIndex(index, opcode);
         final JvmConstant tag = getTagAt(cpi);
         switch (tag.name) {
             case "InvokeDynamic":
@@ -685,13 +688,19 @@ public Signature lookupSignature(int cpi) {
     }
 
     @Override
-    public JavaConstant lookupAppendix(int cpi, int opcode) {
+    public JavaConstant lookupAppendix(int rawIndex, int opcode) {
         if (!Bytecodes.isInvoke(opcode)) {
-            throw new IllegalArgumentException("expected an invoke bytecode at " + cpi + ", got " + opcode);
+            throw new IllegalArgumentException("expected an invoke bytecode for " + rawIndex + ", got " + opcode);
         }
 
-        final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
-        return compilerToVM().lookupAppendixInPool(this, index);
+        if (opcode == Bytecodes.INVOKEDYNAMIC) {
+          if (!isInvokedynamicIndex(rawIndex)) {
+              throw new IllegalArgumentException("expected a raw index for INVOKEDYNAMIC but got " + rawIndex);
+          }
+          return compilerToVM().lookupAppendixInPool(this, rawIndex);
+        } else {
+          return compilerToVM().lookupAppendixInPool(this, rawIndexToConstantPoolCacheIndex(rawIndex, opcode));
+        }
     }
 
     /**
@@ -709,19 +718,19 @@ private static JavaType getJavaType(final Object type) {
     }
 
     @Override
-    public JavaMethod lookupMethod(int cpi, int opcode, ResolvedJavaMethod caller) {
-        final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
-        final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, index, (byte) opcode, (HotSpotResolvedJavaMethodImpl) caller);
+    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);
         if (method != null) {
             return method;
         } else {
             // Get the method's name and signature.
-            String name = getNameOf(index, opcode);
-            HotSpotSignature signature = new HotSpotSignature(runtime(), getSignatureOf(index, opcode));
+            String name = getNameOf(cpci, opcode);
+            HotSpotSignature signature = new HotSpotSignature(runtime(), getSignatureOf(cpci, opcode));
             if (opcode == Bytecodes.INVOKEDYNAMIC) {
                 return new UnresolvedJavaMethod(name, signature, runtime().getMethodHandleClass());
             } else {
-                final int klassIndex = getKlassRefIndexAt(index, opcode);
+                final int klassIndex = getKlassRefIndexAt(cpci, opcode);
                 final Object type = compilerToVM().lookupKlassInPool(this, klassIndex);
                 return new UnresolvedJavaMethod(name, signature, getJavaType(type));
             }
@@ -780,7 +789,6 @@ public JavaType lookupReferencedType(int rawIndex, int opcode) {
 
     @Override
     public JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode) {
-        final int cpi = compilerToVM().decodeFieldIndexToCPIndex(this, rawIndex);
         final int nameAndTypeIndex = getNameAndTypeRefIndexAt(rawIndex, opcode);
         final int typeIndex = getSignatureRefIndexAt(nameAndTypeIndex);
         String typeName = lookupUtf8(typeIndex);
@@ -813,32 +821,23 @@ public JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode
     }
 
     /**
-     * Converts a raw index from the bytecodes to a constant pool index (not a cache index).
+     * Converts a raw index for the INVOKEDYNAMIC bytecode to a constant pool index.
      *
      * @param rawIndex index from the bytecode
      *
-     * @param opcode bytecode to convert the index for
+     * @param opcode bytecode to convert the index for. Must be INVOKEDYNAMIC.
      *
      * @return constant pool index
      */
-    public int rawIndexToConstantPoolIndex(int rawIndex, int opcode) {
+    private int indyIndexConstantPoolIndex(int rawIndex, int opcode) {
         if (isInvokedynamicIndex(rawIndex)) {
             if (opcode != Bytecodes.INVOKEDYNAMIC) {
                 throw new IllegalArgumentException("expected INVOKEDYNAMIC at " + rawIndex + ", got " + opcode);
             }
             return compilerToVM().decodeIndyIndexToCPIndex(this, rawIndex, false);
+        } else {
+          throw new IllegalArgumentException("expected a raw index for INVOKEDYNAMIC but got " + rawIndex);
         }
-        if (opcode == Bytecodes.INVOKEDYNAMIC) {
-            throw new IllegalArgumentException("unexpected INVOKEDYNAMIC at " + rawIndex);
-        }
-        if (opcode == Bytecodes.GETSTATIC ||
-            opcode == Bytecodes.PUTSTATIC ||
-            opcode == Bytecodes.GETFIELD ||
-            opcode == Bytecodes.PUTFIELD) {
-            return compilerToVM().decodeFieldIndexToCPIndex(this, rawIndex);
-        }
-        int index = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
-        return compilerToVM().constantPoolRemapInstructionOperandFromCache(this, index);
     }
 
     @Override

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java

@@ -172,19 +172,21 @@ interface BootstrapMethodInvocation {
 
     /**
      * Gets the details for invoking a bootstrap method associated with the
-     * {@code CONSTANT_Dynamic_info} or {@code CONSTANT_InvokeDynamic_info} pool entry {@code cpi}
+     * {@code CONSTANT_Dynamic_info} or {@code CONSTANT_InvokeDynamic_info} pool entry
      * in the constant pool.
      *
-     * @param cpi a constant pool index
-     * @param opcode the opcode of the instruction that has {@code cpi} as an operand or -1 if
-     *            {@code cpi} was not decoded from an instruction stream
-     * @return the bootstrap method invocation details or {@code null} if the entry at {@code cpi}
+     * @param index if {@code opcode} is -1,  {@code index} is a constant pool index. Otherwise {@code opcode}
+     *              must be {@code Bytecodes.INVOKEDYNAMIC}, and {@code index} must be the operand of that
+     *              opcode in the bytecode stream (i.e., a {@code rawIndex}).
+     * @param opcode must be {@code Bytecodes.INVOKEDYNAMIC}, or -1 if
+     *            {@code index} was not decoded from a bytecode stream
+     * @return the bootstrap method invocation details or {@code null} if the entry specified by {@code index}
      *         is not a {@code CONSTANT_Dynamic_info} or @{code CONSTANT_InvokeDynamic_info}
      * @throws IllegalArgumentException if the bootstrap method invocation makes use of
      *             {@code java.lang.invoke.BootstrapCallInfo}
      * @jvms 4.7.23 The {@code BootstrapMethods} Attribute
      */
-    default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int cpi, int opcode) {
+    default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int index, int opcode) {
         throw new UnsupportedOperationException();
     }
 
@@ -242,10 +244,9 @@ default BootstrapMethodInvocation lookupBootstrapMethodInvocation(int cpi, int o
     /**
      * Looks up the appendix at the specified index.
      *
-     * @param cpi the constant pool index
-     * @param opcode the opcode of the instruction for which the lookup is being performed or
-     *            {@code -1}
+     * @param rawIndex index in the bytecode stream after the {@code opcode} (could be rewritten for some opcodes)
+     * @param opcode the opcode of the instruction for which the lookup is being performed
      * @return the appendix if it exists and is resolved or {@code null}
      */
-    JavaConstant lookupAppendix(int cpi, int opcode);
+    JavaConstant lookupAppendix(int rawIndex, int opcode);
 }

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestDynamicConstant.java

@@ -271,8 +271,8 @@ private static void assertNoEagerConstantResolution(Class<?> testClass, Constant
     private static void assertLookupBMIDoesNotInvokeBM(MetaAccessProvider metaAccess, Class<?> testClass) throws Exception {
         ResolvedJavaMethod shouldNotBeCalled = metaAccess.lookupJavaMethod(testClass.getDeclaredMethod("shouldNotBeCalled"));
         ConstantPool cp = shouldNotBeCalled.getConstantPool();
-        int cpi = getFirstInvokedynamicOperand(shouldNotBeCalled);
-        BootstrapMethodInvocation bmi = cp.lookupBootstrapMethodInvocation(cpi, INVOKEDYNAMIC);
+        int rawIndex = getFirstInvokedynamicOperand(shouldNotBeCalled);
+        BootstrapMethodInvocation bmi = cp.lookupBootstrapMethodInvocation(rawIndex, INVOKEDYNAMIC);
         Assert.assertEquals(bmi.getName(), "do_shouldNotBeCalled");
         Assert.assertEquals(bmi.getMethod().getName(), "shouldNotBeCalledBSM");
     }
@@ -408,8 +408,8 @@ private static int getFirstInvokedynamicOperand(ResolvedJavaMethod method) {
      * Ensures that loadReferencedType for an invokedynamic call site does not throw an exception.
      */
     private static void testLoadReferencedType(ResolvedJavaMethod method, ConstantPool cp) {
-        int cpi = getFirstInvokedynamicOperand(method);
-        cp.loadReferencedType(cpi, INVOKEDYNAMIC, false);
+        int rawIndex = getFirstInvokedynamicOperand(method);
+        cp.loadReferencedType(rawIndex, INVOKEDYNAMIC, false);
     }
 
     // @formatter:off

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java

@@ -37,9 +37,15 @@
  */
 package jdk.vm.ci.runtime.test;
 
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.invoke.MethodType;
+
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaField;
 import jdk.vm.ci.meta.JavaMethod;
 import jdk.vm.ci.meta.MetaAccessProvider;
@@ -82,9 +88,12 @@ static Object cloneObjectArray(Object[] arr) {
         return arr.clone();
     }
 
-    public static final int ALOAD_0 = 42; // 0x2A
-    public static final int GETSTATIC = 178; // 0xB2
-    public static final int INVOKEVIRTUAL = 182; // 0xB6
+    public static final int ICONST_0 = 3;
+    public static final int ALOAD_0 = 42;
+    public static final int ALOAD_1 = 43;
+    public static final int GETSTATIC = 178;
+    public static final int INVOKEVIRTUAL = 182;
+    public static final int INVOKEDYNAMIC = 186;
 
     public static int beU2(byte[] data, int bci) {
         return ((data[bci] & 0xff) << 8) | (data[bci + 1] & 0xff);
@@ -94,6 +103,10 @@ public static int beU1(byte[] data, int bci) {
         return data[bci] & 0xff;
     }
 
+    public static int beS4(byte[] data, int bci) {
+        return (data[bci] << 24) | ((data[bci + 1] & 0xff) << 16) | ((data[bci + 2] & 0xff) << 8) | (data[bci + 3] & 0xff);
+    }
+
     @Test
     public void lookupArrayCloneMethodTest() throws Exception {
         MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess();
@@ -138,4 +151,86 @@ public void lookupFieldTest() throws Exception {
         JavaField field =  m.getConstantPool().lookupField(rawIndex, m, GETSTATIC);
         Assert.assertEquals("someStaticField", field.getName(), "Wrong field name; rawIndex = " + rawIndex + ";");
     }
+
+    static String concatString1(String a, String b) {
+        return a + b;
+    }
+
+    static String concatString2(String a, String b) {
+        return a + b;
+    }
+
+    static void invokeHandle(MethodHandle mh) throws Throwable  {
+        mh.invokeExact(0);
+    }
+
+    static void intFunc(int t) {}
+
+    @Test
+    public void lookupAppendixTest() throws Throwable {
+        // We want at least two indy bytecodes -- with a single indy, the rawIndex is -1,
+        // or 0xffffffff. Even if we load it with the wrong endianness, it will still come
+        // "correctly" out as -1.
+        concatString1("aaa", "bbb"); // force the indy to be resolved
+        concatString2("aaa", "bbb"); // force the indy to be resolved
+
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
+        MethodType mt = MethodType.methodType(void.class, int.class);
+        MethodHandle mh = lookup.findStatic(ConstantPoolTest.class, "intFunc", mt);
+        invokeHandle(mh);
+
+        lookupAppendixTest_dynamic("concatString1");
+        lookupAppendixTest_dynamic("concatString2");
+        lookupAppendixTest_virtual();
+    }
+
+    public 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;");
+        ResolvedJavaMethod m = type.findMethod(methodName, 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);
+        JavaConstant constant = m.getConstantPool().lookupAppendix(rawIndex, INVOKEDYNAMIC);
+        Assert.assertTrue(constant.toString().startsWith("Object["), "wrong appendix: " + constant);
+    }
+
+    public 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");
+        ResolvedJavaMethod m = type.findMethod("invokeHandle", methodSig);
+        Assert.assertNotNull(m);
+
+        // Expected
+        // aload_0
+        // iconst_0
+        // invokevirtual #rawIndex // Method java/lang/invoke/MethodHandle.invokeExact:(I)V
+        byte[] bytecode = m.getCode();
+        Assert.assertNotNull(bytecode);
+        Assert.assertEquals(6, bytecode.length);
+        Assert.assertEquals(ALOAD_0, beU1(bytecode, 0));
+        Assert.assertEquals(ICONST_0, beU1(bytecode, 1));
+        Assert.assertEquals(INVOKEVIRTUAL, beU1(bytecode, 2));
+
+        int rawIndex = beU2(bytecode, 3);
+        //System.out.println("rawIndex = " + rawIndex);
+        JavaConstant constant = m.getConstantPool().lookupAppendix(rawIndex, INVOKEVIRTUAL);
+        //System.out.println("constant = " + constant);
+        Assert.assertTrue(constant.toString().startsWith("Object["), "wrong appendix: " + constant);
+    }
 }