8231606: _method_ordering is not set during CDS dynamic dump time

Add the missing DynamicDumpSharedSpaces check in sort_methods(); replace the (DumpSharedSpaces || DynamicDumpSharedSpaces) with the Arguments::is_dumping_archive() function call.

Reviewed-by: iklam, coleenp, jiangli
This commit is contained in:
Calvin Cheung 2019-10-02 16:55:08 -07:00
parent 72c2079fd0
commit b08a8c5cc3
24 changed files with 73 additions and 54 deletions

View File

@ -3004,7 +3004,7 @@ static const intArray* sort_methods(Array<Method*>* methods) {
// We temporarily use the vtable_index field in the Method* to store the
// class file index, so we can read in after calling qsort.
// Put the method ordering in the shared archive.
if (JvmtiExport::can_maintain_original_method_order() || DumpSharedSpaces) {
if (JvmtiExport::can_maintain_original_method_order() || Arguments::is_dumping_archive()) {
for (int index = 0; index < length; index++) {
Method* const m = methods->at(index);
assert(!m->valid_vtable_index(), "vtable index should not be set");
@ -3018,7 +3018,7 @@ static const intArray* sort_methods(Array<Method*>* methods) {
intArray* method_ordering = NULL;
// If JVMTI original method ordering or sharing is enabled construct int
// array remembering the original ordering
if (JvmtiExport::can_maintain_original_method_order() || DumpSharedSpaces) {
if (JvmtiExport::can_maintain_original_method_order() || Arguments::is_dumping_archive()) {
method_ordering = new intArray(length, length, -1);
for (int index = 0; index < length; index++) {
Method* const m = methods->at(index);

View File

@ -462,7 +462,7 @@ bool ClassPathImageEntry::is_modules_image() const {
#if INCLUDE_CDS
void ClassLoader::exit_with_path_failure(const char* error, const char* message) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "only called at dump time");
Arguments::assert_is_dumping_archive();
tty->print_cr("Hint: enable -Xlog:class+path=info to diagnose the failure");
vm_exit_during_initialization(error, message);
}
@ -532,7 +532,7 @@ void ClassLoader::setup_bootstrap_search_path() {
#if INCLUDE_CDS
void ClassLoader::setup_app_search_path(const char *class_path) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "Sanity");
Arguments::assert_is_dumping_archive();
ResourceMark rm;
ClasspathStream cp_stream(class_path);
@ -546,7 +546,7 @@ void ClassLoader::setup_app_search_path(const char *class_path) {
void ClassLoader::add_to_module_path_entries(const char* path,
ClassPathEntry* entry) {
assert(entry != NULL, "ClassPathEntry should not be NULL");
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
// The entry does not exist, add to the list
if (_module_path_entries == NULL) {
@ -560,7 +560,7 @@ void ClassLoader::add_to_module_path_entries(const char* path,
// Add a module path to the _module_path_entries list.
void ClassLoader::update_module_path_entry_list(const char *path, TRAPS) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
struct stat st;
if (os::stat(path, &st) != 0) {
tty->print_cr("os::stat error %d (%s). CDS dump aborted (path was \"%s\").",
@ -656,7 +656,7 @@ void ClassLoader::setup_boot_search_path(const char *class_path) {
bool set_base_piece = true;
#if INCLUDE_CDS
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
if (!Arguments::has_jimage()) {
vm_exit_during_initialization("CDS is not supported in exploded JDK build", NULL);
}
@ -1360,7 +1360,7 @@ char* ClassLoader::skip_uri_protocol(char* source) {
// Record the shared classpath index and loader type for classes loaded
// by the builtin loaders at dump time.
void ClassLoader::record_result(InstanceKlass* ik, const ClassFileStream* stream, TRAPS) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "sanity");
Arguments::assert_is_dumping_archive();
assert(stream != NULL, "sanity");
if (ik->is_unsafe_anonymous()) {
@ -1537,13 +1537,13 @@ void ClassLoader::initialize() {
#if INCLUDE_CDS
void ClassLoader::initialize_shared_path() {
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
ClassLoaderExt::setup_search_paths();
}
}
void ClassLoader::initialize_module_path(TRAPS) {
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
ClassLoaderExt::setup_module_paths(THREAD);
FileMapInfo::allocate_shared_path_table();
}

View File

@ -26,6 +26,7 @@
#define SHARE_CLASSFILE_CLASSLOADER_HPP
#include "jimage.hpp"
#include "runtime/arguments.hpp"
#include "runtime/handles.hpp"
#include "runtime/perfData.hpp"
#include "utilities/exceptions.hpp"
@ -395,8 +396,7 @@ class ClassLoader: AllStatic {
// Helper function used by CDS code to get the number of module path
// entries during shared classpath setup time.
static int num_module_path_entries() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"Should only be called at CDS dump time");
Arguments::assert_is_dumping_archive();
int num_entries = 0;
ClassPathEntry* e= ClassLoader::_module_path_entries;
while (e != NULL) {

View File

@ -62,8 +62,7 @@ inline ClassPathEntry* ClassLoader::classpath_entry(int n) {
// entries during shared classpath setup time.
inline int ClassLoader::num_boot_classpath_entries() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"Should only be called at CDS dump time");
Arguments::assert_is_dumping_archive();
assert(has_jrt_entry(), "must have a java runtime image");
int num_entries = 1; // count the runtime image
ClassPathEntry* e = ClassLoader::_first_append_entry;
@ -85,8 +84,7 @@ inline ClassPathEntry* ClassLoader::get_next_boot_classpath_entry(ClassPathEntry
// Helper function used by CDS code to get the number of app classpath
// entries during shared classpath setup time.
inline int ClassLoader::num_app_classpath_entries() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"Should only be called at CDS dump time");
Arguments::assert_is_dumping_archive();
int num_entries = 0;
ClassPathEntry* e= ClassLoader::_app_classpath_entries;
while (e != NULL) {

View File

@ -62,8 +62,7 @@ void ClassLoaderExt::append_boot_classpath(ClassPathEntry* new_entry) {
}
void ClassLoaderExt::setup_app_search_path() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"this function is only used at CDS dump time");
Arguments::assert_is_dumping_archive();
_app_class_paths_start_index = ClassLoader::num_boot_classpath_entries();
char* app_class_path = os::strdup(Arguments::get_appclasspath());
@ -92,8 +91,7 @@ void ClassLoaderExt::process_module_table(ModuleEntryTable* met, TRAPS) {
}
}
void ClassLoaderExt::setup_module_paths(TRAPS) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"this function is only used with CDS dump time");
Arguments::assert_is_dumping_archive();
_app_module_paths_start_index = ClassLoader::num_boot_classpath_entries() +
ClassLoader::num_app_classpath_entries();
Handle system_class_loader (THREAD, SystemDictionary::java_system_loader());
@ -231,7 +229,7 @@ void ClassLoaderExt::setup_search_paths() {
void ClassLoaderExt::record_result(const s2 classpath_index,
InstanceKlass* result,
TRAPS) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "Sanity");
Arguments::assert_is_dumping_archive();
// We need to remember where the class comes from during dumping.
oop loader = result->class_loader();

View File

@ -42,7 +42,7 @@
//
CompactHashtableWriter::CompactHashtableWriter(int num_entries,
CompactHashtableStats* stats) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump-time only");
Arguments::assert_is_dumping_archive();
assert(num_entries >= 0, "sanity");
_num_buckets = calculate_num_buckets(num_entries);
assert(_num_buckets > 0, "no buckets");

View File

@ -246,7 +246,7 @@ void Dictionary::all_entries_do(KlassClosure* closure) {
// Used to scan and relocate the classes during CDS archive dump.
void Dictionary::classes_do(MetaspaceClosure* it) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump-time only");
Arguments::assert_is_dumping_archive();
for (int index = 0; index < table_size(); index++) {
for (DictionaryEntry* probe = bucket(index);
probe != NULL;

View File

@ -218,7 +218,7 @@ InstanceKlass* KlassFactory::create_from_stream(ClassFileStream* stream,
JFR_ONLY(ON_KLASS_CREATION(result, parser, THREAD);)
#if INCLUDE_CDS
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
ClassLoader::record_result(result, stream, THREAD);
}
#endif // INCLUDE_CDS

View File

@ -220,7 +220,7 @@ Symbol* SymbolTable::allocate_symbol(const char* name, int len, bool c_heap) {
assert (len <= Symbol::max_length(), "should be checked by caller");
Symbol* sym;
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
c_heap = false;
}
if (c_heap) {
@ -283,7 +283,7 @@ public:
};
void SymbolTable::metaspace_pointers_do(MetaspaceClosure* it) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "called only during dump time");
Arguments::assert_is_dumping_archive();
MetaspacePointersDo mpd(it);
_local_table->do_safepoint_scan(mpd);
}

View File

@ -1029,7 +1029,7 @@ DumpTimeSharedClassInfo* SystemDictionaryShared::find_or_allocate_info_for(Insta
}
void SystemDictionaryShared::set_shared_class_misc_info(InstanceKlass* k, ClassFileStream* cfs) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "only when dumping");
Arguments::assert_is_dumping_archive();
assert(!is_builtin(k), "must be unregistered class");
DumpTimeSharedClassInfo* info = find_or_allocate_info_for(k);
info->_clsfile_size = cfs->length();
@ -1185,7 +1185,7 @@ void SystemDictionaryShared::check_excluded_classes() {
bool SystemDictionaryShared::is_excluded_class(InstanceKlass* k) {
assert(_no_class_loading_should_happen, "sanity");
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "only when dumping");
Arguments::assert_is_dumping_archive();
return find_or_allocate_info_for(k)->is_excluded();
}
@ -1209,7 +1209,7 @@ void SystemDictionaryShared::dumptime_classes_do(class MetaspaceClosure* it) {
bool SystemDictionaryShared::add_verification_constraint(InstanceKlass* k, Symbol* name,
Symbol* from_name, bool from_field_is_protected, bool from_is_array, bool from_is_object) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "called at dump time only");
Arguments::assert_is_dumping_archive();
DumpTimeSharedClassInfo* info = find_or_allocate_info_for(k);
info->add_verification_constraint(k, name, from_name, from_field_is_protected,
from_is_array, from_is_object);

View File

@ -94,7 +94,7 @@ bool VerificationType::is_reference_assignable_from(
return true;
}
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
if (SystemDictionaryShared::add_verification_constraint(klass,
name(), from.name(), from_field_is_protected, from.is_array(),
from.is_object())) {

View File

@ -168,7 +168,7 @@ static void log_jdk_jfr_module_resolution_error(TRAPS) {
static bool is_cds_dump_requested() {
// we will not be able to launch recordings if a cds dump is being requested
if ((DumpSharedSpaces || DynamicDumpSharedSpaces) && (JfrOptionSet::startup_recording_options() != NULL)) {
if (Arguments::is_dumping_archive() && (JfrOptionSet::startup_recording_options() != NULL)) {
warning("JFR will be disabled during CDS dumping");
teardown_startup_support();
return true;

View File

@ -263,7 +263,7 @@ void SharedClassPathEntry::init_as_non_existent(const char* path, TRAPS) {
void SharedClassPathEntry::init(bool is_modules_image,
ClassPathEntry* cpe, TRAPS) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
_timestamp = 0;
_filesize = 0;
_from_class_path_attr = false;
@ -397,7 +397,7 @@ void SharedPathTable::dumptime_init(ClassLoaderData* loader_data, Thread* THREAD
}
void FileMapInfo::allocate_shared_path_table() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "Sanity");
Arguments::assert_is_dumping_archive();
EXCEPTION_MARK; // The following calls should never throw, but would exit VM on error.
ClassLoaderData* loader_data = ClassLoaderData::the_null_class_loader_data();
@ -444,7 +444,7 @@ int FileMapInfo::add_shared_classpaths(int i, const char* which, ClassPathEntry
}
void FileMapInfo::check_nonempty_dir_in_shared_path_table() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
bool has_nonempty_dir = false;
@ -471,7 +471,7 @@ void FileMapInfo::check_nonempty_dir_in_shared_path_table() {
}
void FileMapInfo::record_non_existent_class_path_entry(const char* path) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
log_info(class, path)("non-existent Class-Path entry %s", path);
if (_non_existent_class_paths == NULL) {
_non_existent_class_paths = new (ResourceObj::C_HEAP, mtInternal)GrowableArray<const char*>(10, true);
@ -480,7 +480,7 @@ void FileMapInfo::record_non_existent_class_path_entry(const char* path) {
}
int FileMapInfo::num_non_existent_class_paths() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
if (_non_existent_class_paths != NULL) {
return _non_existent_class_paths->length();
} else {
@ -1150,7 +1150,7 @@ void FileMapRegion::init(bool is_heap_region, char* base, size_t size, bool read
void FileMapInfo::write_region(int region, char* base, size_t size,
bool read_only, bool allow_exec) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "Dump time only");
Arguments::assert_is_dumping_archive();
FileMapRegion* si = space_at(region);
char* target_base = base;

View File

@ -425,7 +425,7 @@ void MetaspaceShared::read_extra_data(const char* filename, TRAPS) {
}
void MetaspaceShared::commit_shared_space_to(char* newtop) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump-time only");
Arguments::assert_is_dumping_archive();
char* base = _shared_rs.base();
size_t need_committed_size = newtop - base;
size_t has_committed_size = _shared_vs.committed_size();
@ -509,8 +509,7 @@ address MetaspaceShared::i2i_entry_code_buffers(size_t total_size) {
}
uintx MetaspaceShared::object_delta_uintx(void* obj) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
"supported only for dumping");
Arguments::assert_is_dumping_archive();
if (DumpSharedSpaces) {
assert(shared_rs()->contains(obj), "must be");
} else {

View File

@ -710,7 +710,7 @@ jint universe_init() {
}
#if INCLUDE_CDS
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
MetaspaceShared::prepare_for_dumping();
}
#endif

View File

@ -26,6 +26,7 @@
#define SHARE_OOPS_CONSTMETHOD_HPP
#include "oops/oop.hpp"
#include "runtime/arguments.hpp"
#include "utilities/align.hpp"
// An ConstMethod represents portions of a Java method which are not written to after
@ -293,7 +294,7 @@ public:
_adapter = adapter;
}
void set_adapter_trampoline(AdapterHandlerEntry** trampoline) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "must be");
Arguments::assert_is_dumping_archive();
if (DumpSharedSpaces) {
assert(*trampoline == NULL,
"must be NULL during dump time, to be initialized at run time");

View File

@ -708,7 +708,7 @@ void ConstantPoolCache::remove_unshareable_info() {
}
void ConstantPoolCache::walk_entries_for_initialization(bool check_only) {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "sanity");
Arguments::assert_is_dumping_archive();
// When dumping the archive, we want to clean up the ConstantPoolCache
// to remove any effect of linking due to the execution of Java code --
// each ConstantPoolCacheEntry will have the same contents as if

View File

@ -453,7 +453,7 @@ InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind, Klass
assert(is_instance_klass(), "is layout incorrect?");
assert(size_helper() == parser.layout_size(), "incorrect size_helper?");
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
SystemDictionaryShared::init_dumptime_info(this);
}
}
@ -603,7 +603,7 @@ void InstanceKlass::deallocate_contents(ClassLoaderData* loader_data) {
}
set_annotations(NULL);
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
SystemDictionaryShared::remove_dumptime_info(this);
}
}
@ -2229,7 +2229,7 @@ bool InstanceKlass::should_store_fingerprint(bool is_unsafe_anonymous) {
// (1) We are running AOT to generate a shared library.
return true;
}
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
// (2) We are running -Xshare:dump or -XX:ArchiveClassesAtExit to create a shared archive
return true;
}
@ -2477,7 +2477,7 @@ void InstanceKlass::unload_class(InstanceKlass* ik) {
// notify ClassLoadingService of class unload
ClassLoadingService::notify_class_unloaded(ik);
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
SystemDictionaryShared::remove_dumptime_info(ik);
}

View File

@ -525,7 +525,7 @@ void Klass::metaspace_pointers_do(MetaspaceClosure* it) {
}
void Klass::remove_unshareable_info() {
assert (DumpSharedSpaces || DynamicDumpSharedSpaces,
assert (Arguments::is_dumping_archive(),
"only called during CDS dump time");
JFR_ONLY(REMOVE_ID(this);)
if (log_is_enabled(Trace, cds, unshareable)) {
@ -543,7 +543,7 @@ void Klass::remove_unshareable_info() {
}
void Klass::remove_java_mirror() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "only called during CDS dump time");
Arguments::assert_is_dumping_archive();
if (log_is_enabled(Trace, cds, unshareable)) {
ResourceMark rm;
log_trace(cds, unshareable)("remove java_mirror: %s", external_name());

View File

@ -979,7 +979,7 @@ void Method::unlink_code() {
void Method::unlink_method() {
_code = NULL;
assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "dump time only");
Arguments::assert_is_dumping_archive();
// Set the values to what they should be at run time. Note that
// this Method can no longer be executed during dump time.
_i2i_entry = Interpreter::entry_for_cds_method(this);

View File

@ -1454,7 +1454,7 @@ const char* unsupported_options[] = { "--limit-modules",
"--patch-module"
};
void Arguments::check_unsupported_dumping_properties() {
assert(DumpSharedSpaces || DynamicDumpSharedSpaces,
assert(is_dumping_archive(),
"this function is only used with CDS dump time");
assert(ARRAY_SIZE(unsupported_properties) == ARRAY_SIZE(unsupported_options), "must be");
// If a vm option is found in the unsupported_options array, vm will exit with an error message.
@ -3537,7 +3537,7 @@ bool Arguments::init_shared_archive_paths() {
SharedArchivePath = get_default_shared_archive_path();
} else {
int archives = num_archives(SharedArchiveFile);
if (DynamicDumpSharedSpaces || DumpSharedSpaces) {
if (is_dumping_archive()) {
if (archives > 1) {
vm_exit_during_initialization(
"Cannot have more than 1 archive file specified in -XX:SharedArchiveFile during CDS dumping");
@ -3550,7 +3550,7 @@ bool Arguments::init_shared_archive_paths() {
}
}
}
if (!DynamicDumpSharedSpaces && !DumpSharedSpaces){
if (!is_dumping_archive()){
if (archives > 2) {
vm_exit_during_initialization(
"Cannot have more than 2 archive files specified in the -XX:SharedArchiveFile option");

View File

@ -647,6 +647,12 @@ class Arguments : AllStatic {
static bool atojulong(const char *s, julong* result);
static bool has_jfr_option() NOT_JFR_RETURN_(false);
static bool is_dumping_archive() { return DumpSharedSpaces || DynamicDumpSharedSpaces; }
static void assert_is_dumping_archive() {
assert(Arguments::is_dumping_archive(), "dump time only");
}
};
// Disable options not supported in this release, with a warning if they

View File

@ -4185,7 +4185,7 @@ void Threads::create_vm_init_agents() {
for (agent = Arguments::agents(); agent != NULL; agent = agent->next()) {
// CDS dumping does not support native JVMTI agent.
// CDS dumping supports Java agent if the AllowArchivingWithJavaAgent diagnostic option is specified.
if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
if (Arguments::is_dumping_archive()) {
if(!agent->is_instrument_lib()) {
vm_exit_during_cds_dumping("CDS dumping does not support native JVMTI agent, name", agent->name());
} else if (!AllowArchivingWithJavaAgent) {

View File

@ -52,6 +52,9 @@ public class HelloDynamic extends DynamicArchiveTestBase {
doTest(baseArchiveName, topArchiveName);
}
private static final String JDWP_OPTION =
"-Xrunjdwp:transport=dt_socket,server=y,suspend=n";
private static void doTest(String baseArchiveName, String topArchiveName) throws Exception {
String appJar = ClassFileInstaller.getJarPath("hello.jar");
String mainClass = "Hello";
@ -71,5 +74,19 @@ public class HelloDynamic extends DynamicArchiveTestBase {
output.shouldContain("Hello source: shared objects file")
.shouldHaveExitValue(0);
});
// Sanity test with JDWP options.
// Test with the default base archive should be sufficient.
if (baseArchiveName == null) {
run2(baseArchiveName, topArchiveName,
JDWP_OPTION,
"-Xlog:class+load",
"-Xlog:cds+dynamic=debug,cds=debug",
"-cp", appJar, mainClass)
.assertNormalExit(output -> {
output.shouldContain("Hello source: shared objects file")
.shouldHaveExitValue(0);
});
}
}
}