8152844: JVM InstanceKlass Methods For Obtaining Package/Module Should Be Moved to Klass
authorrprotacio
Tue, 26 Apr 2016 09:08:12 -0400
changeset 38094 46977cd73d86
parent 38090 34d0dd634032
child 38095 bc6ab798f9e7
8152844: JVM InstanceKlass Methods For Obtaining Package/Module Should Be Moved to Klass Summary: Converted package() and module() functions to pure virtual functions of Klass Reviewed-by: dholmes, coleenp, lfoltan, hseigel
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/src/share/vm/oops/klass.hpp
hotspot/src/share/vm/oops/objArrayKlass.cpp
hotspot/src/share/vm/oops/objArrayKlass.hpp
hotspot/src/share/vm/oops/typeArrayKlass.cpp
hotspot/src/share/vm/oops/typeArrayKlass.hpp
hotspot/src/share/vm/runtime/reflection.cpp
hotspot/test/runtime/modules/getModuleJNI/GetModule.java
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Tue Apr 26 09:08:12 2016 -0400
@@ -2296,7 +2296,7 @@
   PackageEntry* classpkg2;
   if (class2->is_instance_klass()) {
     classloader2 = class2->class_loader();
-    classpkg2 = InstanceKlass::cast(class2)->package();
+    classpkg2 = class2->package();
   } else {
     assert(class2->is_typeArray_klass(), "should be type array");
     classloader2 = NULL;
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Tue Apr 26 09:08:12 2016 -0400
@@ -27,9 +27,9 @@
 
 #include "classfile/classLoader.hpp"
 #include "classfile/classLoaderData.hpp"
+#include "classfile/moduleEntry.hpp"
 #include "classfile/packageEntry.hpp"
 #include "gc/shared/specialized_oop_closures.hpp"
-#include "classfile/moduleEntry.hpp"
 #include "logging/logLevel.hpp"
 #include "memory/referenceType.hpp"
 #include "oops/annotations.hpp"
--- a/hotspot/src/share/vm/oops/klass.hpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/klass.hpp	Tue Apr 26 09:08:12 2016 -0400
@@ -52,11 +52,13 @@
 template <class T> class Array;
 template <class T> class GrowableArray;
 class ClassLoaderData;
+class fieldDescriptor;
+class KlassSizeStats;
 class klassVtable;
+class ModuleEntry;
+class PackageEntry;
 class ParCompactionManager;
 class PSPromotionManager;
-class KlassSizeStats;
-class fieldDescriptor;
 class vtableEntry;
 
 class Klass : public Metadata {
@@ -274,6 +276,9 @@
     _shared_class_path_index = index;
   };
 
+  // Obtain the module or package for this class
+  virtual ModuleEntry* module() const = 0;
+  virtual PackageEntry* package() const = 0;
 
  protected:                                // internal accessors
   void     set_subklass(Klass* s);
--- a/hotspot/src/share/vm/oops/objArrayKlass.cpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/objArrayKlass.cpp	Tue Apr 26 09:08:12 2016 -0400
@@ -23,6 +23,8 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/moduleEntry.hpp"
+#include "classfile/packageEntry.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "classfile/vmSymbols.hpp"
@@ -135,14 +137,7 @@
   // GC walks these as strong roots.
   loader_data->add_class(oak);
 
-  // The array is defined in the module of its bottom class
-  Klass* bottom_klass = oak->bottom_klass();
-  ModuleEntry* module;
-  if (bottom_klass->is_instance_klass()) {
-    module = InstanceKlass::cast(bottom_klass)->module();
-  } else {
-    module = ModuleEntryTable::javabase_module();
-  }
+  ModuleEntry* module = oak->module();
   assert(module != NULL, "No module entry for array");
 
   // Call complete_create_array_klass after all instance variables has been initialized.
@@ -422,6 +417,16 @@
                         | (JVM_ACC_ABSTRACT | JVM_ACC_FINAL);
 }
 
+ModuleEntry* ObjArrayKlass::module() const {
+  assert(bottom_klass() != NULL, "ObjArrayKlass returned unexpected NULL bottom_klass");
+  // The array is defined in the module of its bottom class
+  return bottom_klass()->module();
+}
+
+PackageEntry* ObjArrayKlass::package() const {
+  assert(bottom_klass() != NULL, "ObjArrayKlass returned unexpected NULL bottom_klass");
+  return bottom_klass()->package();
+}
 
 // Printing
 
--- a/hotspot/src/share/vm/oops/objArrayKlass.hpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/objArrayKlass.hpp	Tue Apr 26 09:08:12 2016 -0400
@@ -54,6 +54,9 @@
   void set_bottom_klass(Klass* k)   { _bottom_klass = k; }
   Klass** bottom_klass_addr()       { return &_bottom_klass; }
 
+  ModuleEntry* module() const;
+  PackageEntry* package() const;
+
   // Compiler/Interpreter offset
   static ByteSize element_klass_offset() { return in_ByteSize(offset_of(ObjArrayKlass, _element_klass)); }
 
--- a/hotspot/src/share/vm/oops/typeArrayKlass.cpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/typeArrayKlass.cpp	Tue Apr 26 09:08:12 2016 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, 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
@@ -23,6 +23,8 @@
  */
 
 #include "precompiled.hpp"
+#include "classfile/moduleEntry.hpp"
+#include "classfile/packageEntry.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "classfile/vmSymbols.hpp"
@@ -341,3 +343,12 @@
 const char* TypeArrayKlass::internal_name() const {
   return Klass::external_name();
 }
+
+// A TypeArrayKlass is an array of a primitive type, its defining module is java.base
+ModuleEntry* TypeArrayKlass::module() const {
+  return ModuleEntryTable::javabase_module();
+}
+
+PackageEntry* TypeArrayKlass::package() const {
+  return NULL;
+}
--- a/hotspot/src/share/vm/oops/typeArrayKlass.hpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/oops/typeArrayKlass.hpp	Tue Apr 26 09:08:12 2016 -0400
@@ -150,6 +150,9 @@
 
  public:
   const char* internal_name() const;
+
+  ModuleEntry* module() const;
+  PackageEntry* package() const;
 };
 
 #endif // SHARE_VM_OOPS_TYPEARRAYKLASS_HPP
--- a/hotspot/src/share/vm/runtime/reflection.cpp	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/src/share/vm/runtime/reflection.cpp	Tue Apr 26 09:08:12 2016 -0400
@@ -502,17 +502,16 @@
     }
 
     // Find the module entry for current_class, the accessor
-    ModuleEntry* module_from = InstanceKlass::cast(current_class)->module();
+    ModuleEntry* module_from = current_class->module();
     // Find the module entry for new_class, the accessee
     if (new_class->is_objArray_klass()) {
       new_class = ObjArrayKlass::cast(new_class)->bottom_klass();
     }
-    if (!new_class->is_instance_klass()) {
-      // Everyone can read a typearray.
-      assert (new_class->is_typeArray_klass(), "Unexpected klass type");
+    if (new_class->is_typeArray_klass()) {
+      // A TypeArray's defining module is java.base, access to the TypeArray is allowed
       return ACCESS_OK;
     }
-    ModuleEntry* module_to = InstanceKlass::cast(new_class)->module();
+    ModuleEntry* module_to = new_class->module();
 
     // both in same (possibly unnamed) module
     if (module_from == module_to) {
@@ -532,7 +531,7 @@
       return MODULE_NOT_READABLE;
     }
 
-    PackageEntry* package_to = InstanceKlass::cast(new_class)->package();
+    PackageEntry* package_to = new_class->package();
     assert(package_to != NULL, "can not obtain new_class' package");
 
     // Once readability is established, if module_to exports T unqualifiedly,
@@ -570,20 +569,13 @@
   char * msg = NULL;
   if (result != OTHER_PROBLEM && new_class != NULL && current_class != NULL) {
     // Find the module entry for current_class, the accessor
-    ModuleEntry* module_from = InstanceKlass::cast(current_class)->module();
+    ModuleEntry* module_from = current_class->module();
     const char * module_from_name = module_from->is_named() ? module_from->name()->as_C_string() : UNNAMED_MODULE;
     const char * current_class_name = current_class->external_name();
 
     // Find the module entry for new_class, the accessee
     ModuleEntry* module_to = NULL;
-    if (new_class->is_objArray_klass()) {
-      new_class = ObjArrayKlass::cast(new_class)->bottom_klass();
-    }
-    if (new_class->is_instance_klass()) {
-      module_to = InstanceKlass::cast(new_class)->module();
-    } else {
-      module_to = ModuleEntryTable::javabase_module();
-    }
+    module_to = new_class->module();
     const char * module_to_name = module_to->is_named() ? module_to->name()->as_C_string() : UNNAMED_MODULE;
     const char * new_class_name = new_class->external_name();
 
@@ -611,10 +603,10 @@
       }
 
     } else if (result == TYPE_NOT_EXPORTED) {
-      assert(InstanceKlass::cast(new_class)->package() != NULL,
+      assert(new_class->package() != NULL,
              "Unnamed packages are always exported");
       const char * package_name =
-        InstanceKlass::cast(new_class)->package()->name()->as_klass_external_name();
+        new_class->package()->name()->as_klass_external_name();
       assert(module_to->is_named(), "Unnamed modules export all packages");
       if (module_from->is_named()) {
         size_t len = 118 + strlen(current_class_name) + 2*strlen(module_from_name) +
--- a/hotspot/test/runtime/modules/getModuleJNI/GetModule.java	Tue Apr 26 11:49:37 2016 +0000
+++ b/hotspot/test/runtime/modules/getModuleJNI/GetModule.java	Tue Apr 26 09:08:12 2016 -0400
@@ -42,10 +42,22 @@
 
     public static void main(String[] args) {
         Module module;
+        Module javaBaseModule;
+
+        // Module for primitive type, should be "java.base"
+        java.lang.Integer primitive_int = 1;
+        try {
+            javaBaseModule = (Module)callGetModule(primitive_int.getClass());
+            if (!javaBaseModule.getName().equals("java.base")) {
+                throw new RuntimeException("Unexpected module name for primitive type: " +
+                                           javaBaseModule.getName());
+            }
+        } catch(Throwable e) {
+            throw new RuntimeException("Unexpected exception for Integer: " + e.toString());
+        }
 
         // Module for array of primitives, should be "java.base"
         int[] int_array = {1, 2, 3};
-        Module javaBaseModule;
         try {
             javaBaseModule = (Module)callGetModule(int_array.getClass());
             if (!javaBaseModule.getName().equals("java.base")) {
@@ -56,7 +68,19 @@
             throw new RuntimeException("Unexpected exception for [I: " + e.toString());
         }
 
-        // Module for java.lang.String
+        // Module for multi-dimensional array of primitives, should be "java.base"
+        int[][] multi_int_array = { {1, 2, 3}, {4, 5, 6} };
+        try {
+            javaBaseModule = (Module)callGetModule(multi_int_array.getClass());
+            if (!javaBaseModule.getName().equals("java.base")) {
+                throw new RuntimeException("Unexpected module name for multi-dimensional array of primitives: " +
+                                           javaBaseModule.getName());
+            }
+        } catch(Throwable e) {
+            throw new RuntimeException("Unexpected exception for multi-dimensional Integer array: " + e.toString());
+        }
+
+        // Module for java.lang.String, should be "java.base"
         java.lang.String str = "abc";
         try {
             module = (Module)callGetModule(str.getClass());
@@ -68,6 +92,30 @@
             throw new RuntimeException("Unexpected exception for String: " + e.toString());
         }
 
+        // Module for array of java.lang.Strings, should be "java.base"
+        java.lang.String[] str_array = {"a", "b", "c"};
+        try {
+            javaBaseModule = (Module)callGetModule(str_array.getClass());
+            if (!javaBaseModule.getName().equals("java.base")) {
+                throw new RuntimeException("Unexpected module name for array of Strings: " +
+                                           javaBaseModule.getName());
+            }
+        } catch(Throwable e) {
+            throw new RuntimeException("Unexpected exception for String array: " + e.toString());
+        }
+
+        // Module for multi-dimensional array of java.lang.Strings, should be "java.base"
+        java.lang.String[][] multi_str_array = { {"a", "b", "c"}, {"d", "e", "f"} };
+        try {
+            javaBaseModule = (Module)callGetModule(multi_str_array.getClass());
+            if (!javaBaseModule.getName().equals("java.base")) {
+                throw new RuntimeException("Unexpected module name for multi-dimensional array of Strings: " +
+                                           javaBaseModule.getName());
+            }
+        } catch(Throwable e) {
+            throw new RuntimeException("Unexpected exception for multidimensional String array: " + e.toString());
+        }
+
         // Module for java.lang.management.LockInfo
         try {
             LockInfo li = new LockInfo("java.lang.Class", 57);