8242452: During module definition, move conversion of packages from native to VM
8242290: Pointless verification in get_package_entry_by_name
Reviewed-by: lfoltan, iklam, hseigel
diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp
index db1a629..80d5fef 100644
--- a/src/hotspot/share/classfile/javaClasses.cpp
+++ b/src/hotspot/share/classfile/javaClasses.cpp
@@ -649,9 +649,14 @@
}
char* java_lang_String::as_utf8_string(oop java_string) {
- typeArrayOop value = java_lang_String::value(java_string);
- int length = java_lang_String::length(java_string, value);
- bool is_latin1 = java_lang_String::is_latin1(java_string);
+ int length;
+ return as_utf8_string(java_string, length);
+}
+
+char* java_lang_String::as_utf8_string(oop java_string, int& length) {
+ typeArrayOop value = java_lang_String::value(java_string);
+ length = java_lang_String::length(java_string, value);
+ bool is_latin1 = java_lang_String::is_latin1(java_string);
if (!is_latin1) {
jchar* position = (length == 0) ? NULL : value->char_at_addr(0);
return UNICODE::as_utf8(position, length);
@@ -661,6 +666,29 @@
}
}
+// Uses a provided buffer if it's sufficiently large, otherwise allocates
+// a resource array to fit
+char* java_lang_String::as_utf8_string_full(oop java_string, char* buf, int buflen, int& utf8_len) {
+ typeArrayOop value = java_lang_String::value(java_string);
+ int len = java_lang_String::length(java_string, value);
+ bool is_latin1 = java_lang_String::is_latin1(java_string);
+ if (!is_latin1) {
+ jchar *position = (len == 0) ? NULL : value->char_at_addr(0);
+ utf8_len = UNICODE::utf8_length(position, len);
+ if (utf8_len >= buflen) {
+ buf = NEW_RESOURCE_ARRAY(char, utf8_len + 1);
+ }
+ return UNICODE::as_utf8(position, len, buf, utf8_len + 1);
+ } else {
+ jbyte *position = (len == 0) ? NULL : value->byte_at_addr(0);
+ utf8_len = UNICODE::utf8_length(position, len);
+ if (utf8_len >= buflen) {
+ buf = NEW_RESOURCE_ARRAY(char, utf8_len + 1);
+ }
+ return UNICODE::as_utf8(position, len, buf, utf8_len + 1);
+ }
+}
+
char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen) {
assert(value_equals(value, java_lang_String::value(java_string)),
"value must be same as java_lang_String::value(java_string)");
diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp
index 1494635..2a28724 100644
--- a/src/hotspot/share/classfile/javaClasses.hpp
+++ b/src/hotspot/share/classfile/javaClasses.hpp
@@ -168,6 +168,8 @@
// String converters
static char* as_utf8_string(oop java_string);
+ static char* as_utf8_string(oop java_string, int& length);
+ static char* as_utf8_string_full(oop java_string, char* buf, int buflen, int& length);
static char* as_utf8_string(oop java_string, char* buf, int buflen);
static char* as_utf8_string(oop java_string, int start, int len);
static char* as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen);
diff --git a/src/hotspot/share/classfile/modules.cpp b/src/hotspot/share/classfile/modules.cpp
index 83b0a40..1b09052 100644
--- a/src/hotspot/share/classfile/modules.cpp
+++ b/src/hotspot/share/classfile/modules.cpp
@@ -40,49 +40,44 @@
#include "logging/log.hpp"
#include "logging/logStream.hpp"
#include "memory/resourceArea.hpp"
-#include "oops/instanceKlass.hpp"
-#include "runtime/arguments.hpp"
#include "runtime/handles.inline.hpp"
#include "runtime/javaCalls.hpp"
#include "runtime/jniHandles.inline.hpp"
-#include "runtime/reflection.hpp"
#include "utilities/stringUtils.hpp"
#include "utilities/utf8.hpp"
-static bool verify_module_name(const char *module_name) {
- if (module_name == NULL) return false;
- int len = (int)strlen(module_name);
+static bool verify_module_name(const char *module_name, int len) {
+ assert(module_name != NULL, "invariant");
return (len > 0 && len <= Symbol::max_length());
}
-bool Modules::verify_package_name(const char* package_name) {
- if (package_name == NULL) return false;
- int len = (int)strlen(package_name);
+static bool verify_package_name(const char* package_name, int len) {
+ assert(package_name != NULL, "Package name derived from non-null jstring can't be NULL");
return (len > 0 && len <= Symbol::max_length() &&
- UTF8::is_legal_utf8((const unsigned char *)package_name, len, false) &&
ClassFileParser::verify_unqualified_name(package_name, len,
ClassFileParser::LegalClass));
}
-static char* get_module_name(oop module, TRAPS) {
+static char* get_module_name(oop module, int& len, TRAPS) {
oop name_oop = java_lang_Module::name(module);
if (name_oop == NULL) {
THROW_MSG_NULL(vmSymbols::java_lang_NullPointerException(), "Null module name");
}
- char* module_name = java_lang_String::as_utf8_string(name_oop);
- if (!verify_module_name(module_name)) {
+ char* module_name = java_lang_String::as_utf8_string(name_oop, len);
+ if (!verify_module_name(module_name, len)) {
THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(),
- err_msg("Invalid module name: %s",
- module_name != NULL ? module_name : "NULL"));
+ err_msg("Invalid module name: %s", module_name));
}
return module_name;
}
-static const char* get_module_version(jstring version) {
- if (version == NULL) {
+static Symbol* as_symbol(jstring str_object) {
+ if (str_object == NULL) {
return NULL;
}
- return java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(version));
+ int len;
+ char* str = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(str_object), len);
+ return SymbolTable::new_symbol(str, len);
}
ModuleEntryTable* Modules::get_module_entry_table(Handle h_loader) {
@@ -100,7 +95,7 @@
}
static ModuleEntry* get_module_entry(jobject module, TRAPS) {
- oop m = JNIHandles::resolve(module);
+ oop m = JNIHandles::resolve_non_null(module);
if (!java_lang_Module::is_instance(m)) {
THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(),
"module is not an instance of type java.lang.Module");
@@ -109,10 +104,10 @@
}
-static PackageEntry* get_locked_package_entry(ModuleEntry* module_entry, const char* package_name, TRAPS) {
+static PackageEntry* get_locked_package_entry(ModuleEntry* module_entry, const char* package_name, int len, TRAPS) {
assert(Module_lock->owned_by_self(), "should have the Module_lock");
assert(package_name != NULL, "Precondition");
- TempNewSymbol pkg_symbol = SymbolTable::new_symbol(package_name);
+ TempNewSymbol pkg_symbol = SymbolTable::new_symbol(package_name, len);
PackageEntryTable* package_entry_table = module_entry->loader_data()->packages();
assert(package_entry_table != NULL, "Unexpected null package entry table");
PackageEntry* package_entry = package_entry_table->locked_lookup_only(pkg_symbol);
@@ -124,13 +119,10 @@
Handle h_loader,
TRAPS) {
if (package != NULL) {
- ResourceMark rm(THREAD);
- if (Modules::verify_package_name(package->as_C_string())) {
- PackageEntryTable* const package_entry_table =
- get_package_entry_table(h_loader);
- assert(package_entry_table != NULL, "Unexpected null package entry table");
- return package_entry_table->lookup_only(package);
- }
+ PackageEntryTable* const package_entry_table =
+ get_package_entry_table(h_loader);
+ assert(package_entry_table != NULL, "Unexpected null package entry table");
+ return package_entry_table->lookup_only(package);
}
return NULL;
}
@@ -140,43 +132,50 @@
return res != NULL;
}
-static void define_javabase_module(jobject module, jstring version,
- jstring location, const char* const* packages,
- jsize num_packages, TRAPS) {
- ResourceMark rm(THREAD);
+// Converts the String oop to an internal package
+// Will use the provided buffer if it's sufficiently large, otherwise allocates
+// a resource array
+// The length of the resulting string will be assigned to utf8_len
+static const char* as_internal_package(oop package_string, char* buf, int buflen, int& utf8_len) {
+ char* package_name = java_lang_String::as_utf8_string_full(package_string, buf, buflen, utf8_len);
- Handle module_handle(THREAD, JNIHandles::resolve(module));
-
- // Obtain java.base's module version
- const char* module_version = get_module_version(version);
- TempNewSymbol version_symbol;
- if (module_version != NULL) {
- version_symbol = SymbolTable::new_symbol(module_version);
- } else {
- version_symbol = NULL;
- }
-
- // Obtain java.base's location
- const char* module_location = NULL;
- TempNewSymbol location_symbol = NULL;
- if (location != NULL) {
- module_location =
- java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(location));
- if (module_location != NULL) {
- location_symbol = SymbolTable::new_symbol(module_location);
+ // Turn all '/'s into '.'s
+ for (int index = 0; index < utf8_len; index++) {
+ if (package_name[index] == JVM_SIGNATURE_DOT) {
+ package_name[index] = JVM_SIGNATURE_SLASH;
}
}
+ return package_name;
+}
+static void define_javabase_module(Handle module_handle, jstring version, jstring location,
+ objArrayHandle pkgs, int num_packages, TRAPS) {
+ ResourceMark rm(THREAD);
+
+ // Obtain java.base's module version
+ TempNewSymbol version_symbol = as_symbol(version);
+
+ // Obtain java.base's location
+ TempNewSymbol location_symbol = as_symbol(location);
// Check that the packages are syntactically ok.
+ char buf[128];
GrowableArray<Symbol*>* pkg_list = new GrowableArray<Symbol*>(num_packages);
for (int x = 0; x < num_packages; x++) {
- const char *package_name = packages[x];
- if (!Modules::verify_package_name(package_name)) {
+ oop pkg_str = pkgs->obj_at(x);
+
+ if (pkg_str == NULL || pkg_str->klass() != SystemDictionary::String_klass()) {
+ THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+ err_msg("Bad package name"));
+ }
+
+ int package_len;
+ const char* package_name = as_internal_package(pkg_str, buf, sizeof(buf), package_len);
+ if (!verify_package_name(package_name, package_len)) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Invalid package name: %s for module: " JAVA_BASE_NAME, package_name));
}
- Symbol* pkg_symbol = SymbolTable::new_symbol(package_name);
+ Symbol* pkg_symbol = SymbolTable::new_symbol(package_name, package_len);
pkg_list->append(pkg_symbol);
}
@@ -237,11 +236,11 @@
ModuleEntryTable::patch_javabase_entries(module_handle);
log_info(module, load)(JAVA_BASE_NAME " location: %s",
- module_location != NULL ? module_location : "NULL");
+ location_symbol != NULL ? location_symbol->as_C_string() : "NULL");
log_debug(module)("define_javabase_module(): Definition of module: "
JAVA_BASE_NAME ", version: %s, location: %s, package #: %d",
- module_version != NULL ? module_version : "NULL",
- module_location != NULL ? module_location : "NULL",
+ version_symbol != NULL ? version_symbol->as_C_string() : "NULL",
+ location_symbol != NULL ? location_symbol->as_C_string() : "NULL",
pkg_list->length());
// packages defined to java.base
@@ -268,45 +267,37 @@
}
void Modules::define_module(jobject module, jboolean is_open, jstring version,
- jstring location, const char* const* packages,
- jsize num_packages, TRAPS) {
+ jstring location, jobjectArray packages, TRAPS) {
ResourceMark rm(THREAD);
if (module == NULL) {
THROW_MSG(vmSymbols::java_lang_NullPointerException(), "Null module object");
}
- if (num_packages < 0) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
- "num_packages must be >= 0");
- }
-
- if (packages == NULL && num_packages > 0) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
- "num_packages should be zero if packages is null");
- }
-
- Handle module_handle(THREAD, JNIHandles::resolve(module));
+ Handle module_handle(THREAD, JNIHandles::resolve_non_null(module));
if (!java_lang_Module::is_instance(module_handle())) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
"module is not an instance of type java.lang.Module");
}
- char* module_name = get_module_name(module_handle(), CHECK);
+ int module_name_len;
+ char* module_name = get_module_name(module_handle(), module_name_len, CHECK);
if (module_name == NULL) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
"Module name cannot be null");
}
+ // Resolve packages
+ objArrayHandle packages_h(THREAD, objArrayOop(JNIHandles::resolve(packages)));
+ int num_packages = (packages_h.is_null() ? 0 : packages_h->length());
+
// Special handling of java.base definition
if (strcmp(module_name, JAVA_BASE_NAME) == 0) {
assert(is_open == JNI_FALSE, "java.base module cannot be open");
- define_javabase_module(module, version, location, packages, num_packages, CHECK);
+ define_javabase_module(module_handle, version, location, packages_h, num_packages, CHECK);
return;
}
- const char* module_version = get_module_version(version);
-
oop loader = java_lang_Module::loader(module_handle());
// Make sure loader is not the jdk.internal.reflect.DelegatingClassLoader.
if (loader != java_lang_ClassLoader::non_reflection_class_loader(loader)) {
@@ -319,20 +310,31 @@
ClassLoaderData* loader_data = SystemDictionary::register_loader(h_loader);
assert(loader_data != NULL, "class loader data shouldn't be null");
+ // Only modules defined to either the boot or platform class loader, can define a "java/" package.
+ bool java_pkg_disallowed = !h_loader.is_null() &&
+ !SystemDictionary::is_platform_class_loader(h_loader());
+
// Check that the list of packages has no duplicates and that the
// packages are syntactically ok.
+ char buf[128];
GrowableArray<Symbol*>* pkg_list = new GrowableArray<Symbol*>(num_packages);
for (int x = 0; x < num_packages; x++) {
- const char* package_name = packages[x];
- if (!verify_package_name(package_name)) {
+ oop pkg_str = packages_h->obj_at(x);
+ if (pkg_str == NULL || pkg_str->klass() != SystemDictionary::String_klass()) {
+ THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
+ err_msg("Bad package name"));
+ }
+
+ int package_len;
+ const char* package_name = as_internal_package(pkg_str, buf, sizeof(buf), package_len);
+ if (!verify_package_name(package_name, package_len)) {
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Invalid package name: %s for module: %s",
package_name, module_name));
}
// Only modules defined to either the boot or platform class loader, can define a "java/" package.
- if (!h_loader.is_null() &&
- !SystemDictionary::is_platform_class_loader(h_loader()) &&
+ if (java_pkg_disallowed &&
(strncmp(package_name, JAVAPKG, JAVAPKG_LEN) == 0 &&
(package_name[JAVAPKG_LEN] == JVM_SIGNATURE_SLASH || package_name[JAVAPKG_LEN] == '\0'))) {
const char* class_loader_name = loader_data->loader_name_and_id();
@@ -348,7 +350,7 @@
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), message);
}
- Symbol* pkg_symbol = SymbolTable::new_symbol(package_name);
+ Symbol* pkg_symbol = SymbolTable::new_symbol(package_name, package_len);
pkg_list->append(pkg_symbol);
}
@@ -356,28 +358,15 @@
assert(module_table != NULL, "module entry table shouldn't be null");
// Create symbol* entry for module name.
- TempNewSymbol module_symbol = SymbolTable::new_symbol(module_name);
+ TempNewSymbol module_symbol = SymbolTable::new_symbol(module_name, module_name_len);
bool dupl_modules = false;
- // Create symbol* entry for module version.
- TempNewSymbol version_symbol;
- if (module_version != NULL) {
- version_symbol = SymbolTable::new_symbol(module_version);
- } else {
- version_symbol = NULL;
- }
+ // Create symbol for module version.
+ TempNewSymbol version_symbol = as_symbol(version);
// Create symbol* entry for module location.
- const char* module_location = NULL;
- TempNewSymbol location_symbol = NULL;
- if (location != NULL) {
- module_location =
- java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(location));
- if (module_location != NULL) {
- location_symbol = SymbolTable::new_symbol(module_location);
- }
- }
+ TempNewSymbol location_symbol = as_symbol(location);
PackageEntryTable* package_table = NULL;
PackageEntry* existing_pkg = NULL;
@@ -439,13 +428,13 @@
}
log_info(module, load)("%s location: %s", module_name,
- module_location != NULL ? module_location : "NULL");
+ location_symbol != NULL ? location_symbol->as_C_string() : "NULL");
LogTarget(Debug, module) lt;
if (lt.is_enabled()) {
LogStream ls(lt);
ls.print("define_module(): creation of module: %s, version: %s, location: %s, ",
- module_name, module_version != NULL ? module_version : "NULL",
- module_location != NULL ? module_location : "NULL");
+ module_name, version_symbol != NULL ? version_symbol->as_C_string() : "NULL",
+ location_symbol != NULL ? location_symbol->as_C_string() : "NULL");
loader_data->print_value_on(&ls);
ls.print_cr(", package #: %d", pkg_list->length());
for (int y = 0; y < pkg_list->length(); y++) {
@@ -486,7 +475,6 @@
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
"Class loader must be the boot class loader");
}
- Handle h_loader(THREAD, loader);
log_debug(module)("set_bootloader_unnamed_module(): recording unnamed module for boot loader");
@@ -499,7 +487,8 @@
java_lang_Module::set_module_entry(module_handle(), unnamed_module);
}
-void Modules::add_module_exports(jobject from_module, const char* package_name, jobject to_module, TRAPS) {
+void Modules::add_module_exports(jobject from_module, jstring package_name, jobject to_module, TRAPS) {
+
if (package_name == NULL) {
THROW_MSG(vmSymbols::java_lang_NullPointerException(),
"package is null");
@@ -529,9 +518,14 @@
}
PackageEntry* package_entry = NULL;
+ char buf[128];
+ int package_len;
+
+ ResourceMark rm(THREAD);
+ const char* pkg = as_internal_package(JNIHandles::resolve_non_null(package_name), buf, sizeof(buf), package_len);
{
MutexLocker ml(THREAD, Module_lock);
- package_entry = get_locked_package_entry(from_module_entry, package_name, CHECK);
+ package_entry = get_locked_package_entry(from_module_entry, pkg, package_len, CHECK);
// Do nothing if modules are the same
// If the package is not found we'll throw an exception later
if (from_module_entry != to_module_entry &&
@@ -542,15 +536,13 @@
// Handle errors and logging outside locked section
if (package_entry == NULL) {
- ResourceMark rm(THREAD);
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package %s not found in from_module %s",
- package_name != NULL ? package_name : "",
+ pkg != NULL ? pkg : "",
from_module_entry->name()->as_C_string()));
}
if (log_is_enabled(Debug, module)) {
- ResourceMark rm(THREAD);
log_debug(module)("add_module_exports(): package %s in module %s is exported to module %s",
package_entry->name()->as_C_string(),
from_module_entry->name()->as_C_string(),
@@ -561,7 +553,7 @@
}
-void Modules::add_module_exports_qualified(jobject from_module, const char* package,
+void Modules::add_module_exports_qualified(jobject from_module, jstring package,
jobject to_module, TRAPS) {
if (to_module == NULL) {
THROW_MSG(vmSymbols::java_lang_NullPointerException(),
@@ -675,7 +667,7 @@
}
// Export package in module to all unnamed modules.
-void Modules::add_module_exports_to_all_unnamed(jobject module, const char* package_name, TRAPS) {
+void Modules::add_module_exports_to_all_unnamed(jobject module, jstring package_name, TRAPS) {
if (module == NULL) {
THROW_MSG(vmSymbols::java_lang_NullPointerException(),
"module is null");
@@ -694,10 +686,14 @@
if (!module_entry->is_named() || module_entry->is_open())
return;
+ ResourceMark rm(THREAD);
+ char buf[128];
+ int pkg_len;
+ const char* pkg = as_internal_package(JNIHandles::resolve_non_null(package_name), buf, sizeof(buf), pkg_len);
PackageEntry* package_entry = NULL;
{
MutexLocker m1(THREAD, Module_lock);
- package_entry = get_locked_package_entry(module_entry, package_name, CHECK);
+ package_entry = get_locked_package_entry(module_entry, pkg, pkg_len, CHECK);
// Mark package as exported to all unnamed modules.
if (package_entry != NULL) {
@@ -707,15 +703,13 @@
// Handle errors and logging outside locked section
if (package_entry == NULL) {
- ResourceMark rm(THREAD);
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
err_msg("Package %s not found in module %s",
- package_name != NULL ? package_name : "",
+ pkg != NULL ? pkg : "",
module_entry->name()->as_C_string()));
}
if (log_is_enabled(Debug, module)) {
- ResourceMark rm(THREAD);
log_debug(module)("add_module_exports_to_all_unnamed(): package %s in module"
" %s is exported to all unnamed modules",
package_entry->name()->as_C_string(),
diff --git a/src/hotspot/share/classfile/modules.hpp b/src/hotspot/share/classfile/modules.hpp
index 1c668eb..434a0cf 100644
--- a/src/hotspot/share/classfile/modules.hpp
+++ b/src/hotspot/share/classfile/modules.hpp
@@ -37,23 +37,20 @@
// define_module defines a module containing the specified packages. It binds the
// module to its class loader by creating the ModuleEntry record in the
// ClassLoader's ModuleEntry table, and creates PackageEntry records in the class
- // loader's PackageEntry table. As in JVM_DefineClass the jstring format for all
- // package names must use "/" and not "."
+ // loader's PackageEntry table. The jstring for all package names will convert "."
+ // to "/"
//
// IllegalArgumentExceptions are thrown for the following :
// * Module's Class loader is not a subclass of java.lang.ClassLoader
// * Module's Class loader already has a module with that name
// * Module's Class loader has already defined types for any of the module's packages
// * Module_name is syntactically bad
- // * Packages contains an illegal package name
+ // * Packages contains an illegal package name or a non-String object
// * A package already exists in another module for this class loader
// * Module is an unnamed module
- // * num_packages is negative
- // * num_packages is non-zero when packages is null
// NullPointerExceptions are thrown if module is null.
static void define_module(jobject module, jboolean is_open, jstring version,
- jstring location, const char* const* packages,
- jsize num_packages, TRAPS);
+ jstring location, jobjectArray packages, TRAPS);
// Provides the java.lang.Module for the unnamed module defined
// to the boot loader.
@@ -67,7 +64,7 @@
// This either does a qualified export of package in module from_module to module
// to_module or, if to_module is null, does an unqualified export of package.
- // The format for the package name must use "/' not ".".
+ // Any "." in the package name will be converted to "/"
//
// Error conditions causing IlegalArgumentException to be throw :
// * Module from_module does not exist
@@ -75,10 +72,10 @@
// * Package is not syntactically correct
// * Package is not defined for from_module's class loader
// * Package is not in module from_module.
- static void add_module_exports(jobject from_module, const char* package, jobject to_module, TRAPS);
+ static void add_module_exports(jobject from_module, jstring package, jobject to_module, TRAPS);
// This does a qualified export of package in module from_module to module
- // to_module. The format for the package name must use "/' not ".".
+ // to_module. Any "." in the package name will be converted to "/"
//
// Error conditions causing IlegalArgumentException to be throw :
// * Module from_module does not exist
@@ -86,7 +83,7 @@
// * Package is not syntactically correct
// * Package is not defined for from_module's class loader
// * Package is not in module from_module.
- static void add_module_exports_qualified(jobject from_module, const char* package, jobject to_module, TRAPS);
+ static void add_module_exports_qualified(jobject from_module, jstring package, jobject to_module, TRAPS);
// add_reads_module adds module to_module to the list of modules that from_module
// can read. If from_module is the same as to_module then this is a no-op.
@@ -111,10 +108,7 @@
// If either module or package is null then NullPointerException is thrown.
// If module or package is bad, or module is unnamed, or package is not in
// module then IllegalArgumentException is thrown.
- static void add_module_exports_to_all_unnamed(jobject module, const char* package, TRAPS);
-
- // Return TRUE if package_name is syntactically valid, false otherwise.
- static bool verify_package_name(const char *package_name);
+ static void add_module_exports_to_all_unnamed(jobject module, jstring package, TRAPS);
// Return TRUE iff package is defined by loader
static bool is_package_defined(Symbol* package_name, Handle h_loader, TRAPS);
diff --git a/src/hotspot/share/include/jvm.h b/src/hotspot/share/include/jvm.h
index 811041a..a1b693e 100644
--- a/src/hotspot/share/include/jvm.h
+++ b/src/hotspot/share/include/jvm.h
@@ -399,12 +399,11 @@
* is_open: specifies if module is open (currently ignored)
* version: the module version
* location: the module location
- * packages: list of packages in the module
- * num_packages: number of packages in the module
+ * packages: array of packages in the module
*/
JNIEXPORT void JNICALL
JVM_DefineModule(JNIEnv *env, jobject module, jboolean is_open, jstring version,
- jstring location, const char* const* packages, jsize num_packages);
+ jstring location, jobjectArray packages);
/*
* Set the boot loader's unnamed module.
@@ -420,7 +419,7 @@
* to_module: module to export the package to
*/
JNIEXPORT void JNICALL
-JVM_AddModuleExports(JNIEnv *env, jobject from_module, const char* package, jobject to_module);
+JVM_AddModuleExports(JNIEnv *env, jobject from_module, jstring package, jobject to_module);
/*
* Do an export of a package to all unnamed modules.
@@ -428,7 +427,7 @@
* package: name of the package to export to all unnamed modules
*/
JNIEXPORT void JNICALL
-JVM_AddModuleExportsToAllUnnamed(JNIEnv *env, jobject from_module, const char* package);
+JVM_AddModuleExportsToAllUnnamed(JNIEnv *env, jobject from_module, jstring package);
/*
* Do an unqualified export of a package.
@@ -436,7 +435,7 @@
* package: name of the package to export
*/
JNIEXPORT void JNICALL
-JVM_AddModuleExportsToAll(JNIEnv *env, jobject from_module, const char* package);
+JVM_AddModuleExportsToAll(JNIEnv *env, jobject from_module, jstring package);
/*
* Add a module to the list of modules that a given module can read.
diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp
index 6085177..2245e5e 100644
--- a/src/hotspot/share/prims/jvm.cpp
+++ b/src/hotspot/share/prims/jvm.cpp
@@ -1058,9 +1058,9 @@
// Module support //////////////////////////////////////////////////////////////////////////////
JVM_ENTRY(void, JVM_DefineModule(JNIEnv *env, jobject module, jboolean is_open, jstring version,
- jstring location, const char* const* packages, jsize num_packages))
+ jstring location, jobjectArray packages))
JVMWrapper("JVM_DefineModule");
- Modules::define_module(module, is_open, version, location, packages, num_packages, CHECK);
+ Modules::define_module(module, is_open, version, location, packages, CHECK);
JVM_END
JVM_ENTRY(void, JVM_SetBootLoaderUnnamedModule(JNIEnv *env, jobject module))
@@ -1068,17 +1068,17 @@
Modules::set_bootloader_unnamed_module(module, CHECK);
JVM_END
-JVM_ENTRY(void, JVM_AddModuleExports(JNIEnv *env, jobject from_module, const char* package, jobject to_module))
+JVM_ENTRY(void, JVM_AddModuleExports(JNIEnv *env, jobject from_module, jstring package, jobject to_module))
JVMWrapper("JVM_AddModuleExports");
Modules::add_module_exports_qualified(from_module, package, to_module, CHECK);
JVM_END
-JVM_ENTRY(void, JVM_AddModuleExportsToAllUnnamed(JNIEnv *env, jobject from_module, const char* package))
+JVM_ENTRY(void, JVM_AddModuleExportsToAllUnnamed(JNIEnv *env, jobject from_module, jstring package))
JVMWrapper("JVM_AddModuleExportsToAllUnnamed");
Modules::add_module_exports_to_all_unnamed(from_module, package, CHECK);
JVM_END
-JVM_ENTRY(void, JVM_AddModuleExportsToAll(JNIEnv *env, jobject from_module, const char* package))
+JVM_ENTRY(void, JVM_AddModuleExportsToAll(JNIEnv *env, jobject from_module, jstring package))
JVMWrapper("JVM_AddModuleExportsToAll");
Modules::add_module_exports(from_module, package, NULL, CHECK);
JVM_END
diff --git a/src/hotspot/share/prims/jvmtiEnv.cpp b/src/hotspot/share/prims/jvmtiEnv.cpp
index 2514a8b..a9aaa76 100644
--- a/src/hotspot/share/prims/jvmtiEnv.cpp
+++ b/src/hotspot/share/prims/jvmtiEnv.cpp
@@ -214,7 +214,6 @@
JvmtiEnv::GetNamedModule(jobject class_loader, const char* package_name, jobject* module_ptr) {
JavaThread* THREAD = JavaThread::current(); // pass to macros
ResourceMark rm(THREAD);
-
Handle h_loader (THREAD, JNIHandles::resolve(class_loader));
// Check that loader is a subclass of java.lang.ClassLoader.
if (h_loader.not_null() && !java_lang_ClassLoader::is_subclass(h_loader->klass())) {
diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp
index d5ffb46..8c5d8d9 100644
--- a/src/hotspot/share/prims/whitebox.cpp
+++ b/src/hotspot/share/prims/whitebox.cpp
@@ -1716,52 +1716,19 @@
WB_ENTRY(void, WB_DefineModule(JNIEnv* env, jobject o, jobject module, jboolean is_open,
jstring version, jstring location, jobjectArray packages))
- ResourceMark rm(THREAD);
-
- objArrayOop packages_oop = objArrayOop(JNIHandles::resolve(packages));
- objArrayHandle packages_h(THREAD, packages_oop);
- int num_packages = (packages_h == NULL ? 0 : packages_h->length());
-
- char** pkgs = NULL;
- if (num_packages > 0) {
- pkgs = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char*, num_packages);
- for (int x = 0; x < num_packages; x++) {
- oop pkg_str = packages_h->obj_at(x);
- if (pkg_str == NULL || !pkg_str->is_a(SystemDictionary::String_klass())) {
- THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
- err_msg("Bad package name"));
- }
- pkgs[x] = java_lang_String::as_utf8_string(pkg_str);
- }
- }
- Modules::define_module(module, is_open, version, location, (const char* const*)pkgs, num_packages, CHECK);
+ Modules::define_module(module, is_open, version, location, packages, CHECK);
WB_END
WB_ENTRY(void, WB_AddModuleExports(JNIEnv* env, jobject o, jobject from_module, jstring package, jobject to_module))
- ResourceMark rm(THREAD);
- char* package_name = NULL;
- if (package != NULL) {
- package_name = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(package));
- }
- Modules::add_module_exports_qualified(from_module, package_name, to_module, CHECK);
+ Modules::add_module_exports_qualified(from_module, package, to_module, CHECK);
WB_END
WB_ENTRY(void, WB_AddModuleExportsToAllUnnamed(JNIEnv* env, jobject o, jclass module, jstring package))
- ResourceMark rm(THREAD);
- char* package_name = NULL;
- if (package != NULL) {
- package_name = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(package));
- }
- Modules::add_module_exports_to_all_unnamed(module, package_name, CHECK);
+ Modules::add_module_exports_to_all_unnamed(module, package, CHECK);
WB_END
WB_ENTRY(void, WB_AddModuleExportsToAll(JNIEnv* env, jobject o, jclass module, jstring package))
- ResourceMark rm(THREAD);
- char* package_name = NULL;
- if (package != NULL) {
- package_name = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(package));
- }
- Modules::add_module_exports(module, package_name, NULL, CHECK);
+ Modules::add_module_exports(module, package, NULL, CHECK);
WB_END
WB_ENTRY(void, WB_AddReadsModule(JNIEnv* env, jobject o, jobject from_module, jobject source_module))
diff --git a/src/java.base/share/native/libjava/Module.c b/src/java.base/share/native/libjava/Module.c
index f1c13ad..9882398 100644
--- a/src/java.base/share/native/libjava/Module.c
+++ b/src/java.base/share/native/libjava/Module.c
@@ -31,81 +31,12 @@
#include "java_lang_Module.h"
-/*
- * Gets the UTF-8 chars for the string and translates '.' to '/'. Does no
- * further validation, assumption being that both calling code in
- * java.lang.Module and VM will do deeper validation.
- */
-static char*
-GetInternalPackageName(JNIEnv *env, jstring pkg, char* buf, jsize buf_size)
-{
- jsize len;
- jsize unicode_len;
- char* p;
- char* utf_str;
-
- len = (*env)->GetStringUTFLength(env, pkg);
- unicode_len = (*env)->GetStringLength(env, pkg);
- if (len >= buf_size) {
- utf_str = malloc(len + 1);
- if (utf_str == NULL) {
- JNU_ThrowOutOfMemoryError(env, NULL);
- return NULL;
- }
- } else {
- utf_str = buf;
- }
- (*env)->GetStringUTFRegion(env, pkg, 0, unicode_len, utf_str);
-
- p = utf_str;
- while (*p != '\0') {
- if (*p == '.') {
- *p = '/';
- }
- p++;
- }
- return utf_str;
-}
-
JNIEXPORT void JNICALL
Java_java_lang_Module_defineModule0(JNIEnv *env, jclass cls, jobject module,
jboolean is_open, jstring version,
jstring location, jobjectArray packages)
{
- char** pkgs = NULL;
- jsize num_packages = (*env)->GetArrayLength(env, packages);
-
- if (num_packages != 0 && (pkgs = calloc(num_packages, sizeof(char*))) == NULL) {
- JNU_ThrowOutOfMemoryError(env, NULL);
- return;
- } else if ((*env)->EnsureLocalCapacity(env, (jint)num_packages) == 0) {
- jboolean failed = JNI_FALSE;
- int idx;
- for (idx = 0; idx < num_packages; idx++) {
- jstring pkg = (*env)->GetObjectArrayElement(env, packages, idx);
- char* name = GetInternalPackageName(env, pkg, NULL, 0);
- if (name != NULL) {
- pkgs[idx] = name;
- } else {
- failed = JNI_TRUE;
- break;
- }
- }
- if (!failed) {
- JVM_DefineModule(env, module, is_open, version, location,
- (const char* const*)pkgs, num_packages);
- }
- }
-
- if (num_packages > 0) {
- int idx;
- for (idx = 0; idx < num_packages; idx++) {
- if (pkgs[idx] != NULL) {
- free(pkgs[idx]);
- }
- }
- free(pkgs);
- }
+ JVM_DefineModule(env, module, is_open, version, location, packages);
}
JNIEXPORT void JNICALL
@@ -118,61 +49,19 @@
Java_java_lang_Module_addExports0(JNIEnv *env, jclass cls, jobject from,
jstring pkg, jobject to)
{
- char buf[128];
- char* pkg_name;
-
- if (pkg == NULL) {
- JNU_ThrowNullPointerException(env, "package is null");
- return;
- }
-
- pkg_name = GetInternalPackageName(env, pkg, buf, (jsize)sizeof(buf));
- if (pkg_name != NULL) {
- JVM_AddModuleExports(env, from, pkg_name, to);
- if (pkg_name != buf) {
- free(pkg_name);
- }
- }
+ JVM_AddModuleExports(env, from, pkg, to);
}
JNIEXPORT void JNICALL
Java_java_lang_Module_addExportsToAll0(JNIEnv *env, jclass cls, jobject from,
jstring pkg)
{
- char buf[128];
- char* pkg_name;
-
- if (pkg == NULL) {
- JNU_ThrowNullPointerException(env, "package is null");
- return;
- }
-
- pkg_name = GetInternalPackageName(env, pkg, buf, (jsize)sizeof(buf));
- if (pkg_name != NULL) {
- JVM_AddModuleExportsToAll(env, from, pkg_name);
- if (pkg_name != buf) {
- free(pkg_name);
- }
- }
+ JVM_AddModuleExportsToAll(env, from, pkg);
}
JNIEXPORT void JNICALL
Java_java_lang_Module_addExportsToAllUnnamed0(JNIEnv *env, jclass cls,
jobject from, jstring pkg)
{
- char buf[128];
- char* pkg_name;
-
- if (pkg == NULL) {
- JNU_ThrowNullPointerException(env, "package is null");
- return;
- }
-
- pkg_name = GetInternalPackageName(env, pkg, buf, (jsize)sizeof(buf));
- if (pkg_name != NULL) {
- JVM_AddModuleExportsToAllUnnamed(env, from, pkg_name);
- if (pkg_name != buf) {
- free(pkg_name);
- }
- }
+ JVM_AddModuleExportsToAllUnnamed(env, from, pkg);
}
diff --git a/test/hotspot/jtreg/runtime/modules/JVMAddModuleExports.java b/test/hotspot/jtreg/runtime/modules/JVMAddModuleExports.java
index 5681e7e..b93f9cd 100644
--- a/test/hotspot/jtreg/runtime/modules/JVMAddModuleExports.java
+++ b/test/hotspot/jtreg/runtime/modules/JVMAddModuleExports.java
@@ -127,13 +127,8 @@
ModuleHelper.AddModuleExports(from_module, "x/apackage", to_module);
ModuleHelper.AddModuleExports(from_module, "x/apackage", to_module);
- // Export a package, using '.' instead of '/'
- try {
- ModuleHelper.AddModuleExports(from_module, "x.apackage", to_module);
- throw new RuntimeException("Failed to get the expected IAE");
- } catch(IllegalArgumentException e) {
- // Expected
- }
+ // Export the same package, using '.' instead of '/'
+ ModuleHelper.AddModuleExports(from_module, "x.apackage", to_module);
// Export a package to the unnamed module and then to a specific module.
// The qualified export should be ignored.
diff --git a/test/hotspot/jtreg/runtime/modules/JVMDefineModule.java b/test/hotspot/jtreg/runtime/modules/JVMDefineModule.java
index f3407f1..6e70dfc 100644
--- a/test/hotspot/jtreg/runtime/modules/JVMDefineModule.java
+++ b/test/hotspot/jtreg/runtime/modules/JVMDefineModule.java
@@ -195,16 +195,10 @@
assertNotNull(m, "Module should not be null");
ModuleHelper.DefineModule(m, false, "9.0", "module.name/here", new String[] { });
- // Invalid package name, expect an IAE
+ // Package name with dots, should be okay
m = ModuleHelper.ModuleObject("moduleFive", cl, new String[] { "your.apackage" });
- try {
- ModuleHelper.DefineModule(m, false, "9.0", "module.name/here", new String[] { "your.apackage" });
- throw new RuntimeException("Failed to get expected IAE for your.apackage");
- } catch(IllegalArgumentException e) {
- if (!e.getMessage().contains("Invalid package name")) {
- throw new RuntimeException("Failed to get expected IAE message for bad package name: " + e.getMessage());
- }
- }
+ assertNotNull(m, "Module should not be null");
+ ModuleHelper.DefineModule(m, false, "9.0", "moduleFive", new String[] { "your.apackage" });
// Invalid package name, expect an IAE
m = ModuleHelper.ModuleObject("moduleSix", cl, new String[] { "foo" }); // Name irrelevant
@@ -280,6 +274,38 @@
m = ModuleHelper.ModuleObject("moduleEleven", cl, new String[] { "a_package_11" });
assertNotNull(m, "Module should not be null");
ModuleHelper.DefineModule(m, false, "9.0", "", new String[] { "a_package_11" });
+
+ // module with very long package names, should be okay
+ String[] longPackageNames = new String[] {
+ "mypackage/mypackage/mypackage/mypackage1",
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage2",
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage/" +
+ "mypackage/mypackage/mypackage/mypackage3"
+ };
+ m = ModuleHelper.ModuleObject("moduleTwelve", cl, longPackageNames);
+ assertNotNull(m, "Module should not be null");
+ ModuleHelper.DefineModule(m, false, "9.0", "moduleTwelve", longPackageNames);
+ // Indirectly test that the package names doesn't get truncated when defined
+ for (int i = 0; i < longPackageNames.length; i++) {
+ ModuleHelper.AddModuleExportsToAllUnnamed(m, longPackageNames[i]);
+ }
}
static class MyClassLoader extends ClassLoader { }
