Skip to content

8315637: JDK-8314249 broke libgraal #15575

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
iklam wants to merge 2 commits into from
Closed

8315637: JDK-8314249 broke libgraal #15575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6557266
8315637: JDK-8314249 broke libgraal
iklam Sep 5, 2023
7324517
fixed white spaces
iklam Sep 6, 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
41 changes: 24 additions & 17 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 @@ -336,48 +336,55 @@ int lookupNameAndTypeRefIndexInPool(HotSpotConstantPool constantPool, int rawInd

/**
* 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
Expand Down
48 changes: 17 additions & 31 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 @@ -396,18 +396,6 @@ private int getNameAndTypeRefIndexAt(int rawIndex, int opcode) {
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) {
Copy link
Contributor

@matias9927 matias9927 Sep 6, 2023

Choose a reason for hiding this comment

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

This method, and getSignatureOf() seem to be unused but is it possible that they could have use in the future?

Copy link
Member Author

@iklam iklam Sep 6, 2023

Choose a reason for hiding this comment

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

These were private methods that had a single caller. All they did was to forward the call to CompilerToVM, so I removed them so that I didn't need to worry about renaming their parameters.

In the future, after we get rid of which and cpci, maybe we can refactor the code to have better organization, and possibly bring back these methods.

return compilerToVM().lookupNameInPool(this, rawIndex, opcode);
}

/**
* Gets the name reference index of a {@code JVM_CONSTANT_NameAndType} constant pool entry at
* index {@code index}.
Expand All @@ -421,17 +409,6 @@ private int getNameRefIndexAt(int index) {
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
Expand All @@ -449,12 +426,13 @@ private int getSignatureRefIndexAt(int index) {
/**
* 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);
}

/**
Expand Down Expand Up @@ -719,18 +697,26 @@ private static JavaType getJavaType(final Object type) {

@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));
}
Expand Down
96 changes: 94 additions & 2 deletions ...eg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/ConstantPoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ static String concatString2(String a, String b) {
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);
}
Expand All @@ -184,7 +189,7 @@ public void lookupAppendixTest() throws Throwable {
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;");
Expand All @@ -209,7 +214,7 @@ public void lookupAppendixTest_dynamic(String methodName) throws Exception {
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");
Expand All @@ -233,4 +238,91 @@ public void lookupAppendixTest_virtual() throws Exception {
//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<Invokers$Holder.linkToTargetMethod(Object, Object, Object)>"),
"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<Object.hashCode()>"),
"wrong method: " + callee);
}
}