8221539: [metaspace] Improve MetaspaceObj::is_metaspace_obj() and friends
Reviewed-by: adinn, coleenp, mdoerr
--- a/src/hotspot/cpu/aarch64/frame_aarch64.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/cpu/aarch64/frame_aarch64.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -559,7 +559,7 @@
// validate constantPoolCache*
ConstantPoolCache* cp = *interpreter_frame_cache_addr();
- if (cp == NULL || !cp->is_metaspace_object()) return false;
+ if (MetaspaceObj::is_valid(cp) == false) return false;
// validate locals
--- a/src/hotspot/cpu/arm/frame_arm.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/cpu/arm/frame_arm.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -494,7 +494,7 @@
// validate ConstantPoolCache*
ConstantPoolCache* cp = *interpreter_frame_cache_addr();
- if (cp == NULL || !cp->is_metaspace_object()) return false;
+ if (MetaspaceObj::is_valid(cp) == false) return false;
// validate locals
--- a/src/hotspot/cpu/sparc/frame_sparc.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/cpu/sparc/frame_sparc.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -665,7 +665,7 @@
// validate ConstantPoolCache*
ConstantPoolCache* cp = *interpreter_frame_cache_addr();
- if (cp == NULL || !cp->is_metaspace_object()) return false;
+ if (MetaspaceObj::is_valid(cp) == false) return false;
// validate locals
--- a/src/hotspot/cpu/x86/frame_x86.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/cpu/x86/frame_x86.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -546,7 +546,7 @@
// validate ConstantPoolCache*
ConstantPoolCache* cp = *interpreter_frame_cache_addr();
- if (cp == NULL || !cp->is_metaspace_object()) return false;
+ if (MetaspaceObj::is_valid(cp) == false) return false;
// validate locals
--- a/src/hotspot/os_cpu/linux_ppc/thread_linux_ppc.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/os_cpu/linux_ppc/thread_linux_ppc.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -66,7 +66,7 @@
if (ret_frame.is_interpreted_frame()) {
frame::ijava_state* istate = ret_frame.get_ijava_state();
- if (!((Method*)(istate->method))->is_metaspace_object()) {
+ if (MetaspaceObj::is_valid((Method*)(istate->method)) == false) {
return false;
}
uint64_t reg_bcp = uc->uc_mcontext.regs->gpr[14/*R14_bcp*/];
--- a/src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -63,7 +63,8 @@
if (ret_frame.is_interpreted_frame()) {
frame::z_ijava_state* istate = ret_frame.ijava_state_unchecked();
- if ((stack_base() >= (address)istate && (address)istate > stack_end()) || !((Method*)(istate->method))->is_metaspace_object()) {
+ if ((stack_base() >= (address)istate && (address)istate > stack_end()) ||
+ MetaspaceObj::is_valid((Method*)(istate->method)) == false) {
return false;
}
uint64_t reg_bcp = uc->uc_mcontext.gregs[13/*Z_BCP*/];
--- a/src/hotspot/share/memory/allocation.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/memory/allocation.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -84,8 +84,14 @@
return Metaspace::allocate(loader_data, word_size, type, THREAD);
}
-bool MetaspaceObj::is_metaspace_object() const {
- return Metaspace::contains((void*)this);
+bool MetaspaceObj::is_valid(const MetaspaceObj* p) {
+ // Weed out obvious bogus values first without traversing metaspace
+ if ((size_t)p < os::min_page_size()) {
+ return false;
+ } else if (!is_aligned((address)p, sizeof(MetaWord))) {
+ return false;
+ }
+ return Metaspace::contains((void*)p);
}
void MetaspaceObj::print_address_on(outputStream* st) const {
--- a/src/hotspot/share/memory/allocation.hpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/memory/allocation.hpp Wed Mar 27 14:13:34 2019 +0100
@@ -258,12 +258,19 @@
static void* _shared_metaspace_top; // (exclusive) high address
public:
- bool is_metaspace_object() const;
- bool is_shared() const {
+
+ // Returns true if the pointer points to a valid MetaspaceObj. A valid
+ // MetaspaceObj is MetaWord-aligned and contained within either
+ // non-shared or shared metaspace.
+ static bool is_valid(const MetaspaceObj* p);
+
+ static bool is_shared(const MetaspaceObj* p) {
// If no shared metaspace regions are mapped, _shared_metaspace_{base,top} will
// both be NULL and all values of p will be rejected quickly.
- return (((void*)this) < _shared_metaspace_top && ((void*)this) >= _shared_metaspace_base);
+ return (((void*)p) < _shared_metaspace_top && ((void*)p) >= _shared_metaspace_base);
}
+ bool is_shared() const { return MetaspaceObj::is_shared(this); }
+
void print_address_on(outputStream* st) const; // nonvirtual address printing
static void set_shared_metaspace_range(void* base, void* top) {
--- a/src/hotspot/share/memory/metaspace/virtualSpaceList.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/virtualSpaceList.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -92,9 +92,9 @@
assert_lock_strong(MetaspaceExpand_lock);
// Don't use a VirtualSpaceListIterator because this
// list is being changed and a straightforward use of an iterator is not safe.
- VirtualSpaceNode* purged_vsl = NULL;
VirtualSpaceNode* prev_vsl = virtual_space_list();
VirtualSpaceNode* next_vsl = prev_vsl;
+ int num_purged_nodes = 0;
while (next_vsl != NULL) {
VirtualSpaceNode* vsl = next_vsl;
DEBUG_ONLY(vsl->verify(false);)
@@ -118,20 +118,17 @@
dec_reserved_words(vsl->reserved_words());
dec_committed_words(vsl->committed_words());
dec_virtual_space_count();
- purged_vsl = vsl;
delete vsl;
+ num_purged_nodes ++;
} else {
prev_vsl = vsl;
}
}
+
+ // Verify list
#ifdef ASSERT
- if (purged_vsl != NULL) {
- // List should be stable enough to use an iterator here.
- VirtualSpaceListIterator iter(virtual_space_list());
- while (iter.repeat()) {
- VirtualSpaceNode* vsl = iter.get_next();
- assert(vsl != purged_vsl, "Purge of vsl failed");
- }
+ if (num_purged_nodes > 0) {
+ verify(false);
}
#endif
}
@@ -143,11 +140,13 @@
VirtualSpaceNode* VirtualSpaceList::find_enclosing_space(const void* ptr) {
// List should be stable enough to use an iterator here because removing virtual
// space nodes is only allowed at a safepoint.
- VirtualSpaceListIterator iter(virtual_space_list());
- while (iter.repeat()) {
- VirtualSpaceNode* vsn = iter.get_next();
- if (vsn->contains(ptr)) {
- return vsn;
+ if (is_within_envelope((address)ptr)) {
+ VirtualSpaceListIterator iter(virtual_space_list());
+ while (iter.repeat()) {
+ VirtualSpaceNode* vsn = iter.get_next();
+ if (vsn->contains(ptr)) {
+ return vsn;
+ }
}
}
return NULL;
@@ -170,7 +169,9 @@
_is_class(false),
_reserved_words(0),
_committed_words(0),
- _virtual_space_count(0) {
+ _virtual_space_count(0),
+ _envelope_lo((address)max_uintx),
+ _envelope_hi(NULL) {
MutexLockerEx cl(MetaspaceExpand_lock,
Mutex::_no_safepoint_check_flag);
create_new_virtual_space(word_size);
@@ -182,12 +183,17 @@
_is_class(true),
_reserved_words(0),
_committed_words(0),
- _virtual_space_count(0) {
+ _virtual_space_count(0),
+ _envelope_lo((address)max_uintx),
+ _envelope_hi(NULL) {
MutexLockerEx cl(MetaspaceExpand_lock,
Mutex::_no_safepoint_check_flag);
VirtualSpaceNode* class_entry = new VirtualSpaceNode(is_class(), rs);
bool succeeded = class_entry->initialize();
if (succeeded) {
+ expand_envelope_to_include_node(class_entry);
+ // ensure lock-free iteration sees fully initialized node
+ OrderAccess::storestore();
link_vs(class_entry);
}
}
@@ -224,12 +230,16 @@
} else {
assert(new_entry->reserved_words() == vs_word_size,
"Reserved memory size differs from requested memory size");
+ expand_envelope_to_include_node(new_entry);
// ensure lock-free iteration sees fully initialized node
OrderAccess::storestore();
link_vs(new_entry);
DEBUG_ONLY(Atomic::inc(&g_internal_statistics.num_vsnodes_created));
return true;
}
+
+ DEBUG_ONLY(verify(false);)
+
}
void VirtualSpaceList::link_vs(VirtualSpaceNode* new_entry) {
@@ -399,5 +409,41 @@
}
}
+// Given a node, expand range such that it includes the node.
+void VirtualSpaceList::expand_envelope_to_include_node(const VirtualSpaceNode* node) {
+ _envelope_lo = MIN2(_envelope_lo, (address)node->low_boundary());
+ _envelope_hi = MAX2(_envelope_hi, (address)node->high_boundary());
+}
+
+
+#ifdef ASSERT
+void VirtualSpaceList::verify(bool slow) {
+ VirtualSpaceNode* list = virtual_space_list();
+ VirtualSpaceListIterator iter(list);
+ size_t reserved = 0;
+ size_t committed = 0;
+ size_t node_count = 0;
+ while (iter.repeat()) {
+ VirtualSpaceNode* node = iter.get_next();
+ if (slow) {
+ node->verify(true);
+ }
+ // Check that the node resides fully within our envelope.
+ assert((address)node->low_boundary() >= _envelope_lo && (address)node->high_boundary() <= _envelope_hi,
+ "Node " SIZE_FORMAT " [" PTR_FORMAT ", " PTR_FORMAT ") outside envelope [" PTR_FORMAT ", " PTR_FORMAT ").",
+ node_count, p2i(node->low_boundary()), p2i(node->high_boundary()), p2i(_envelope_lo), p2i(_envelope_hi));
+ reserved += node->reserved_words();
+ committed += node->committed_words();
+ node_count ++;
+ }
+ assert(reserved == reserved_words() && committed == committed_words() && node_count == _virtual_space_count,
+ "Mismatch: reserved real: " SIZE_FORMAT " expected: " SIZE_FORMAT
+ ", committed real: " SIZE_FORMAT " expected: " SIZE_FORMAT
+ ", node count real: " SIZE_FORMAT " expected: " SIZE_FORMAT ".",
+ reserved, reserved_words(), committed, committed_words(),
+ node_count, _virtual_space_count);
+}
+#endif // ASSERT
+
} // namespace metaspace
--- a/src/hotspot/share/memory/metaspace/virtualSpaceList.hpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/virtualSpaceList.hpp Wed Mar 27 14:13:34 2019 +0100
@@ -58,6 +58,19 @@
// Number of virtual spaces
size_t _virtual_space_count;
+ // Optimization: we keep an address range to quickly exclude pointers
+ // which are clearly not pointing into metaspace. This is an optimization for
+ // VirtualSpaceList::contains().
+ address _envelope_lo;
+ address _envelope_hi;
+
+ bool is_within_envelope(address p) const {
+ return p >= _envelope_lo && p < _envelope_hi;
+ }
+
+ // Given a node, expand range such that it includes the node.
+ void expand_envelope_to_include_node(const VirtualSpaceNode* node);
+
~VirtualSpaceList();
VirtualSpaceNode* virtual_space_list() const { return _virtual_space_list; }
@@ -80,6 +93,8 @@
// virtual space and add the chunks to the free list.
void retire_current_virtual_space();
+ DEBUG_ONLY(bool contains_node(const VirtualSpaceNode* node) const;)
+
public:
VirtualSpaceList(size_t word_size);
VirtualSpaceList(ReservedSpace rs);
@@ -126,6 +141,8 @@
void print_on(outputStream* st, size_t scale) const;
void print_map(outputStream* st) const;
+ DEBUG_ONLY(void verify(bool slow);)
+
class VirtualSpaceListIterator : public StackObj {
VirtualSpaceNode* _virtual_spaces;
public:
--- a/src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp Wed Mar 27 14:13:34 2019 +0100
@@ -60,6 +60,8 @@
// Convenience functions to access the _virtual_space
char* low() const { return virtual_space()->low(); }
char* high() const { return virtual_space()->high(); }
+ char* low_boundary() const { return virtual_space()->low_boundary(); }
+ char* high_boundary() const { return virtual_space()->high_boundary(); }
// The first Metachunk will be allocated at the bottom of the
// VirtualSpace
--- a/src/hotspot/share/oops/instanceKlass.cpp Fri Apr 05 09:18:18 2019 +0200
+++ b/src/hotspot/share/oops/instanceKlass.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -3104,7 +3104,7 @@
for (int i = 0; i < len; i++) {
intptr_t e = start[i];
st->print("%d : " INTPTR_FORMAT, i, e);
- if (e != 0 && ((Metadata*)e)->is_metaspace_object()) {
+ if (MetaspaceObj::is_valid((Metadata*)e)) {
st->print(" ");
((Metadata*)e)->print_value_on(st);
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/gtest/memory/test_is_metaspace_obj.cpp Wed Mar 27 14:13:34 2019 +0100
@@ -0,0 +1,115 @@
+/*
+ * 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "precompiled.hpp"
+#include "memory/allocation.inline.hpp"
+#include "memory/metaspace.hpp"
+#include "memory/metaspace/virtualSpaceList.hpp"
+#include "runtime/mutex.hpp"
+#include "runtime/mutexLocker.hpp"
+#include "runtime/os.hpp"
+#include "unittest.hpp"
+
+using namespace metaspace;
+
+
+// Test the cheerful multitude of metaspace-contains-functions.
+class MetaspaceIsMetaspaceObjTest : public ::testing::Test {
+ Mutex* _lock;
+ ClassLoaderMetaspace* _ms;
+
+public:
+
+ MetaspaceIsMetaspaceObjTest() : _lock(NULL), _ms(NULL) {}
+
+ virtual void SetUp() {
+ }
+
+ virtual void TearDown() {
+ delete _ms;
+ delete _lock;
+ }
+
+ void do_test(Metaspace::MetadataType mdType) {
+ _lock = new Mutex(Monitor::native, "gtest-IsMetaspaceObjTest-lock", false, Monitor::_safepoint_check_never);
+ {
+ MutexLockerEx ml(_lock, Mutex::_no_safepoint_check_flag);
+ _ms = new ClassLoaderMetaspace(_lock, Metaspace::StandardMetaspaceType);
+ }
+
+ const MetaspaceObj* p = (MetaspaceObj*) _ms->allocate(42, mdType);
+
+ // Test MetaspaceObj::is_metaspace_object
+ ASSERT_TRUE(MetaspaceObj::is_valid(p));
+
+ // A misaligned object shall not be recognized
+ const MetaspaceObj* p_misaligned = (MetaspaceObj*)((address)p) + 1;
+ ASSERT_FALSE(MetaspaceObj::is_valid(p_misaligned));
+
+ // Test VirtualSpaceList::contains and find_enclosing_space
+ VirtualSpaceList* list = Metaspace::space_list();
+ if (mdType == Metaspace::ClassType && Metaspace::using_class_space()) {
+ list = Metaspace::class_space_list();
+ }
+ ASSERT_TRUE(list->contains(p));
+ VirtualSpaceNode* const n = list->find_enclosing_space(p);
+ ASSERT_TRUE(n != NULL);
+ ASSERT_TRUE(n->contains(p));
+
+ // A misaligned pointer shall be recognized by list::contains
+ ASSERT_TRUE(list->contains((address)p) + 1);
+
+ // Now for some bogus values
+ ASSERT_FALSE(MetaspaceObj::is_valid((MetaspaceObj*)NULL));
+
+ // Should exercise various paths in MetaspaceObj::is_valid()
+ ASSERT_FALSE(MetaspaceObj::is_valid((MetaspaceObj*)1024));
+ ASSERT_FALSE(MetaspaceObj::is_valid((MetaspaceObj*)8192));
+
+ MetaspaceObj* p_stack = (MetaspaceObj*) &_lock;
+ ASSERT_FALSE(MetaspaceObj::is_valid(p_stack));
+
+ MetaspaceObj* p_heap = (MetaspaceObj*) os::malloc(41, mtInternal);
+ ASSERT_FALSE(MetaspaceObj::is_valid(p_heap));
+ os::free(p_heap);
+
+ // Test Metaspace::contains_xxx
+ ASSERT_TRUE(Metaspace::contains(p));
+ ASSERT_TRUE(Metaspace::contains_non_shared(p));
+
+ delete _ms;
+ _ms = NULL;
+ delete _lock;
+ _lock = NULL;
+ }
+
+};
+
+TEST_VM_F(MetaspaceIsMetaspaceObjTest, non_class_space) {
+ do_test(Metaspace::NonClassType);
+}
+
+TEST_VM_F(MetaspaceIsMetaspaceObjTest, class_space) {
+ do_test(Metaspace::ClassType);
+}
+