# HG changeset patch
# User stuefe
# Date 1553692414 -3600
#      Wed Mar 27 14:13:34 2019 +0100
# Node ID f6c9e38f2b8f0e3f3769dfd688daa4b26c4c4a78
# Parent  9559ba212c1889e0c77e7bf6cc7a689632b87bcd
8221539: [metaspace] Improve MetaspaceObj::is_metaspace_obj() and friends
Reviewed-by: adinn

diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/cpu/aarch64/frame_aarch64.cpp
--- a/src/hotspot/cpu/aarch64/frame_aarch64.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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 (cp == NULL || MetaspaceObj::is_valid(cp) == false) return false;
 
   // validate locals
 
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/cpu/arm/frame_arm.cpp
--- a/src/hotspot/cpu/arm/frame_arm.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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 (cp == NULL || MetaspaceObj::is_valid(cp) == false) return false;
 
   // validate locals
 
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/cpu/sparc/frame_sparc.cpp
--- a/src/hotspot/cpu/sparc/frame_sparc.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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 (cp == NULL || MetaspaceObj::is_valid(cp) == false) return false;
 
   // validate locals
 
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/cpu/x86/frame_x86.cpp
--- a/src/hotspot/cpu/x86/frame_x86.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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 (cp == NULL || MetaspaceObj::is_valid(cp) == false) return false;
 
   // validate locals
 
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/os_cpu/linux_ppc/thread_linux_ppc.cpp
--- a/src/hotspot/os_cpu/linux_ppc/thread_linux_ppc.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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*/];
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp
--- a/src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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*/];
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/memory/allocation.cpp
--- a/src/hotspot/share/memory/allocation.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ b/src/hotspot/share/memory/allocation.cpp	Wed Mar 27 14:13:34 2019 +0100
@@ -84,8 +84,18 @@
   return Metaspace::allocate(loader_data, word_size, type, THREAD);
 }
 
+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);
+}
+
 bool MetaspaceObj::is_metaspace_object() const {
-  return Metaspace::contains((void*)this);
+  return MetaspaceObj::is_valid(this);
 }
 
 void MetaspaceObj::print_address_on(outputStream* st) const {
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/memory/allocation.hpp
--- a/src/hotspot/share/memory/allocation.hpp	Tue Apr 02 13:08:38 2019 -0400
+++ b/src/hotspot/share/memory/allocation.hpp	Wed Mar 27 14:13:34 2019 +0100
@@ -258,6 +258,7 @@
   static void* _shared_metaspace_top;  // (exclusive) high address
 
  public:
+  static bool is_valid(const MetaspaceObj* p);
   bool is_metaspace_object() const;
   bool is_shared() const {
     // If no shared metaspace regions are mapped, _shared_metaspace_{base,top} will
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/memory/metaspace/virtualSpaceList.cpp
--- a/src/hotspot/share/memory/metaspace/virtualSpaceList.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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
 
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/memory/metaspace/virtualSpaceList.hpp
--- a/src/hotspot/share/memory/metaspace/virtualSpaceList.hpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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:
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp
--- a/src/hotspot/share/memory/metaspace/virtualSpaceNode.hpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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
diff -r 9559ba212c18 -r f6c9e38f2b8f src/hotspot/share/oops/instanceKlass.cpp
--- a/src/hotspot/share/oops/instanceKlass.cpp	Tue Apr 02 13:08:38 2019 -0400
+++ 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 (e != 0 && MetaspaceObj::is_valid((Metadata*)e)) {
       st->print(" ");
       ((Metadata*)e)->print_value_on(st);
     }
diff -r 9559ba212c18 -r f6c9e38f2b8f test/hotspot/gtest/memory/test_is_metaspace_obj.cpp
--- /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,106 @@
+/*
+ * 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 "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);
+    }
+
+    MetaWord* const p = _ms->allocate(42, mdType);
+
+    // Test MetaspaceObj::is_metaspace_object
+    ASSERT_TRUE(((const MetaspaceObj*)p)->is_metaspace_object());
+
+    // A misaligned object shall not be recognized
+    ASSERT_FALSE(((const MetaspaceObj*)(((address)p) + 1))->is_metaspace_object());
+
+    // 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(((const MetaspaceObj*)((address)NULL))->is_metaspace_object());
+    ASSERT_FALSE(((const MetaspaceObj*)((address)1024))->is_metaspace_object());
+
+    // some stack address
+    ASSERT_FALSE(((const MetaspaceObj*)(&_lock))->is_metaspace_object());
+
+    // 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);
+}
+