8314078: HotSpotConstantPool.lookupField() asserts due to field changes in ConstantPool.cpp

Reviewed-by: dnsimon, coleenp

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -1620,6 +1620,14 @@ C2V_VMENTRY_0(int, decodeIndyIndexToCPIndex, (JNIEnv* env, jobject, ARGUMENT_PAI
   return cp->resolved_indy_entry_at(indy_index)->constant_pool_index();
 C2V_END
 
+C2V_VMENTRY_0(int, decodeFieldIndexToCPIndex, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint field_index))
+  constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
+  if (field_index < 0 || field_index >= cp->resolved_field_entries_length()) {
+    JVMCI_THROW_MSG_0(IllegalStateException, err_msg("invalid field index %d", field_index));
+  }
+  return cp->resolved_field_entry_at(field_index)->constant_pool_index();
+C2V_END
+
 C2V_VMENTRY(void, resolveInvokeHandleInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index))
   constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
   Klass* holder = cp->klass_ref_at(index, Bytecodes::_invokehandle, CHECK);
@@ -3143,6 +3151,7 @@ JNINativeMethod CompilerToVM::methods[] = {
   {CC "getUncachedStringInPool",                      CC "(" HS_CONSTANT_POOL2 "I)" JAVACONSTANT,                                           FN_PTR(getUncachedStringInPool)},
   {CC "resolveTypeInPool",                            CC "(" HS_CONSTANT_POOL2 "I)" HS_KLASS,                                               FN_PTR(resolveTypeInPool)},
   {CC "resolveFieldInPool",                           CC "(" HS_CONSTANT_POOL2 "I" HS_METHOD2 "B[I)" HS_KLASS,                              FN_PTR(resolveFieldInPool)},
+  {CC "decodeFieldIndexToCPIndex",                    CC "(" HS_CONSTANT_POOL2 "I)I",                                                       FN_PTR(decodeFieldIndexToCPIndex)},
   {CC "decodeIndyIndexToCPIndex",                     CC "(" HS_CONSTANT_POOL2 "IZ)I",                                                      FN_PTR(decodeIndyIndexToCPIndex)},
   {CC "resolveInvokeHandleInPool",                    CC "(" HS_CONSTANT_POOL2 "I)V",                                                       FN_PTR(resolveInvokeHandleInPool)},
   {CC "isResolvedInvokeHandleInPool",                 CC "(" HS_CONSTANT_POOL2 "I)I",                                                       FN_PTR(isResolvedInvokeHandleInPool)},

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

@@ -435,6 +435,19 @@ final class CompilerToVM {
 
     private native int decodeIndyIndexToCPIndex(HotSpotConstantPool constantPool, long constantPoolPointer, int encoded_indy_index, boolean resolve);
 
+    /**
+     * Converts the {@code rawIndex} operand of a rewritten getfield/putfield/getstatic/putstatic instruction
+     * to an index directly into {@code constantPool}.
+     *
+     * @throws IllegalArgumentException if {@code rawIndex} is out of range.
+     * @return {@code JVM_CONSTANT_FieldRef} constant pool entry index for the invokedynamic
+     */
+    int decodeFieldIndexToCPIndex(HotSpotConstantPool constantPool, int rawIndex) {
+        return decodeFieldIndexToCPIndex(constantPool, constantPool.getConstantPoolPointer(), rawIndex);
+    }
+
+    private native int decodeFieldIndexToCPIndex(HotSpotConstantPool constantPool, long constantPoolPointer, int rawIndex);
+
     /**
      * Resolves the details for invoking the bootstrap method associated with the
      * {@code CONSTANT_Dynamic_info} or @{code CONSTANT_InvokeDynamic_info} entry at {@code cpi} in
@@ -507,8 +520,8 @@ final class CompilerToVM {
     private native HotSpotResolvedObjectTypeImpl resolveTypeInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi) throws LinkageError;
 
     /**
-     * Looks up and attempts to resolve the {@code JVM_CONSTANT_Field} entry for at index
+     * Looks up and attempts to resolve the {@code JVM_CONSTANT_Field} entry denoted by
-     * {@code cpi} in {@code constantPool}. For some opcodes, checks are performed that require the
+     * {@code rawIndex}. For some opcodes, checks are performed that require the
      * {@code method} that contains {@code opcode} to be specified. The values returned in
      * {@code info} are:
      *
@@ -520,19 +533,18 @@ final class CompilerToVM {
      *     ]
      * </pre>
      *
-     * The behavior of this method is undefined if {@code cpi} does not denote a
+     * The behavior of this method is undefined if {@code rawIndex} is invalid.
-     * {@code JVM_CONSTANT_Field} entry.
      *
      * @param info an array in which the details of the field are returned
      * @return the type defining the field if resolution is successful, null otherwise
      */
-    HotSpotResolvedObjectTypeImpl resolveFieldInPool(HotSpotConstantPool constantPool, int cpi, HotSpotResolvedJavaMethodImpl method, byte opcode, int[] info) {
+    HotSpotResolvedObjectTypeImpl resolveFieldInPool(HotSpotConstantPool constantPool, int rawIndex, HotSpotResolvedJavaMethodImpl method, byte opcode, int[] info) {
         long methodPointer = method != null ? method.getMethodPointer() : 0L;
-        return resolveFieldInPool(constantPool, constantPool.getConstantPoolPointer(), cpi, method, methodPointer, opcode, info);
+        return resolveFieldInPool(constantPool, constantPool.getConstantPoolPointer(), rawIndex, method, methodPointer, opcode, info);
     }
 
     private native HotSpotResolvedObjectTypeImpl resolveFieldInPool(HotSpotConstantPool constantPool, long constantPoolPointer,
-                    int cpi, HotSpotResolvedJavaMethodImpl method, long methodPointer, byte opcode, int[] info);
+                    int rawIndex, HotSpotResolvedJavaMethodImpl method, long methodPointer, byte opcode, int[] info);
 
     /**
      * Converts {@code cpci} from an index into the cache for {@code constantPool} to an index

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

@@ -260,14 +260,10 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
                 throw new IllegalArgumentException("not an invokedynamic constant pool index " + index);
             }
         } else {
-            if (opcode == Bytecodes.GETFIELD ||
+            if (opcode == Bytecodes.INVOKEINTERFACE ||
-                            opcode == Bytecodes.PUTFIELD ||
+                opcode == Bytecodes.INVOKEVIRTUAL ||
-                            opcode == Bytecodes.GETSTATIC ||
+                opcode == Bytecodes.INVOKESPECIAL ||
-                            opcode == Bytecodes.PUTSTATIC ||
+                opcode == Bytecodes.INVOKESTATIC) {
-                            opcode == Bytecodes.INVOKEINTERFACE ||
-                            opcode == Bytecodes.INVOKEVIRTUAL ||
-                            opcode == Bytecodes.INVOKESPECIAL ||
-                            opcode == Bytecodes.INVOKESTATIC) {
                 index = rawIndex + config().constantPoolCpCacheIndexTag;
             } else {
                 throw new IllegalArgumentException("unexpected opcode " + opcode);
@@ -748,8 +744,8 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
     }
 
     @Override
-    public JavaType lookupReferencedType(int cpi, int opcode) {
+    public JavaType lookupReferencedType(int rawIndex, int opcode) {
-        int index;
+        int cpi;
         switch (opcode) {
             case Bytecodes.CHECKCAST:
             case Bytecodes.INSTANCEOF:
@@ -759,43 +755,45 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
             case Bytecodes.LDC:
             case Bytecodes.LDC_W:
             case Bytecodes.LDC2_W:
-                index = cpi;
+                cpi = rawIndex;
                 break;
             case Bytecodes.GETSTATIC:
             case Bytecodes.PUTSTATIC:
             case Bytecodes.GETFIELD:
             case Bytecodes.PUTFIELD:
+                cpi = getKlassRefIndexAt(rawIndex, opcode);
+                break;
             case Bytecodes.INVOKEVIRTUAL:
             case Bytecodes.INVOKESPECIAL:
             case Bytecodes.INVOKESTATIC:
             case Bytecodes.INVOKEINTERFACE: {
-                index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
+                int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
-                index = getKlassRefIndexAt(index, opcode);
+                cpi = getKlassRefIndexAt(cpci, opcode);
                 break;
             }
             default:
                 throw JVMCIError.shouldNotReachHere("Unexpected opcode " + opcode);
         }
-        final Object type = compilerToVM().lookupKlassInPool(this, index);
+        final Object type = compilerToVM().lookupKlassInPool(this, cpi);
         return getJavaType(type);
     }
 
     @Override
-    public JavaField lookupField(int cpi, ResolvedJavaMethod method, int opcode) {
+    public JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode) {
-        final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
+        final int cpi = compilerToVM().decodeFieldIndexToCPIndex(this, rawIndex);
-        final int nameAndTypeIndex = getNameAndTypeRefIndexAt(index, opcode);
+        final int nameAndTypeIndex = getNameAndTypeRefIndexAt(rawIndex, opcode);
         final int typeIndex = getSignatureRefIndexAt(nameAndTypeIndex);
         String typeName = lookupUtf8(typeIndex);
         JavaType type = runtime().lookupType(typeName, getHolder(), false);
 
-        final int holderIndex = getKlassRefIndexAt(index, opcode);
+        final int holderIndex = getKlassRefIndexAt(rawIndex, opcode);
         JavaType fieldHolder = lookupType(holderIndex, opcode);
 
         if (fieldHolder instanceof HotSpotResolvedObjectTypeImpl) {
             int[] info = new int[4];
             HotSpotResolvedObjectTypeImpl resolvedHolder;
             try {
-                resolvedHolder = compilerToVM().resolveFieldInPool(this, index, (HotSpotResolvedJavaMethodImpl) method, (byte) opcode, info);
+                resolvedHolder = compilerToVM().resolveFieldInPool(this, rawIndex, (HotSpotResolvedJavaMethodImpl) method, (byte) opcode, info);
             } catch (Throwable t) {
                 /*
                  * If there was an exception resolving the field we give up and return an unresolved
@@ -833,19 +831,25 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
         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
-    public void loadReferencedType(int cpi, int opcode) {
+    public void loadReferencedType(int rawIndex, int opcode) {
-        loadReferencedType(cpi, opcode, true /* initialize */);
+        loadReferencedType(rawIndex, opcode, true /* initialize */);
     }
 
     @Override
     @SuppressWarnings("fallthrough")
-    public void loadReferencedType(int cpi, int opcode, boolean initialize) {
+    public void loadReferencedType(int rawIndex, int opcode, boolean initialize) {
-        int index;
+        int cpi;
         switch (opcode) {
             case Bytecodes.CHECKCAST:
             case Bytecodes.INSTANCEOF:
@@ -855,57 +859,59 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
             case Bytecodes.LDC:
             case Bytecodes.LDC_W:
             case Bytecodes.LDC2_W:
-                index = cpi;
+                cpi = rawIndex;
                 break;
             case Bytecodes.INVOKEDYNAMIC: {
                 // invokedynamic indices are different from constant pool cache indices
-                if (!isInvokedynamicIndex(cpi)) {
+                if (!isInvokedynamicIndex(rawIndex)) {
-                    throw new IllegalArgumentException("must use invokedynamic index but got " + cpi);
+                    throw new IllegalArgumentException("must use invokedynamic index but got " + rawIndex);
                 }
-                index = compilerToVM().decodeIndyIndexToCPIndex(this, cpi, true);
+                cpi = compilerToVM().decodeIndyIndexToCPIndex(this, rawIndex, true);
                 break;
             }
             case Bytecodes.GETSTATIC:
             case Bytecodes.PUTSTATIC:
             case Bytecodes.GETFIELD:
             case Bytecodes.PUTFIELD:
+                cpi = compilerToVM().decodeFieldIndexToCPIndex(this, rawIndex);
+                break;
             case Bytecodes.INVOKEVIRTUAL:
             case Bytecodes.INVOKESPECIAL:
             case Bytecodes.INVOKESTATIC:
             case Bytecodes.INVOKEINTERFACE: {
                 // invoke and field instructions point to a constant pool cache entry.
-                index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
+                int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
-                index = compilerToVM().constantPoolRemapInstructionOperandFromCache(this, index);
+                cpi = compilerToVM().constantPoolRemapInstructionOperandFromCache(this, cpci);
                 break;
             }
             default:
                 throw JVMCIError.shouldNotReachHere("Unexpected opcode " + opcode);
         }
 
-        final JvmConstant tag = getTagAt(index);
+        final JvmConstant tag = getTagAt(cpi);
         if (tag == null) {
-            assert getTagAt(index - 1) == constants.jvmDouble || getTagAt(index - 1) == constants.jvmLong;
+            assert getTagAt(cpi - 1) == constants.jvmDouble || getTagAt(cpi - 1) == constants.jvmLong;
             return;
         }
         switch (tag.name) {
             case "Methodref":
             case "Fieldref":
             case "InterfaceMethodref":
-                index = getUncachedKlassRefIndexAt(index);
+                cpi = getUncachedKlassRefIndexAt(cpi);
                 // Read the tag only once because it could change between multiple reads.
-                final JvmConstant klassTag = getTagAt(index);
+                final JvmConstant klassTag = getTagAt(cpi);
                 assert klassTag == constants.jvmClass || klassTag == constants.jvmUnresolvedClass || klassTag == constants.jvmUnresolvedClassInError : klassTag;
                 // fall through
             case "Class":
             case "UnresolvedClass":
             case "UnresolvedClassInError":
-                final HotSpotResolvedObjectTypeImpl type = compilerToVM().resolveTypeInPool(this, index);
+                final HotSpotResolvedObjectTypeImpl type = compilerToVM().resolveTypeInPool(this, cpi);
                 if (initialize && !type.isPrimitive() && !type.isArray()) {
                     type.ensureInitialized();
                 }
                 if (tag == constants.jvmMethodref) {
                     if (Bytecodes.isInvokeHandleAlias(opcode) && isSignaturePolymorphicHolder(type)) {
-                        final int methodRefCacheIndex = rawIndexToConstantPoolCacheIndex(cpi, opcode);
+                        final int methodRefCacheIndex = rawIndexToConstantPoolCacheIndex(rawIndex, opcode);
                         checkTag(compilerToVM().constantPoolRemapInstructionOperandFromCache(this, methodRefCacheIndex), constants.jvmMethodref);
                         compilerToVM().resolveInvokeHandleInPool(this, methodRefCacheIndex);
                     }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2023, 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
@@ -29,6 +29,14 @@ import java.util.List;
  * Represents the runtime representation of the constant pool that is used by the compiler when
  * parsing bytecode. Provides methods to look up a constant pool entry without performing
  * resolution. They are used during compilation.
+ *
+ * The following convention is used 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>
+ * </ul>
+ *
+ * Some of the methods are currently not using the convention correctly. That will be addressed in JDK-8314172.
  */
 public interface ConstantPool {
 
@@ -44,53 +52,50 @@ public interface ConstantPool {
      * initialized. This can be used to compile time resolve a type. It works for field, method, or
      * type constant pool entries.
      *
-     * @param cpi the index of the constant pool entry that references the type
+     * @param rawIndex index in the bytecode stream after the {@code opcode} (could be rewritten for some opcodes)
      * @param opcode the opcode of the instruction that references the type
      */
-    void loadReferencedType(int cpi, int opcode);
+    void loadReferencedType(int rawIndex, int opcode);
 
     /**
      * Ensures that the type referenced by the specified constant pool entry is loaded. This can be
      * used to compile time resolve a type. It works for field, method, or type constant pool
      * entries.
      *
-     * @param cpi the index of the constant pool entry that references the type
+     * @param rawIndex index in the bytecode stream after the {@code opcode} (could be rewritten for some opcodes)
      * @param opcode the opcode of the instruction that references the type
      * @param initialize if {@code true}, the referenced type is either guaranteed to be initialized
      *            upon return or an initialization exception is thrown
      */
-    default void loadReferencedType(int cpi, int opcode, boolean initialize) {
+    default void loadReferencedType(int rawIndex, int opcode, boolean initialize) {
         if (initialize) {
-            loadReferencedType(cpi, opcode);
+            loadReferencedType(rawIndex, opcode);
         } else {
             throw new UnsupportedOperationException();
         }
     }
 
     /**
-     * Looks up the type referenced by the constant pool entry at {@code cpi} as referenced by the
+     * Looks up the type referenced by the {@code rawIndex}.
-     * {@code opcode} bytecode instruction.
      *
-     * @param cpi the index of a constant pool entry that references a type
+     * @param rawIndex index in the bytecode stream after the {@code opcode} (could be rewritten for some opcodes)
-     * @param opcode the opcode of the instruction with {@code cpi} as an operand
+     * @param opcode the opcode of the instruction with {@code rawIndex} as an operand
      * @return a reference to the compiler interface type
      */
-    JavaType lookupReferencedType(int cpi, int opcode);
+    JavaType lookupReferencedType(int rawIndex, int opcode);
 
     /**
-     * Looks up a reference to a field. If {@code opcode} is non-negative, then resolution checks
+     * Looks up a reference to a field. Resolution checks
      * specific to the bytecode it denotes are performed if the field is already resolved. Checks
      * for some bytecodes require the method that contains the bytecode to be specified. Should any
      * of these checks fail, an unresolved field reference is returned.
      *
-     * @param cpi the constant pool index
+     * @param rawIndex rewritten index in the bytecode stream after the {@code opcode}
-     * @param opcode the opcode of the instruction for which the lookup is being performed or
+     * @param opcode the opcode of the instruction for which the lookup is being performed
-     *            {@code -1}
      * @param method the method for which the lookup is being performed
-     * @return a reference to the field at {@code cpi} in this pool
+     * @return a reference to the field at {@code rawIndex} in this pool
-     * @throws ClassFormatError if the entry at {@code cpi} is not a field
      */
-    JavaField lookupField(int cpi, ResolvedJavaMethod method, int opcode);
+    JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode);
 
     /**
      * Looks up a reference to a method. If {@code opcode} is non-negative, then resolution checks

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2023, 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
@@ -40,10 +40,12 @@ package jdk.vm.ci.runtime.test;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import jdk.vm.ci.meta.JavaField;
 import jdk.vm.ci.meta.JavaMethod;
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
+import jdk.vm.ci.meta.Signature;
 import jdk.vm.ci.runtime.JVMCI;
 
 public class ConstantPoolTest {
@@ -81,6 +83,7 @@ public class ConstantPoolTest {
     }
 
     public static final int ALOAD_0 = 42; // 0x2A
+    public static final int GETSTATIC = 178; // 0xB2
     public static final int INVOKEVIRTUAL = 182; // 0xB6
 
     public static int beU2(byte[] data, int bci) {
@@ -108,4 +111,31 @@ public class ConstantPoolTest {
             }
         }
     }
+
+    static int someStaticField = 1;
+    static int getStaticField() {
+        return someStaticField;
+    }
+
+    @Test
+    public void lookupFieldTest() throws Exception {
+        MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess();
+        ResolvedJavaType type = metaAccess.lookupJavaType(ConstantPoolTest.class);
+
+        String methodName = "getStaticField";
+        Signature methodSig = metaAccess.parseMethodDescriptor("()I");
+        ResolvedJavaMethod m = type.findMethod(methodName, methodSig);
+        Assert.assertNotNull(m);
+
+        // Expected:
+        // 0: getstatic "someStaticField":"I";
+        // 3: ireturn;
+        byte[] bytecode = m.getCode();
+        Assert.assertNotNull(bytecode);
+        Assert.assertEquals(4, bytecode.length);
+        Assert.assertEquals(GETSTATIC, beU1(bytecode, 0));
+        int rawIndex = beU2(bytecode, 1);
+        JavaField field =  m.getConstantPool().lookupField(rawIndex, m, GETSTATIC);
+        Assert.assertEquals("someStaticField", field.getName(), "Wrong field name; rawIndex = " + rawIndex + ";");
+    }
 }