From 911d1dbbf7362693c736b905b42e5150fc4f8a96 Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Mon, 14 Aug 2023 15:37:44 +0000 Subject: [PATCH] 8314078: HotSpotConstantPool.lookupField() asserts due to field changes in ConstantPool.cpp Reviewed-by: dnsimon, coleenp --- src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 9 +++ .../jdk/vm/ci/hotspot/CompilerToVM.java | 26 +++++-- .../vm/ci/hotspot/HotSpotConstantPool.java | 76 ++++++++++--------- .../classes/jdk/vm/ci/meta/ConstantPool.java | 41 +++++----- .../vm/ci/runtime/test/ConstantPoolTest.java | 32 +++++++- 5 files changed, 123 insertions(+), 61 deletions(-) diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index 0b6a821b74b..e5b62375d2a 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/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)}, 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 59f9cf1b129..ed96bd03597 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 @@ -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 - * {@code cpi} in {@code constantPool}. For some opcodes, checks are performed that require the + * Looks up and attempts to resolve the {@code JVM_CONSTANT_Field} entry denoted by + * {@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 { * ] * * - * The behavior of this method is undefined if {@code cpi} does not denote a - * {@code JVM_CONSTANT_Field} entry. + * The behavior of this method is undefined if {@code rawIndex} is invalid. * * @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 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 390becd4179..bc81d77138a 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 @@ -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 || - opcode == Bytecodes.PUTFIELD || - opcode == Bytecodes.GETSTATIC || - opcode == Bytecodes.PUTSTATIC || - opcode == Bytecodes.INVOKEINTERFACE || - opcode == Bytecodes.INVOKEVIRTUAL || - opcode == Bytecodes.INVOKESPECIAL || - opcode == Bytecodes.INVOKESTATIC) { + if (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) { - int index; + public JavaType lookupReferencedType(int rawIndex, int opcode) { + 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); - index = getKlassRefIndexAt(index, opcode); + int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, 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) { - final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode); - final int nameAndTypeIndex = getNameAndTypeRefIndexAt(index, opcode); + 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); 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) { - loadReferencedType(cpi, opcode, true /* initialize */); + public void loadReferencedType(int rawIndex, int opcode) { + loadReferencedType(rawIndex, opcode, true /* initialize */); } @Override @SuppressWarnings("fallthrough") - public void loadReferencedType(int cpi, int opcode, boolean initialize) { - int index; + public void loadReferencedType(int rawIndex, int opcode, boolean initialize) { + 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)) { - throw new IllegalArgumentException("must use invokedynamic index but got " + cpi); + if (!isInvokedynamicIndex(rawIndex)) { + 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); - index = compilerToVM().constantPoolRemapInstructionOperandFromCache(this, index); + int cpci = rawIndexToConstantPoolCacheIndex(rawIndex, opcode); + 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); } diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java index 7c7cfb5687f..44cd1bc0d48 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java +++ b/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: + * + * + * 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 - * {@code opcode} bytecode instruction. + * Looks up the type referenced by the {@code rawIndex}. * - * @param cpi the index of a constant pool entry that references a type - * @param opcode the opcode of the instruction with {@code cpi} as an operand + * @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 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 opcode the opcode of the instruction for which the lookup is being performed or - * {@code -1} + * @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 * @param method the method for which the lookup is being performed - * @return a reference to the field at {@code cpi} in this pool - * @throws ClassFormatError if the entry at {@code cpi} is not a field + * @return a reference to the field at {@code rawIndex} in this pool */ - 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 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 2fb53b38256..1241c33f24b 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 @@ -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 + ";"); + } }