Skip to content
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

Jump to bottom

8281266: [JVMCI] MetaUtil.toInternalName() doesn't handle hidden classes correctly #274

Closed
wants to merge 1 commit into from
Closed

8281266: [JVMCI] MetaUtil.toInternalName() doesn't handle hidden classes correctly #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 33 additions & 19 deletions src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/MetaUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2022, 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 @@ -27,6 +27,11 @@
*/
public class MetaUtil {

public static final char PACKAGE_SEPARATOR_INTERNAL = '/';
public static final char HIDDEN_SEPARATOR_INTERNAL = '.';
public static final char PACKAGE_SEPARATOR_JAVA = HIDDEN_SEPARATOR_INTERNAL;
public static final char HIDDEN_SEPARATOR_JAVA = PACKAGE_SEPARATOR_INTERNAL;

/**
* Extends the functionality of {@link Class#getSimpleName()} to include a non-empty string for
* anonymous and local classes.
Expand Down Expand Up @@ -87,25 +92,27 @@ private static String safeSimpleName(Class<?> clazz) {
}

/**
* Classes for lambdas can have {@code /} characters that are not package separators. These are
* distinguished by being followed by a character that is not a
* Hidden classes have {@code /} characters in their internal names and {@code .} characters in their names returned
* by {@link Class#getName()} that are not package separators.
* These are distinguished by being followed by a character that is not a
* {@link Character#isJavaIdentifierStart(char)} (e.g.,
* "jdk.vm.ci.runtime.test.TypeUniverse$$Lambda$1/869601985").
*
* @param name the name to perform the replacements on
* @param packageSeparator the {@link Character} used as the package separator, e.g. {@code /} in internal form
* @param hiddenSeparator the {@link Character} used as the hidden class separator, e.g. {@code .} in internal form
*/
private static String replacePackageSeparatorsWithDot(String name) {
private static String replacePackageAndHiddenSeparators(String name, Character packageSeparator, Character hiddenSeparator) {
int index = name.indexOf(hiddenSeparator); // check if it's a hidden class
int length = name.length();
int i = 0;
StringBuilder buf = new StringBuilder(length);
while (i < length - 1) {
char ch = name.charAt(i);
if (ch == '/' && Character.isJavaIdentifierStart(name.charAt(i + 1))) {
buf.append('.');
} else {
buf.append(ch);
}
i++;
if (index < 0) {
buf.append(name.replace(packageSeparator, hiddenSeparator));
} else {
buf.append(name.substring(0, index).replace(packageSeparator, hiddenSeparator));
buf.append(packageSeparator);
buf.append(name.substring(index + 1));
}
buf.append(name.charAt(length - 1));
return buf.toString();
}

Expand All @@ -122,17 +129,22 @@ private static String replacePackageSeparatorsWithDot(String name) {
public static String internalNameToJava(String name, boolean qualified, boolean classForNameCompatible) {
switch (name.charAt(0)) {
case 'L': {
String result = replacePackageSeparatorsWithDot(name.substring(1, name.length() - 1));
String type = name.substring(1, name.length() - 1);
String result = replacePackageAndHiddenSeparators(type, PACKAGE_SEPARATOR_INTERNAL, HIDDEN_SEPARATOR_INTERNAL);
if (!qualified) {
final int lastDot = result.lastIndexOf('.');
final int lastDot = result.lastIndexOf(HIDDEN_SEPARATOR_INTERNAL);
if (lastDot != -1) {
result = result.substring(lastDot + 1);
}
}
return result;
}
case '[':
return classForNameCompatible ? replacePackageSeparatorsWithDot(name) : internalNameToJava(name.substring(1), qualified, classForNameCompatible) + "[]";
if (classForNameCompatible) {
return replacePackageAndHiddenSeparators(name, PACKAGE_SEPARATOR_INTERNAL, HIDDEN_SEPARATOR_INTERNAL);
} else {
return internalNameToJava(name.substring(1), qualified, false) + "[]";
}
default:
if (name.length() != 1) {
throw new IllegalArgumentException("Illegal internal name: " + name);
Expand Down Expand Up @@ -213,7 +225,7 @@ static void appendProfile(StringBuilder buf, AbstractJavaProfile<?, ?> profile,
public static String toInternalName(String className) {
if (className.startsWith("[")) {
/* Already in the correct array style. */
return className.replace('.', '/');
return replacePackageAndHiddenSeparators(className, PACKAGE_SEPARATOR_JAVA, HIDDEN_SEPARATOR_JAVA);
}

StringBuilder result = new StringBuilder();
Expand Down Expand Up @@ -252,7 +264,9 @@ public static String toInternalName(String className) {
result.append("V");
break;
default:
result.append("L").append(base.replace('.', '/')).append(";");
result.append("L")
.append(replacePackageAndHiddenSeparators(base, PACKAGE_SEPARATOR_JAVA, HIDDEN_SEPARATOR_JAVA))
.append(";");
break;
}
return result.toString();
Expand Down
15 changes: 8 additions & 7 deletions ...ompiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
import static java.lang.reflect.Modifier.isProtected;
import static java.lang.reflect.Modifier.isPublic;
import static java.lang.reflect.Modifier.isStatic;
import static jdk.vm.ci.meta.MetaUtil.internalNameToJava;
import static jdk.vm.ci.meta.MetaUtil.toInternalName;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -163,16 +165,15 @@ public void isArrayTest() {
}

@Test
public void internalNameTest() {
// Verify that the last slash in lambda types are not replaced with a '.' as they
// are part of the type name.
public void lambdaInternalNameTest() {
// Verify that the last dot in lambda types is properly handled when transitioning from internal name to java
// name and vice versa.
Supplier<Runnable> lambda = () -> () -> System.out.println("run");
ResolvedJavaType lambdaType = metaAccess.lookupJavaType(lambda.getClass());
String typeName = lambdaType.getName();
int typeNameLen = TestResolvedJavaType.class.getSimpleName().length();
int index = typeName.indexOf(TestResolvedJavaType.class.getSimpleName());
String suffix = typeName.substring(index + typeNameLen, typeName.length() - 1);
assertEquals(TestResolvedJavaType.class.getName() + suffix, lambdaType.toJavaName());
String javaName = lambda.getClass().getName();
assertEquals(typeName, toInternalName(javaName));
assertEquals(javaName, internalNameToJava(typeName, true, true));
}

@Test
Expand Down