changeset 57172:40df6530c76b nestmates

8223527: Clean up JVM implementation of hidden classes Summary: Patch hidden class name in cp, ignore nestmate attributes for hidden classes, return name with '/' in external_name() Reviewed-by: dholmes, mchung
author hseigel
date Thu, 26 Sep 2019 17:32:10 +0000
parents 9ae19b8cd6fc
children ad257875959a
files src/hotspot/share/classfile/classFileParser.cpp src/hotspot/share/oops/instanceKlass.cpp src/hotspot/share/oops/instanceKlass.hpp src/hotspot/share/oops/klass.cpp test/hotspot/jtreg/runtime/HiddenClasses/DefineHiddenClass.java test/hotspot/jtreg/runtime/HiddenClasses/hidden/NameInString.java
diffstat 6 files changed, 196 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/classfile/classFileParser.cpp	Tue Sep 24 10:07:33 2019 -0700
+++ b/src/hotspot/share/classfile/classFileParser.cpp	Thu Sep 26 17:32:10 2019 +0000
@@ -3513,7 +3513,7 @@
         }
         cfs->skip_u1(attribute_length, CHECK);
       } else if (_major_version >= JAVA_11_VERSION) {
-        if (tag == vmSymbols::tag_nest_members()) {
+        if (tag == vmSymbols::tag_nest_members() && !is_hidden()) { // ignored for hidden classes
           // Check for NestMembers tag
           if (parsed_nest_members_attribute) {
             classfile_parse_error("Multiple NestMembers attributes in class file %s", CHECK);
@@ -3526,7 +3526,7 @@
           nest_members_attribute_start = cfs->current();
           nest_members_attribute_length = attribute_length;
           cfs->skip_u1(nest_members_attribute_length, CHECK);
-        } else if (tag == vmSymbols::tag_nest_host()) {
+        } else if (tag == vmSymbols::tag_nest_host() && !is_hidden()) { // ignored for hidden classes
           if (parsed_nest_host_attribute) {
             classfile_parse_error("Multiple NestHost attributes in class file %s", CHECK);
           } else {
@@ -3546,7 +3546,7 @@
                          class_info_index, CHECK);
           _nest_host = class_info_index;
         } else {
-          // Unknown attribute
+          // Unknown attribute, or ignored for hidden class
           cfs->skip_u1(attribute_length, CHECK);
         }
       } else {
@@ -4748,6 +4748,17 @@
     );
     return;
   }
+
+  // TBD: should this be an assert() ?
+  if (is_hidden() && (is_interface || is_abstract)) {
+    ResourceMark rm(THREAD);
+    Exceptions::fthrow(
+      THREAD_AND_LOCATION,
+      vmSymbols::java_lang_ClassFormatError(),
+      "Illegal class modifiers in hidden class %s: 0x%X",
+      _class_name->as_C_string(), flags);
+    return;
+  }
 }
 
 static bool has_illegal_visibility(jint flags) {
@@ -6099,6 +6110,26 @@
   // un-named, hidden or unsafe-anonymous class.
 
   if (_is_hidden) {
+    ResourceMark rm(THREAD);
+    assert(_class_name != NULL, "Unexpected null _class_name");
+#ifdef ASSERT
+    if (_need_verify) {
+      verify_legal_class_name(_class_name, CHECK);
+    }
+#endif
+
+    // Construct hidden name from _class_name, "+", and &_cp. Note that we can't
+    // use a '/' because that confuses finding the class's package.  Also, can't
+    // use an illegal char such as ';' because that causes serialization issues
+    // and issues with hidden classes that create their own hidden classes.
+    char addr_buf[20];
+    jio_snprintf(addr_buf, 20, INTPTR_FORMAT, p2i(_cp));
+    size_t new_name_len = _class_name->utf8_length() + 2 + strlen(addr_buf);
+    char* new_name = NEW_RESOURCE_ARRAY(char, new_name_len);
+    jio_snprintf(new_name, new_name_len, "%s+%s",
+                 _class_name->as_C_string(), addr_buf);
+    _class_name->decrement_refcount();
+    _class_name = SymbolTable::new_symbol(new_name);
     _class_name->increment_refcount();
 
     // Add a Utf8 entry containing the hidden name.
@@ -6106,12 +6137,6 @@
     int hidden_index = _orig_cp_size; // this is an extra slot we added
     cp->symbol_at_put(hidden_index, _class_name);
 
-    if (_need_verify) {
-      // Since this name was not in the original constant pool, it didn't get
-      // checked during constantpool parsing.  So, check it here.
-      verify_legal_class_name(_class_name, CHECK);
-    }
-
     // Update this_class_index's slot in the constant pool with the new Utf8 entry.
     // We have to update the resolved_klass_index and the name_index together
     // so extract the existing resolved_klass_index first.
--- a/src/hotspot/share/oops/instanceKlass.cpp	Tue Sep 24 10:07:33 2019 -0700
+++ b/src/hotspot/share/oops/instanceKlass.cpp	Thu Sep 26 17:32:10 2019 +0000
@@ -428,13 +428,13 @@
 }
 
 InstanceKlass* InstanceKlass::allocate_instance_klass(const ClassFileParser& parser, TRAPS) {
-  bool is_nonfindable = parser.is_hidden() || parser.is_unsafe_anonymous();
+  bool is_hidden_or_anonymous = parser.is_hidden() || parser.is_unsafe_anonymous();
   const int size = InstanceKlass::size(parser.vtable_size(),
                                        parser.itable_size(),
                                        nonstatic_oop_map_size(parser.total_oop_map_count()),
                                        parser.is_interface(),
-                                       is_nonfindable,
-                                       should_store_fingerprint(is_nonfindable));
+                                       is_hidden_or_anonymous,
+                                       should_store_fingerprint(is_hidden_or_anonymous));
 
   const Symbol* const class_name = parser.class_name();
   assert(class_name != NULL, "invariant");
@@ -2285,7 +2285,7 @@
   return true;
 }
 
-bool InstanceKlass::should_store_fingerprint(bool is_nonfindable) {
+bool InstanceKlass::should_store_fingerprint(bool is_hidden_or_anonymous) {
 #if INCLUDE_AOT
   // We store the fingerprint into the InstanceKlass only in the following 2 cases:
   if (CalculateClassFingerprint) {
@@ -2296,7 +2296,7 @@
     // (2) We are running -Xshare:dump or -XX:ArchiveClassesAtExit to create a shared archive
     return true;
   }
-  if (UseAOT && is_nonfindable) {
+  if (UseAOT && is_hidden_or_anonymous) {
     // (3) We are using AOT code from a shared library and see a hidden or unsafe anonymous class
     return true;
   }
--- a/src/hotspot/share/oops/instanceKlass.hpp	Tue Sep 24 10:07:33 2019 -0700
+++ b/src/hotspot/share/oops/instanceKlass.hpp	Thu Sep 26 17:32:10 2019 +0000
@@ -759,7 +759,7 @@
   }
   bool supers_have_passed_fingerprint_checks();
 
-  static bool should_store_fingerprint(bool is_nonfindable);
+  static bool should_store_fingerprint(bool is_hidden_or_anonymous);
   bool should_store_fingerprint() const { return should_store_fingerprint(is_hidden() || is_unsafe_anonymous()); }
   bool has_stored_fingerprint() const;
   uint64_t get_stored_fingerprint() const;
@@ -1033,13 +1033,13 @@
 
   static int size(int vtable_length, int itable_length,
                   int nonstatic_oop_map_size,
-                  bool is_interface, bool is_hidden, bool has_stored_fingerprint) {
+                  bool is_interface, bool is_hidden_or_anonymous, bool has_stored_fingerprint) {
     return align_metadata_size(header_size() +
            vtable_length +
            itable_length +
            nonstatic_oop_map_size +
            (is_interface ? (int)sizeof(Klass*)/wordSize : 0) +
-           (is_hidden ? (int)sizeof(Klass*)/wordSize : 0) +
+           (is_hidden_or_anonymous ? (int)sizeof(Klass*)/wordSize : 0) +
            (has_stored_fingerprint ? (int)sizeof(uint64_t*)/wordSize : 0));
   }
   int size() const                    { return size(vtable_length(),
--- a/src/hotspot/share/oops/klass.cpp	Tue Sep 24 10:07:33 2019 -0700
+++ b/src/hotspot/share/oops/klass.cpp	Thu Sep 26 17:32:10 2019 +0000
@@ -688,7 +688,7 @@
 const char* Klass::external_name() const {
   if (is_instance_klass()) {
     const InstanceKlass* ik = static_cast<const InstanceKlass*>(this);
-    if (ik->is_unsafe_anonymous() || ik->is_hidden()) {
+    if (ik->is_unsafe_anonymous()) {
       char addr_buf[20];
       jio_snprintf(addr_buf, 20, "/" INTPTR_FORMAT, p2i(ik));
       size_t addr_len = strlen(addr_buf);
@@ -699,6 +699,19 @@
       strcpy(result + name_len, addr_buf);
       assert(strlen(result) == name_len + addr_len, "");
       return result;
+
+    } else if (ik->is_hidden()) {
+      // Replace the last '+' char with '/'.
+      size_t name_len = name()->utf8_length();
+      char* result = NEW_RESOURCE_ARRAY(char, name_len + 1);
+      name()->as_klass_external_name(result, (int)name_len + 1);
+      for (int index = (int)name_len; index > 0; index--) {
+        if (result[index] == '+') {
+          result[index] = '/';
+          break;
+        }
+      }
+      return result;
     }
   }
   if (name() == NULL)  return "<unknown>";
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/HiddenClasses/DefineHiddenClass.java	Thu Sep 26 17:32:10 2019 +0000
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @modules java.base/jdk.internal.misc
+ * @library /test/lib
+ * @build jdk.test.lib.JDKToolLauncher
+ *        jdk.test.lib.process.ProcessTools
+ *        jdk.test.lib.Utils
+ * @run main/othervm -Xverify:remote DefineHiddenClass
+ */
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import static java.lang.invoke.MethodHandles.Lookup.ClassOption.*;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import jdk.internal.misc.Unsafe;
+
+import jdk.test.lib.JDKToolLauncher;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.Utils;
+
+/* package-private */ interface Test {
+  void test();
+}
+
+
+public class DefineHiddenClass {
+
+    static final Class<?> klass = DefineHiddenClass.class;
+    static final Path SRC_DIR = Paths.get(Utils.TEST_SRC, "hidden");
+    static final Path CLASSES_DIR = Paths.get(Utils.TEST_CLASSES, "hidden");
+
+    static void compileSources(String sourceFile) throws Throwable {
+        JDKToolLauncher launcher = JDKToolLauncher.createUsingTestJDK("javac");
+        launcher.addToolArg("-cp")
+                .addToolArg(Utils.TEST_CLASSES.toString())
+                .addToolArg("-d")
+                .addToolArg(CLASSES_DIR.toString())
+            .addToolArg(Paths.get(SRC_DIR.toString(), sourceFile).toString());
+
+        int exitCode = ProcessTools.executeCommand(launcher.getCommand())
+                                   .getExitValue();
+        if (exitCode != 0) {
+            throw new RuntimeException("Compilation of the test failed. "
+                    + "Unexpected exit code: " + exitCode);
+        }
+    }
+
+    static byte[] readClassFile(String classFileName) throws Exception {
+        File classFile = new File(CLASSES_DIR + File.separator + classFileName);
+        try (FileInputStream in = new FileInputStream(classFile);
+             ByteArrayOutputStream out = new ByteArrayOutputStream())
+        {
+            int b;
+            while ((b = in.read()) != -1) {
+                out.write(b);
+            }
+            return out.toByteArray();
+        }
+    }
+
+    public static void main(String[] args) throws Throwable {
+        compileSources("NameInString.java");
+        Lookup lookup = MethodHandles.lookup();
+        byte[] bytes = readClassFile("NameInString.class");
+        Class<?> c = lookup.defineHiddenClass(bytes, true, NESTMATE).lookupClass();
+        Test t = (Test) c.newInstance();
+        t.test();
+    }
+
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/HiddenClasses/hidden/NameInString.java	Thu Sep 26 17:32:10 2019 +0000
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * Test that a string that is the same as its hidden class name does not
+ * get clobbered when the JVM changes the hidden class's name.
+ */
+public class NameInString implements Test {
+
+    private String realTest() {
+        return "NameInString";
+    }
+
+    public void test() {
+        String result = realTest();
+        // Make sure that the Utf8 constant pool entry for "NameInString" is okay.
+        if (!result.substring(0, 6).equals("NameIn") ||
+            !result.substring(6).equals("String")) {
+            throw new RuntimeException("'NameInString is bad: " + result);
+        }
+
+    }
+}