8170987: Module system implementation refresh (12/2016)
authoralanb
Fri, 16 Dec 2016 05:46:52 +0000
changeset 42630 ae91fbc4b59f
parent 42540 bc0b1757609d
child 42631 c9f65c500158
8170987: Module system implementation refresh (12/2016) Reviewed-by: lfoltan, coleenp, mchung Contributed-by: harold.seigel@oracle.com, serguei.spitsyn@oracle.com
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/classFileParser.hpp
hotspot/src/share/vm/classfile/modules.cpp
hotspot/test/runtime/modules/AccessCheck/ModuleLibrary.java
hotspot/test/runtime/modules/JVMDefineModule.java
hotspot/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Fri Dec 16 05:46:52 2016 +0000
@@ -4684,16 +4684,7 @@
   for (const char* p = name; p != name + length;) {
     jchar ch = *p;
     if (ch < 128) {
-      if (ch == '.') {
-        // permit '.' in module names unless it's the first char, or
-        // preceding char is also a '.', or last char is a '.'.
-        if ((type != ClassFileParser::LegalModule) ||
-          (p == name) || (*(p-1) == '.') ||
-          (p == name + length - 1)) {
-          return false;
-        }
-      }
-      if (ch == ';' || ch == '[' ) {
+      if (ch == '.' || ch == ';' || ch == '[' ) {
         return false;   // do not permit '.', ';', or '['
       }
       if (ch == '/') {
--- a/hotspot/src/share/vm/classfile/classFileParser.hpp	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/src/share/vm/classfile/classFileParser.hpp	Fri Dec 16 05:46:52 2016 +0000
@@ -72,7 +72,7 @@
     NOF_PUBLICITY_LEVELS
   };
 
-  enum { LegalClass, LegalField, LegalMethod, LegalModule }; // used to verify unqualified names
+  enum { LegalClass, LegalField, LegalMethod }; // used to verify unqualified names
 
  private:
   const ClassFileStream* _stream; // Actual input stream
--- a/hotspot/src/share/vm/classfile/modules.cpp	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/src/share/vm/classfile/modules.cpp	Fri Dec 16 05:46:52 2016 +0000
@@ -51,10 +51,7 @@
 static bool verify_module_name(char *module_name) {
   if (module_name == NULL) return false;
   int len = (int)strlen(module_name);
-  return (len > 0 && len <= Symbol::max_length() &&
-    UTF8::is_legal_utf8((unsigned char *)module_name, len, false) &&
-    ClassFileParser::verify_unqualified_name(module_name, len,
-    ClassFileParser::LegalModule));
+  return (len > 0 && len <= Symbol::max_length());
 }
 
 bool Modules::verify_package_name(char *package_name) {
--- a/hotspot/test/runtime/modules/AccessCheck/ModuleLibrary.java	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/test/runtime/modules/AccessCheck/ModuleLibrary.java	Fri Dec 16 05:46:52 2016 +0000
@@ -32,7 +32,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Supplier;
 
 /**
  * A container of modules that acts as a ModuleFinder for testing
@@ -52,12 +51,13 @@
 
                 URI uri = URI.create("module:/" + descriptor.name());
 
-                Supplier<ModuleReader> supplier = () -> {
-                    throw new UnsupportedOperationException();
+                ModuleReference mref = new ModuleReference(descriptor, uri) {
+                    @Override
+                    public ModuleReader open() {
+                        throw new UnsupportedOperationException();
+                    }
                 };
 
-                ModuleReference mref = new ModuleReference(descriptor, uri, supplier);
-
                 namesToReference.put(name, mref);
             }
         }
--- a/hotspot/test/runtime/modules/JVMDefineModule.java	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/test/runtime/modules/JVMDefineModule.java	Fri Dec 16 05:46:52 2016 +0000
@@ -58,9 +58,9 @@
 */
 
         // NULL package argument, should not throw an exception
-        m = ModuleHelper.ModuleObject("mymodule2", cl, new String[] { "nullpkg" });
+        m = ModuleHelper.ModuleObject("mymoduleTwo", cl, new String[] { "nullpkg" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, "9.0", "mymodule2/here", null);
+        ModuleHelper.DefineModule(m, "9.0", "mymoduleTwo/here", null);
 
         // Null module argument, expect an NPE
         try {
@@ -160,7 +160,7 @@
             // Expected
         }
 
-        // Bad module name, expect an IAE
+        // Module name with ';', not allowed in java source
         try {
             m = ModuleHelper.ModuleObject("bad;name", cl, new String[] { "mypackage9" });
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage9" });
@@ -169,7 +169,7 @@
             // Expected
         }
 
-        // Bad module name, expect an IAE
+        // Module name with leading dot, not allowed in java source
         try {
             m = ModuleHelper.ModuleObject(".leadingdot", cl, new String[] { "mypackage9a" });
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage9a" });
@@ -178,7 +178,7 @@
             // Expected
         }
 
-        // Bad module name, expect an IAE
+        // Module name with trailing dot, not allowed in java source
         try {
             m = ModuleHelper.ModuleObject("trailingdot.", cl, new String[] { "mypackage9b" });
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage9b" });
@@ -187,11 +187,11 @@
             // Expected
         }
 
-        // Bad module name, expect an IAE
-        m = ModuleHelper.ModuleObject("consecutive..dots", cl, new String[] { "mypackage9c" });
+        // Module name with consecutive dots, not allowed in java source
         try {
-            ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage9c" });
-            throw new RuntimeException("Failed to get expected IAE for consecutive..dots");
+            m = ModuleHelper.ModuleObject("trailingdot.", cl, new String[] { "mypackage9b" });
+            ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "mypackage9b" });
+            throw new RuntimeException("Failed to get expected IAE for trailingdot.");
         } catch(IllegalArgumentException e) {
             // Expected
         }
@@ -207,7 +207,7 @@
         ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { });
 
         // Invalid package name, expect an IAE
-        m = ModuleHelper.ModuleObject("module5", cl, new String[] { "your.package" });
+        m = ModuleHelper.ModuleObject("moduleFive", cl, new String[] { "your.package" });
         try {
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "your.package" });
             throw new RuntimeException("Failed to get expected IAE for your.package");
@@ -218,7 +218,7 @@
         }
 
         // Invalid package name, expect an IAE
-        m = ModuleHelper.ModuleObject("module6", cl, new String[] { "foo" }); // Name irrelevant
+        m = ModuleHelper.ModuleObject("moduleSix", cl, new String[] { "foo" }); // Name irrelevant
         try {
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { ";your/package" });
             throw new RuntimeException("Failed to get expected IAE for ;your.package");
@@ -229,7 +229,7 @@
         }
 
         // Invalid package name, expect an IAE
-        m = ModuleHelper.ModuleObject("module7", cl, new String[] { "foo" }); // Name irrelevant
+        m = ModuleHelper.ModuleObject("moduleSeven", cl, new String[] { "foo" }); // Name irrelevant
         try {
             ModuleHelper.DefineModule(m, "9.0", "module.name/here", new String[] { "7[743" });
             throw new RuntimeException("Failed to get expected IAE for package 7[743");
@@ -240,10 +240,10 @@
         }
 
         // Package named "java" defined to a class loader other than the boot or platform class loader, expect an IAE
-        m = ModuleHelper.ModuleObject("modulejavapkg1", cl, new String[] { "java/foo" });
+        m = ModuleHelper.ModuleObject("modulejavapkgOne", cl, new String[] { "java/foo" });
         try {
             // module m is defined to an instance of MyClassLoader class loader
-            ModuleHelper.DefineModule(m, "9.0", "modulejavapkg1", new String[] { "java/foo" });
+            ModuleHelper.DefineModule(m, "9.0", "modulejavapkgOne", new String[] { "java/foo" });
             throw new RuntimeException("Failed to get expected IAE for package java/foo");
         } catch(IllegalArgumentException e) {
             if (!e.getMessage().contains("prohibited package name")) {
@@ -252,43 +252,43 @@
         }
 
         // Package named "javabar" defined to a class loader other than the boot or platform class loader, should be ok
-        m = ModuleHelper.ModuleObject("modulejavapkg2", cl, new String[] { "javabar" });
+        m = ModuleHelper.ModuleObject("modulejavapkgTwo", cl, new String[] { "javabar" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg2", new String[] { "javabar" });
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkgTwo", new String[] { "javabar" });
 
         // Package named "java" defined to the boot class loader, should be ok
         //   m's type is a java.lang.Object, module is java.base
         //   java.base module is defined to the boot loader
         ClassLoader boot_loader = m.getClass().getClassLoader();
-        m = ModuleHelper.ModuleObject("modulejavapkg3", boot_loader, new String[] { "java/foo" });
+        m = ModuleHelper.ModuleObject("modulejavapkgThree", boot_loader, new String[] { "java/foo" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg3", new String[] { "java/foo" });
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkgThree", new String[] { "java/foo" });
 
         // Package named "java" defined to the platform class loader, should be ok
         //   java.sql module defined to the platform class loader.
         java.sql.Time jst = new java.sql.Time(45 * 1000);
         ClassLoader platform_loader = jst.getClass().getClassLoader();
-        m = ModuleHelper.ModuleObject("modulejavapkg4", platform_loader, new String[] { "java/foo" });
+        m = ModuleHelper.ModuleObject("modulejavapkgFour", platform_loader, new String[] { "java/foo" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, "9.0", "modulejavapkg4", new String[] { "java/foo" });
+        ModuleHelper.DefineModule(m, "9.0", "modulejavapkgFour", new String[] { "java/foo" });
 
         // module version that is null, should be okay
-        m = ModuleHelper.ModuleObject("module8", cl, new String[] { "a_package_8" });
+        m = ModuleHelper.ModuleObject("moduleEight", cl, new String[] { "a_package_8" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, null, "module8/here", new String[] { "a_package_8" });
+        ModuleHelper.DefineModule(m, null, "moduleEight/here", new String[] { "a_package_8" });
 
         // module version that is "", should be okay
-        m = ModuleHelper.ModuleObject("module9", cl, new String[] { "a_package_9" });
+        m = ModuleHelper.ModuleObject("moduleNine", cl, new String[] { "a_package_9" });
         assertNotNull(m, "Module should not be null");
-        ModuleHelper.DefineModule(m, "", "module9/here", new String[] { "a_package_9" });
+        ModuleHelper.DefineModule(m, "", "moduleNine/here", new String[] { "a_package_9" });
 
         // module location that is null, should be okay
-        m = ModuleHelper.ModuleObject("module10", cl, new String[] { "a_package_10" });
+        m = ModuleHelper.ModuleObject("moduleTen", cl, new String[] { "a_package_10" });
         assertNotNull(m, "Module should not be null");
         ModuleHelper.DefineModule(m, "9.0", null, new String[] { "a_package_10" });
 
         // module location that is "", should be okay
-        m = ModuleHelper.ModuleObject("module11", cl, new String[] { "a_package_11" });
+        m = ModuleHelper.ModuleObject("moduleEleven", cl, new String[] { "a_package_11" });
         assertNotNull(m, "Module should not be null");
         ModuleHelper.DefineModule(m, "9.0", "", new String[] { "a_package_11" });
     }
--- a/hotspot/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java	Wed Dec 14 20:23:23 2016 +0000
+++ b/hotspot/test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java	Fri Dec 16 05:46:52 2016 +0000
@@ -49,6 +49,17 @@
 
 public class JvmtiGetAllModulesTest {
 
+    static class MyModuleReference extends ModuleReference {
+        public MyModuleReference(ModuleDescriptor descriptor, URI uri) {
+            super(descriptor, uri);
+        }
+
+        // Trivial implementation to make the class non-abstract
+        public ModuleReader open() {
+            return null;
+        }
+    }
+
     private static native Module[] getModulesNative();
 
     private static Set<Module> getModulesJVMTI() {
@@ -103,11 +114,7 @@
 
             URI uri = URI.create("module:/" + name);
 
-            Supplier<ModuleReader> supplier = () -> {
-                throw new UnsupportedOperationException();
-            };
-
-            ModuleReference mref = new ModuleReference(descriptor, uri, supplier);
+            ModuleReference mref = new MyModuleReference(descriptor, uri);
 
             namesToReference.put(name, mref);
         }