Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6527e67
8314078: HotSpotConstantPool.lookupField() asserts due to field chang…
iklam Aug 11, 2023
c322b8e
(no actual code changes) Fixed variable names and javadoc so that cha…
iklam Aug 11, 2023
21976c0
@dougxc review: Added comments about rawIndex vs cpi
iklam Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1602,6 +1602,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);
Expand Down Expand Up @@ -3125,6 +3133,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)},
Expand Down
26 changes: 19 additions & 7 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,19 @@ int decodeIndyIndexToCPIndex(HotSpotConstantPool constantPool, int encoded_indy_

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
Expand Down Expand Up @@ -475,8 +488,8 @@ HotSpotResolvedObjectTypeImpl resolveTypeInPool(HotSpotConstantPool constantPool
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:
*
Expand All @@ -488,19 +501,18 @@ HotSpotResolvedObjectTypeImpl resolveTypeInPool(HotSpotConstantPool constantPool
* ]
* </pre>
*
* 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
Expand Down
76 changes: 41 additions & 35 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,10 @@ private static int rawIndexToConstantPoolCacheIndex(int rawIndex, int opcode) {
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);
Expand Down Expand Up @@ -748,8 +744,8 @@ public JavaType lookupType(int cpi, int opcode) {
}

@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:
Expand All @@ -759,43 +755,45 @@ public JavaType lookupReferencedType(int cpi, int opcode) {
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the new warning here: cpi is not used. I guess it is correct, seeing how methods are accepting rawIndex now, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. cpi is not used, as the next line uses rawIndex to obtain information about the field bytecode.

final int nameAndTypeIndex = getNameAndTypeRefIndexAt(rawIndex, opcode);

I'll remove the cpi line in my next JVMCI cleanup PR.

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
Expand Down Expand Up @@ -833,19 +831,25 @@ public int rawIndexToConstantPoolIndex(int rawIndex, int opcode) {
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:
Expand All @@ -855,57 +859,59 @@ public void loadReferencedType(int cpi, int opcode, boolean initialize) {
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);
}
Expand Down
41 changes: 23 additions & 18 deletions src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ConstantPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -29,6 +29,14 @@
* 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 {

Expand All @@ -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
Expand Down
Loading