From 069006083327ae9a041e98dbe74713c27d529944 Mon Sep 17 00:00:00 2001 From: goetz Date: Wed, 11 Jul 2018 16:11:10 +0200 Subject: [PATCH] 8206977: Minor improvements of runtime code. Reviewed-by: coleenp, lfoltan --- src/hotspot/cpu/x86/vm_version_ext_x86.cpp | 9 +++---- src/hotspot/cpu/x86/vm_version_ext_x86.hpp | 1 + src/hotspot/os/linux/os_linux.cpp | 16 +++++++----- src/hotspot/os/linux/perfMemory_linux.cpp | 5 ++-- src/hotspot/share/classfile/moduleEntry.cpp | 3 ++- .../share/classfile/systemDictionary.cpp | 22 ++++++++-------- .../classfile/systemDictionaryShared.cpp | 5 ++-- src/hotspot/share/classfile/verifier.cpp | 3 ++- src/hotspot/share/logging/logOutput.cpp | 7 ++--- src/hotspot/share/memory/filemap.cpp | 4 ++- src/hotspot/share/memory/metaspace.cpp | 4 ++- src/hotspot/share/memory/virtualspace.cpp | 26 +++++++++---------- src/hotspot/share/prims/jvmtiEnvBase.cpp | 2 +- src/hotspot/share/runtime/flags/jvmFlag.cpp | 4 +++ src/hotspot/share/services/writeableFlags.cpp | 2 +- src/hotspot/share/utilities/ostream.cpp | 3 ++- .../gtest/logging/logTestUtils.inline.hpp | 3 ++- test/hotspot/gtest/memory/test_metachunk.cpp | 4 +-- 18 files changed, 68 insertions(+), 55 deletions(-) diff --git a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp index a5d60aebe2..3cc775d7ec 100644 --- a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp +++ b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp @@ -470,8 +470,8 @@ int VM_Version_Ext::cpu_extended_brand_string(char* const buf, size_t buf_len) { } size_t VM_Version_Ext::cpu_write_support_string(char* const buf, size_t buf_len) { - assert(buf != NULL, "buffer is NULL!"); - assert(buf_len > 0, "buffer len not enough!"); + guarantee(buf != NULL, "buffer is NULL!"); + guarantee(buf_len > 0, "buffer len not enough!"); unsigned int flag = 0; unsigned int fi = 0; @@ -481,8 +481,7 @@ size_t VM_Version_Ext::cpu_write_support_string(char* const buf, size_t buf_len) #define WRITE_TO_BUF(string) \ { \ int res = jio_snprintf(&buf[written], buf_len - written, "%s%s", prefix, string); \ - if (res < 0 || (size_t) res >= buf_len - 1) { \ - buf[buf_len-1] = '\0'; \ + if (res < 0) { \ return buf_len - 1; \ } \ written += res; \ @@ -592,7 +591,7 @@ int VM_Version_Ext::cpu_detailed_description(char* const buf, size_t buf_len) { _cpuid_info.ext_cpuid1_edx); if (outputLen < 0 || (size_t) outputLen >= buf_len - 1) { - buf[buf_len-1] = '\0'; + if (buf_len > 0) { buf[buf_len-1] = '\0'; } return OS_ERR; } diff --git a/src/hotspot/cpu/x86/vm_version_ext_x86.hpp b/src/hotspot/cpu/x86/vm_version_ext_x86.hpp index de35616b6e..78fb001cb3 100644 --- a/src/hotspot/cpu/x86/vm_version_ext_x86.hpp +++ b/src/hotspot/cpu/x86/vm_version_ext_x86.hpp @@ -63,6 +63,7 @@ class VM_Version_Ext : public VM_Version { static bool cpu_is_em64t(void); static bool is_netburst(void); + // Returns bytes written excluding termninating null byte. static size_t cpu_write_support_string(char* const buf, size_t buf_len); static void resolve_cpu_information_details(void); static jlong max_qualified_cpu_freq_from_brand_string(void); diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp index c48888e678..bc1f3d45dc 100644 --- a/src/hotspot/os/linux/os_linux.cpp +++ b/src/hotspot/os/linux/os_linux.cpp @@ -2108,7 +2108,9 @@ void os::get_summary_os_info(char* buf, size_t buflen) { // special case for debian if (file_exists("/etc/debian_version")) { strncpy(buf, "Debian ", buflen); - parse_os_info(&buf[7], buflen-7, "/etc/debian_version"); + if (buflen > 7) { + parse_os_info(&buf[7], buflen-7, "/etc/debian_version"); + } } else { strncpy(buf, "Linux", buflen); } @@ -2820,8 +2822,8 @@ int os::numa_get_group_id() { } int os::Linux::get_existing_num_nodes() { - size_t node; - size_t highest_node_number = Linux::numa_max_node(); + int node; + int highest_node_number = Linux::numa_max_node(); int num_nodes = 0; // Get the total number of nodes in the system including nodes without memory. @@ -2834,14 +2836,14 @@ int os::Linux::get_existing_num_nodes() { } size_t os::numa_get_leaf_groups(int *ids, size_t size) { - size_t highest_node_number = Linux::numa_max_node(); + int highest_node_number = Linux::numa_max_node(); size_t i = 0; - // Map all node ids in which is possible to allocate memory. Also nodes are + // Map all node ids in which it is possible to allocate memory. Also nodes are // not always consecutively available, i.e. available from 0 to the highest // node number. - for (size_t node = 0; node <= highest_node_number; node++) { - if (Linux::isnode_in_configured_nodes(node)) { + for (int node = 0; node <= highest_node_number; node++) { + if (Linux::isnode_in_configured_nodes((unsigned int)node)) { ids[i++] = node; } } diff --git a/src/hotspot/os/linux/perfMemory_linux.cpp b/src/hotspot/os/linux/perfMemory_linux.cpp index a4ebc03afe..86e75d8151 100644 --- a/src/hotspot/os/linux/perfMemory_linux.cpp +++ b/src/hotspot/os/linux/perfMemory_linux.cpp @@ -534,15 +534,14 @@ static char* get_user_name_slow(int vmid, int nspid, TRAPS) { // directory search char* oldest_user = NULL; time_t oldest_ctime = 0; - char buffer[TMP_BUFFER_LEN]; + char buffer[MAXPATHLEN + 1]; int searchpid; char* tmpdirname = (char *)os::get_temp_directory(); assert(strlen(tmpdirname) == 4, "No longer using /tmp - update buffer size"); if (nspid == -1) { searchpid = vmid; - } - else { + } else { jio_snprintf(buffer, MAXPATHLEN, "/proc/%d/root%s", vmid, tmpdirname); tmpdirname = buffer; searchpid = nspid; diff --git a/src/hotspot/share/classfile/moduleEntry.cpp b/src/hotspot/share/classfile/moduleEntry.cpp index a29783de7b..dad3016a8f 100644 --- a/src/hotspot/share/classfile/moduleEntry.cpp +++ b/src/hotspot/share/classfile/moduleEntry.cpp @@ -387,7 +387,8 @@ ModuleEntry* ModuleEntryTable::new_entry(unsigned int hash, Handle module_handle entry->set_is_patched(); if (log_is_enabled(Trace, module, patch)) { ResourceMark rm; - log_trace(module, patch)("Marked module %s as patched from --patch-module", name->as_C_string()); + log_trace(module, patch)("Marked module %s as patched from --patch-module", + name != NULL ? name->as_C_string() : UNNAMED_MODULE); } } diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index 7ad9601dd3..d6627d0d7a 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -1364,18 +1364,18 @@ InstanceKlass* SystemDictionary::load_shared_class(InstanceKlass* ik, // notify a class loaded from shared object ClassLoadingService::notify_class_loaded(ik, true /* shared class */); - } - ik->set_has_passed_fingerprint_check(false); - if (UseAOT && ik->supers_have_passed_fingerprint_checks()) { - uint64_t aot_fp = AOTLoader::get_saved_fingerprint(ik); - uint64_t cds_fp = ik->get_stored_fingerprint(); - if (aot_fp != 0 && aot_fp == cds_fp) { - // This class matches with a class saved in an AOT library - ik->set_has_passed_fingerprint_check(true); - } else { - ResourceMark rm; - log_info(class, fingerprint)("%s : expected = " PTR64_FORMAT " actual = " PTR64_FORMAT, ik->external_name(), aot_fp, cds_fp); + ik->set_has_passed_fingerprint_check(false); + if (UseAOT && ik->supers_have_passed_fingerprint_checks()) { + uint64_t aot_fp = AOTLoader::get_saved_fingerprint(ik); + uint64_t cds_fp = ik->get_stored_fingerprint(); + if (aot_fp != 0 && aot_fp == cds_fp) { + // This class matches with a class saved in an AOT library + ik->set_has_passed_fingerprint_check(true); + } else { + ResourceMark rm; + log_info(class, fingerprint)("%s : expected = " PTR64_FORMAT " actual = " PTR64_FORMAT, ik->external_name(), aot_fp, cds_fp); + } } } return ik; diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index 2d6a29941b..962c1da6c6 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -766,10 +766,11 @@ bool SystemDictionaryShared::add_verification_constraint(Klass* k, Symbol* name, SharedDictionaryEntry* entry = ((SharedDictionary*)(k->class_loader_data()->dictionary()))->find_entry_for(k); ResourceMark rm; // Lambda classes are not archived and will be regenerated at runtime. - if (entry == NULL && strstr(k->name()->as_C_string(), "Lambda$") != NULL) { + if (entry == NULL) { + guarantee(strstr(k->name()->as_C_string(), "Lambda$") != NULL, + "class should be in dictionary before being verified"); return true; } - assert(entry != NULL, "class should be in dictionary before being verified"); entry->add_verification_constraint(name, from_name, from_field_is_protected, from_is_array, from_is_object); if (entry->is_builtin()) { diff --git a/src/hotspot/share/classfile/verifier.cpp b/src/hotspot/share/classfile/verifier.cpp index 2e76c0ecd9..3efecb0e18 100644 --- a/src/hotspot/share/classfile/verifier.cpp +++ b/src/hotspot/share/classfile/verifier.cpp @@ -719,7 +719,8 @@ void ClassVerifier::verify_method(const methodHandle& m, TRAPS) { ResourceMark rm(THREAD); LogStream ls(lt); current_frame.print_on(&ls); - lt.print("offset = %d, opcode = %s", bci, Bytecodes::name(opcode)); + lt.print("offset = %d, opcode = %s", bci, + opcode == Bytecodes::_illegal ? "illegal" : Bytecodes::name(opcode)); } // Make sure wide instruction is in correct format diff --git a/src/hotspot/share/logging/logOutput.cpp b/src/hotspot/share/logging/logOutput.cpp index d3e9670ea7..a857f42547 100644 --- a/src/hotspot/share/logging/logOutput.cpp +++ b/src/hotspot/share/logging/logOutput.cpp @@ -262,7 +262,9 @@ void LogOutput::update_config_string(const size_t on_level[LogLevel::Count]) { while (n_deviates > 0) { size_t prev_deviates = n_deviates; int max_score = 0; - const LogSelection* best_selection = NULL; + + guarantee(n_selections > 0, "Cannot find maximal selection."); + const LogSelection* best_selection = &selections[0]; for (size_t i = 0; i < n_selections; i++) { // Give the selection a score based on how many deviating tag sets it selects (with correct level) @@ -287,13 +289,12 @@ void LogOutput::update_config_string(const size_t on_level[LogLevel::Count]) { // Pick the selection with the best score, or in the case of a tie, the one with fewest tags if (score > max_score || - (score == max_score && best_selection != NULL && selections[i].ntags() < best_selection->ntags())) { + (score == max_score && selections[i].ntags() < best_selection->ntags())) { max_score = score; best_selection = &selections[i]; } } - assert(best_selection != NULL, "must always find a maximal selection"); add_to_config_string(*best_selection); // Remove all deviates that this selection covered diff --git a/src/hotspot/share/memory/filemap.cpp b/src/hotspot/share/memory/filemap.cpp index 991033613a..181c2d0556 100644 --- a/src/hotspot/share/memory/filemap.cpp +++ b/src/hotspot/share/memory/filemap.cpp @@ -631,7 +631,9 @@ void FileMapInfo::write_region(int region, char* base, size_t size, si->_read_only = read_only; si->_allow_exec = allow_exec; si->_crc = ClassLoader::crc32(0, base, (jint)size); - write_bytes_aligned(base, (int)size); + if (base != NULL) { + write_bytes_aligned(base, (int)size); + } } // Write out the given archive heap memory regions. GC code combines multiple diff --git a/src/hotspot/share/memory/metaspace.cpp b/src/hotspot/share/memory/metaspace.cpp index 40ff2c7091..de2a7afc69 100644 --- a/src/hotspot/share/memory/metaspace.cpp +++ b/src/hotspot/share/memory/metaspace.cpp @@ -1260,7 +1260,9 @@ MetaWord* Metaspace::allocate(ClassLoaderData* loader_data, size_t word_size, tty->print_cr("Please increase MaxMetaspaceSize (currently " SIZE_FORMAT " bytes).", MaxMetaspaceSize); vm_exit(1); } - report_metadata_oome(loader_data, word_size, type, mdtype, CHECK_NULL); + report_metadata_oome(loader_data, word_size, type, mdtype, THREAD); + assert(HAS_PENDING_EXCEPTION, "sanity"); + return NULL; } // Zero initialize. diff --git a/src/hotspot/share/memory/virtualspace.cpp b/src/hotspot/share/memory/virtualspace.cpp index d5764d351a..590704bdcc 100644 --- a/src/hotspot/share/memory/virtualspace.cpp +++ b/src/hotspot/share/memory/virtualspace.cpp @@ -70,6 +70,18 @@ ReservedSpace::ReservedSpace(size_t size, size_t alignment, initialize(size, alignment, large, NULL, executable); } +ReservedSpace::ReservedSpace(char* base, size_t size, size_t alignment, + bool special, bool executable) : _fd_for_heap(-1) { + assert((size % os::vm_allocation_granularity()) == 0, + "size not allocation aligned"); + _base = base; + _size = size; + _alignment = alignment; + _noaccess_prefix = 0; + _special = special; + _executable = executable; +} + // Helper method static void unmap_or_release_memory(char* base, size_t size, bool is_file_mapped) { if (is_file_mapped) { @@ -218,20 +230,6 @@ void ReservedSpace::initialize(size_t size, size_t alignment, bool large, } } - -ReservedSpace::ReservedSpace(char* base, size_t size, size_t alignment, - bool special, bool executable) { - assert((size % os::vm_allocation_granularity()) == 0, - "size not allocation aligned"); - _base = base; - _size = size; - _alignment = alignment; - _noaccess_prefix = 0; - _special = special; - _executable = executable; -} - - ReservedSpace ReservedSpace::first_part(size_t partition_size, size_t alignment, bool split, bool realloc) { assert(partition_size <= size(), "partition failed"); diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index 75c71b0264..8002cbd040 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -1219,7 +1219,7 @@ VM_GetMultipleStackTraces::fill_frames(jthread jt, JavaThread *thr, oop thread_o } infop->state = state; - if (thr != NULL || (state & JVMTI_THREAD_STATE_ALIVE) != 0) { + if (thr != NULL && (state & JVMTI_THREAD_STATE_ALIVE) != 0) { infop->frame_buffer = NEW_RESOURCE_ARRAY(jvmtiFrameInfo, max_frame_count()); env()->get_stack_trace(thr, 0, max_frame_count(), infop->frame_buffer, &(infop->frame_count)); diff --git a/src/hotspot/share/runtime/flags/jvmFlag.cpp b/src/hotspot/share/runtime/flags/jvmFlag.cpp index ae12ef34e5..841a876ecb 100644 --- a/src/hotspot/share/runtime/flags/jvmFlag.cpp +++ b/src/hotspot/share/runtime/flags/jvmFlag.cpp @@ -939,6 +939,10 @@ JVMFlag* JVMFlag::fuzzy_match(const char* name, size_t length, bool allow_locked } } + if (match == NULL) { + return NULL; + } + if (!(match->is_unlocked() || match->is_unlocker())) { if (!allow_locked) { return NULL; diff --git a/src/hotspot/share/services/writeableFlags.cpp b/src/hotspot/share/services/writeableFlags.cpp index fee4e68b51..780df4c588 100644 --- a/src/hotspot/share/services/writeableFlags.cpp +++ b/src/hotspot/share/services/writeableFlags.cpp @@ -79,7 +79,7 @@ static void print_flag_error_message_if_needed(JVMFlag::Error error, const char* case JVMFlag::NON_WRITABLE: buffer_concat(buffer, "flag is not writeable."); break; case JVMFlag::OUT_OF_BOUNDS: - print_flag_error_message_bounds(name, buffer); break; + if (name != NULL) { print_flag_error_message_bounds(name, buffer); } break; case JVMFlag::VIOLATES_CONSTRAINT: buffer_concat(buffer, "value violates its flag's constraint."); break; case JVMFlag::INVALID_FLAG: diff --git a/src/hotspot/share/utilities/ostream.cpp b/src/hotspot/share/utilities/ostream.cpp index 5364c7407c..ed9b43cf82 100644 --- a/src/hotspot/share/utilities/ostream.cpp +++ b/src/hotspot/share/utilities/ostream.cpp @@ -531,7 +531,8 @@ void fileStream::write(const char* s, size_t len) { long fileStream::fileSize() { long size = -1; if (_file != NULL) { - long pos = ::ftell(_file); + long pos = ::ftell(_file); + if (pos < 0) return pos; if (::fseek(_file, 0, SEEK_END) == 0) { size = ::ftell(_file); } diff --git a/test/hotspot/gtest/logging/logTestUtils.inline.hpp b/test/hotspot/gtest/logging/logTestUtils.inline.hpp index bc9d37af0c..5e97882b97 100644 --- a/test/hotspot/gtest/logging/logTestUtils.inline.hpp +++ b/test/hotspot/gtest/logging/logTestUtils.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 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 @@ -95,6 +95,7 @@ static inline char* read_line(FILE* fp) { int buflen = 512; char* buf = NEW_RESOURCE_ARRAY(char, buflen); long pos = ftell(fp); + if (pos < 0) return NULL; char* ret = fgets(buf, buflen, fp); while (ret != NULL && buf[strlen(buf) - 1] != '\n' && !feof(fp)) { diff --git a/test/hotspot/gtest/memory/test_metachunk.cpp b/test/hotspot/gtest/memory/test_metachunk.cpp index 1a00aa510f..49dbd8cd6a 100644 --- a/test/hotspot/gtest/memory/test_metachunk.cpp +++ b/test/hotspot/gtest/memory/test_metachunk.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 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 @@ -60,7 +60,7 @@ TEST(Metachunk, basic) { // Check sizes EXPECT_EQ(metachunk->size(), metachunk->word_size()); EXPECT_EQ(pointer_delta(metachunk->end(), metachunk->bottom(), - sizeof (MetaWord*)), + sizeof (MetaWord)), metachunk->word_size()); // Check usage -- GitLab