changeset 48654:d6388b652504

8192989: runtime/appcds/javaldr/ArrayTest.java crashes with assert(k->is_instance_klass()) Summary: disable loading array classes from the class list Reviewed-by: iklam, jiangli
author ccheung
date Fri, 08 Dec 2017 15:14:08 -0800
parents 0997d6959851
children ecff0c7bfb4d
files src/hotspot/share/classfile/classListParser.cpp src/hotspot/share/classfile/classListParser.hpp src/hotspot/share/memory/metaspaceShared.cpp test/hotspot/jtreg/runtime/appcds/javaldr/ArrayTest.java
diffstat 4 files changed, 50 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/classfile/classListParser.cpp	Fri Dec 08 15:24:47 2017 -0500
+++ b/src/hotspot/share/classfile/classListParser.cpp	Fri Dec 08 15:14:08 2017 -0800
@@ -315,12 +315,13 @@
   return k;
 }
 
-InstanceKlass* ClassListParser::load_current_class(TRAPS) {
+Klass* ClassListParser::load_current_class(TRAPS) {
   TempNewSymbol class_name_symbol = SymbolTable::new_symbol(_class_name, THREAD);
   guarantee(!HAS_PENDING_EXCEPTION, "Exception creating a symbol.");
 
-  InstanceKlass *klass = NULL;
+  Klass *klass = NULL;
   if (!is_loading_from_source()) {
+    // Load classes for the boot/platform/app loaders only.
     if (is_super_specified()) {
       error("If source location is not specified, super class must not be specified");
     }
@@ -330,40 +331,36 @@
 
     bool non_array = !FieldType::is_array(class_name_symbol);
 
-    Handle s = java_lang_String::create_from_symbol(class_name_symbol, CHECK_0);
-    // Translate to external class name format, i.e., convert '/' chars to '.'
-    Handle string = java_lang_String::externalize_classname(s, CHECK_0);
     JavaValue result(T_OBJECT);
-    InstanceKlass* spec_klass =  non_array ?
-      SystemDictionary::ClassLoader_klass() : SystemDictionary::Class_klass();
-    Symbol* method_name = non_array ?
-      vmSymbols::loadClass_name() : vmSymbols::forName_name();
-    Handle loader = Handle(THREAD, SystemDictionary::java_system_loader());
+    if (non_array) {
+      // At this point, we are executing in the context of the boot loader. We
+      // cannot call Class.forName because that is context dependent and
+      // would load only classes for the boot loader.
+      //
+      // Instead, let's call java_system_loader().loadClass() directly, which will
+      // delegate to the correct loader (boot, platform or app) depending on
+      // the class name.
 
-    if (non_array) {
+      Handle s = java_lang_String::create_from_symbol(class_name_symbol, CHECK_0);
+      // ClassLoader.loadClass() wants external class name format, i.e., convert '/' chars to '.'
+      Handle ext_class_name = java_lang_String::externalize_classname(s, CHECK_0);
+      Handle loader = Handle(THREAD, SystemDictionary::java_system_loader());
+
       JavaCalls::call_virtual(&result,
                               loader, //SystemDictionary::java_system_loader(),
-                              spec_klass,
-                              method_name, //vmSymbols::loadClass_name(),
+                              SystemDictionary::ClassLoader_klass(),
+                              vmSymbols::loadClass_name(),
                               vmSymbols::string_class_signature(),
-                              string,
+                              ext_class_name,
                               THREAD);
     } else {
-      JavaCalls::call_static(&result,
-                             spec_klass,
-                             method_name,
-                             vmSymbols::string_class_signature(),
-                             string,
-                             CHECK_NULL);
+      // array classes are not supported in class list.
+      THROW_NULL(vmSymbols::java_lang_ClassNotFoundException());
     }
     assert(result.get_type() == T_OBJECT, "just checking");
     oop obj = (oop) result.get_jobject();
     if (!HAS_PENDING_EXCEPTION && (obj != NULL)) {
-      if (non_array) {
-        klass = InstanceKlass::cast(java_lang_Class::as_Klass(obj));
-      } else {
-        klass = static_cast<InstanceKlass*>(java_lang_Class::array_klass_acquire(obj));
-      }
+      klass = java_lang_Class::as_Klass(obj);
     } else { // load classes in bootclasspath/a
       if (HAS_PENDING_EXCEPTION) {
         CLEAR_PENDING_EXCEPTION;
@@ -372,7 +369,7 @@
       if (non_array) {
         Klass* k = SystemDictionary::resolve_or_null(class_name_symbol, CHECK_NULL);
         if (k != NULL) {
-          klass = InstanceKlass::cast(k);
+          klass = k;
         } else {
           if (!HAS_PENDING_EXCEPTION) {
             THROW_NULL(vmSymbols::java_lang_ClassNotFoundException());
@@ -388,14 +385,15 @@
     }
   }
 
-  if (klass != NULL && is_id_specified()) {
+  if (klass != NULL && klass->is_instance_klass() && is_id_specified()) {
+    InstanceKlass* ik = InstanceKlass::cast(klass);
     int id = this->id();
-    SystemDictionaryShared::update_shared_entry(klass, id);
+    SystemDictionaryShared::update_shared_entry(ik, id);
     InstanceKlass* old = table()->lookup(id);
-    if (old != NULL && old != klass) {
+    if (old != NULL && old != ik) {
       error("Duplicated ID %d for class %s", id, _class_name);
     }
-    table()->add(id, klass);
+    table()->add(id, ik);
   }
 
   return klass;
--- a/src/hotspot/share/classfile/classListParser.hpp	Fri Dec 08 15:24:47 2017 -0500
+++ b/src/hotspot/share/classfile/classListParser.hpp	Fri Dec 08 15:14:08 2017 -0800
@@ -136,7 +136,7 @@
     return _class_name;
   }
 
-  InstanceKlass* load_current_class(TRAPS);
+  Klass* load_current_class(TRAPS);
 
   bool is_loading_from_source();
 
--- a/src/hotspot/share/memory/metaspaceShared.cpp	Fri Dec 08 15:24:47 2017 -0500
+++ b/src/hotspot/share/memory/metaspaceShared.cpp	Fri Dec 08 15:14:08 2017 -0800
@@ -1627,14 +1627,16 @@
           log_trace(cds)("Shared spaces preloaded: %s", klass->external_name());
         }
 
-        InstanceKlass* ik = InstanceKlass::cast(klass);
+        if (klass->is_instance_klass()) {
+          InstanceKlass* ik = InstanceKlass::cast(klass);
 
-        // Link the class to cause the bytecodes to be rewritten and the
-        // cpcache to be created. The linking is done as soon as classes
-        // are loaded in order that the related data structures (klass and
-        // cpCache) are located together.
-        try_link_class(ik, THREAD);
-        guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
+          // Link the class to cause the bytecodes to be rewritten and the
+          // cpcache to be created. The linking is done as soon as classes
+          // are loaded in order that the related data structures (klass and
+          // cpCache) are located together.
+          try_link_class(ik, THREAD);
+          guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
+        }
 
         class_count++;
       }
--- a/test/hotspot/jtreg/runtime/appcds/javaldr/ArrayTest.java	Fri Dec 08 15:24:47 2017 -0500
+++ b/test/hotspot/jtreg/runtime/appcds/javaldr/ArrayTest.java	Fri Dec 08 15:14:08 2017 -0800
@@ -44,7 +44,9 @@
     static String arrayClasses[] = {
         "ArrayTestHelper",
         "[Ljava/lang/Comparable;",
-        "[I"
+        "[I",
+        "[[[Ljava/lang/Object;",
+        "[[B"
     };
 
     public static void main(String[] args) throws Exception {
@@ -56,7 +58,12 @@
         String bootClassPath = "-Xbootclasspath/a:" + whiteBoxJar;
 
         // create an archive containing array classes
-        TestCommon.dump(appJar, TestCommon.list(arrayClasses), bootClassPath, "-verbose:class");
+        OutputAnalyzer output = TestCommon.dump(appJar, TestCommon.list(arrayClasses), bootClassPath, "-verbose:class");
+        // we currently don't support array classes during CDS dump
+        output.shouldContain("Preload Warning: Cannot find [Ljava/lang/Comparable;")
+              .shouldContain("Preload Warning: Cannot find [I")
+              .shouldContain("Preload Warning: Cannot find [[[Ljava/lang/Object;")
+              .shouldContain("Preload Warning: Cannot find [[B");
 
         List<String> argsList = new ArrayList<String>();
         argsList.add("-XX:+UnlockDiagnosticVMOptions");
@@ -67,12 +74,13 @@
         argsList.add("-verbose:class");
         argsList.add("ArrayTestHelper");
         // the following are input args to the ArrayTestHelper.
-        for (int i = 0; i < arrayClasses.length; i++) {
+        // skip checking array classes during run time
+        for (int i = 0; i < 1; i++) {
             argsList.add(arrayClasses[i]);
         }
         String[] opts = new String[argsList.size()];
         opts = argsList.toArray(opts);
-        OutputAnalyzer output = TestCommon.execCommon(opts);
+        output = TestCommon.execCommon(opts);
         TestCommon.checkExec(output);
     }
 }