8204620: ModuleEntry::is_non_jdk_module() determination for what is a jdk module is incorrect

Check module's loader and compare version with java.base's version to improve algorithm

Reviewed-by: lfoltan, mchung

src/hotspot/share/classfile/moduleEntry.cpp

@@ -56,15 +56,33 @@ void ModuleEntry::set_location(Symbol* location) {
   }
 }
 
-bool ModuleEntry::is_non_jdk_module() {
-  ResourceMark rm;
+// Return true if the module's version should be displayed in error messages,
+// logging, etc.
+// Return false if the module's version is null, if it is unnamed, or if the
+// module is not an upgradeable module.
+// Detect if the module is not upgradeable by checking:
+//     1. Module location is "jrt:/java." and its loader is boot or platform
+//     2. Module location is "jrt:/jdk.", its loader is one of the builtin loaders
+//        and its version is the same as module java.base's version
+// The above check is imprecise but should work in almost all cases.
+bool ModuleEntry::should_show_version() {
+  if (version() == NULL || !is_named()) return false;
+
   if (location() != NULL) {
+    ResourceMark rm;
     const char* loc = location()->as_C_string();
-    if (strncmp(loc, "jrt:/java.", 10) != 0 && strncmp(loc, "jrt:/jdk.", 9) != 0) {
-      return true;
+    ClassLoaderData* cld = loader_data();
+
+    if ((cld->is_the_null_class_loader_data() || cld->is_platform_class_loader_data()) &&
+        (strncmp(loc, "jrt:/java.", 10) == 0)) {
+      return false;
+    }
+    if ((ModuleEntryTable::javabase_moduleEntry()->version()->fast_compare(version()) == 0) &&
+        cld->is_permanent_class_loader_data() && (strncmp(loc, "jrt:/jdk.", 9) == 0)) {
+      return false;
     }
   }
-  return false;
+  return true;
 }
 
 void ModuleEntry::set_version(Symbol* version) {

src/hotspot/share/classfile/moduleEntry.hpp

@@ -117,7 +117,7 @@ public:
 
   Symbol*          location() const                    { return _location; }
   void             set_location(Symbol* location);
-  bool             is_non_jdk_module();
+  bool             should_show_version();
 
   bool             can_read(ModuleEntry* m) const;
   bool             has_reads_list() const;

src/hotspot/share/oops/klass.cpp

@@ -809,10 +809,10 @@ const char* Klass::class_loader_and_module_name() const {
       module_name = module->name()->as_C_string();
       msglen += strlen(module_name);
       // Use version if exists and is not a jdk module
-      if (module->is_non_jdk_module() && module->version() != NULL) {
+      if (module->should_show_version()) {
         has_version = true;
         version = module->version()->as_C_string();
-        msglen += strlen("@") + strlen(version);
+        msglen += strlen(version) + 1; // +1 for "@"
       }
     }
   } else {

test/hotspot/jtreg/runtime/modules/CCE_module_msg.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, 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
@@ -48,6 +48,8 @@ public class CCE_module_msg {
     public static void main(String[] args) throws Throwable {
         // Should not display version
         invalidObjectToDerived();
+        invalidTimeToDerived();
+        invalidHeadersToDerived();
         // Should display version
         invalidClassToString();
         // Should display customer class loader
@@ -71,6 +73,42 @@ public class CCE_module_msg {
         }
     }
 
+    // Test with a non-upgradeable 'java.' module other than java.base.
+    public static void invalidTimeToDerived() {
+        java.sql.Time instance = new java.sql.Time(10000);
+        int left = 23;
+        int right = 42;
+        try {
+            for (int i = 0; i < 1; i += 1) {
+                left = ((Derived) (java.lang.Object)instance).method(left, right);
+            }
+            throw new RuntimeException("ClassCastException wasn't thrown, test failed.");
+        } catch (ClassCastException cce) {
+            System.out.println(cce.getMessage());
+            if (!cce.getMessage().contains("java.sql/java.sql.Time cannot be cast to Derived")) {
+                throw new RuntimeException("Wrong message: " + cce.getMessage());
+            }
+        }
+    }
+
+    // Test with a non-upgradeable 'jdk.' module.
+    public static void invalidHeadersToDerived() {
+        com.sun.net.httpserver.Headers instance = new com.sun.net.httpserver.Headers();
+        int left = 23;
+        int right = 42;
+        try {
+            for (int i = 0; i < 1; i += 1) {
+                left = ((Derived) (java.lang.Object)instance).method(left, right);
+            }
+            throw new RuntimeException("ClassCastException wasn't thrown, test failed.");
+        } catch (ClassCastException cce) {
+            System.out.println(cce.getMessage());
+            if (!cce.getMessage().contains("jdk.httpserver/com.sun.net.httpserver.Headers cannot be cast to Derived")) {
+                throw new RuntimeException("Wrong message: " + cce.getMessage());
+            }
+        }
+    }
+
     public static void invalidClassToString() throws Throwable {
         // Get the java.lang.Module object for module java.base.
         Class jlObject = Class.forName("java.lang.Object");