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

Reviewed-by: sgehwolf, dnsimon

Author: zakkak

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

@@ -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
@@ -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.
@@ -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();
     }
 
@@ -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);
@@ -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();
@@ -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();

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

@@ -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;
@@ -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