8337981: ShenandoahHeap::is_in should check for alive regions

Reviewed-by: rkennke, wkemper

Author: shipilev

src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp

@@ -37,7 +37,7 @@ void print_raw_memory(ShenandoahMessageBuffer &msg, void* loc) {
   // should be in heap, in known committed region, within that region.
 
   ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (!heap->is_in(loc)) return;
+  if (!heap->is_in_reserved(loc)) return;
 
   ShenandoahHeapRegion* r = heap->heap_region_containing(loc);
   if (r != nullptr && r->is_committed()) {
@@ -77,7 +77,7 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) {
 
 void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
-  if (heap->is_in(loc)) {
+  if (heap->is_in_reserved(loc)) {
     msg.append("  inside Java heap\n");
     ShenandoahHeapRegion *r = heap->heap_region_containing(loc);
     stringStream ss;
@@ -96,7 +96,7 @@ void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) {
 void ShenandoahAsserts::print_obj_safe(ShenandoahMessageBuffer& msg, void* loc) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   msg.append("  " PTR_FORMAT " - safe print, no details\n", p2i(loc));
-  if (heap->is_in(loc)) {
+  if (heap->is_in_reserved(loc)) {
     ShenandoahHeapRegion* r = heap->heap_region_containing(loc);
     if (r != nullptr) {
       stringStream ss;
@@ -113,7 +113,7 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l
   ShenandoahHeap* heap = ShenandoahHeap::heap();
   ResourceMark rm;
 
-  bool loc_in_heap = (loc != nullptr && heap->is_in(loc));
+  bool loc_in_heap = (loc != nullptr && heap->is_in_reserved(loc));
 
   ShenandoahMessageBuffer msg("%s; %s\n\n", phase, label);
 
@@ -166,22 +166,22 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l
   report_vm_error(file, line, msg.buffer());
 }
 
-void ShenandoahAsserts::assert_in_heap(void* interior_loc, oop obj, const char *file, int line) {
+void ShenandoahAsserts::assert_in_heap_bounds(void* interior_loc, oop obj, const char *file, int line) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
-  if (!heap->is_in(obj)) {
-    print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap failed",
-                  "oop must point to a heap address",
+  if (!heap->is_in_reserved(obj)) {
+    print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_bounds failed",
+                  "oop must be in heap bounds",
                   file, line);
   }
 }
 
-void ShenandoahAsserts::assert_in_heap_or_null(void* interior_loc, oop obj, const char *file, int line) {
+void ShenandoahAsserts::assert_in_heap_bounds_or_null(void* interior_loc, oop obj, const char *file, int line) {
   ShenandoahHeap* heap = ShenandoahHeap::heap();
 
-  if (obj != nullptr && !heap->is_in(obj)) {
-    print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_or_null failed",
-                  "oop must point to a heap address",
+  if (obj != nullptr && !heap->is_in_reserved(obj)) {
+    print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_bounds_or_null failed",
+                  "oop must be in heap bounds",
                   file, line);
   }
 }
@@ -191,9 +191,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
 
   // Step 1. Check that obj is correct.
   // After this step, it is safe to call heap_region_containing().
-  if (!heap->is_in(obj)) {
+  if (!heap->is_in_reserved(obj)) {
     print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
-                  "oop must point to a heap address",
+                  "oop must be in heap bounds",
                   file, line);
   }
 
@@ -210,6 +210,12 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
                   file,line);
   }
 
+  if (!heap->is_in(obj)) {
+    print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
+                  "Object should be in active region area",
+                  file, line);
+  }
+
   oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj);
 
   if (obj != fwd) {
@@ -223,9 +229,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
     }
 
     // Step 2. Check that forwardee is correct
-    if (!heap->is_in(fwd)) {
+    if (!heap->is_in_reserved(fwd)) {
       print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
-                    "Forwardee must point to a heap address",
+                    "Forwardee must be in heap bounds",
                     file, line);
     }
 
@@ -236,9 +242,15 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
     }
 
     // Step 3. Check that forwardee points to correct region
+    if (!heap->is_in(fwd)) {
+      print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
+                    "Forwardee should be in active region area",
+                    file, line);
+    }
+
     if (heap->heap_region_index_containing(fwd) == heap->heap_region_index_containing(obj)) {
       print_failure(_safe_all, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
-                    "Non-trivial forwardee should in another region",
+                    "Non-trivial forwardee should be in another region",
                     file, line);
     }
 

src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp

@@ -53,8 +53,8 @@ class ShenandoahAsserts {
   static void print_rp_failure(const char *label, BoolObjectClosure* actual,
                                const char *file, int line);
 
-  static void assert_in_heap(void* interior_loc, oop obj, const char* file, int line);
-  static void assert_in_heap_or_null(void* interior_loc, oop obj, const char* file, int line);
+  static void assert_in_heap_bounds(void* interior_loc, oop obj, const char* file, int line);
+  static void assert_in_heap_bounds_or_null(void* interior_loc, oop obj, const char* file, int line);
   static void assert_in_correct_region(void* interior_loc, oop obj, const char* file, int line);
 
   static void assert_correct(void* interior_loc, oop obj, const char* file, int line);
@@ -74,10 +74,10 @@ class ShenandoahAsserts {
   static void assert_heaplocked_or_safepoint(const char* file, int line);
 
 #ifdef ASSERT
-#define shenandoah_assert_in_heap(interior_loc, obj) \
-                    ShenandoahAsserts::assert_in_heap(interior_loc, obj, __FILE__, __LINE__)
-#define shenandoah_assert_in_heap_or_null(interior_loc, obj) \
-                    ShenandoahAsserts::assert_in_heap_or_null(interior_loc, obj, __FILE__, __LINE__)
+#define shenandoah_assert_in_heap_bounds(interior_loc, obj) \
+                    ShenandoahAsserts::assert_in_heap_bounds(interior_loc, obj, __FILE__, __LINE__)
+#define shenandoah_assert_in_heap_bounds_or_null(interior_loc, obj) \
+                    ShenandoahAsserts::assert_in_heap_bounds_or_null(interior_loc, obj, __FILE__, __LINE__)
 #define shenandoah_assert_in_correct_region(interior_loc, obj) \
                     ShenandoahAsserts::assert_in_correct_region(interior_loc, obj, __FILE__, __LINE__)
 
@@ -164,8 +164,8 @@ class ShenandoahAsserts {
 #define shenandoah_assert_heaplocked_or_safepoint() \
                     ShenandoahAsserts::assert_heaplocked_or_safepoint(__FILE__, __LINE__)
 #else
-#define shenandoah_assert_in_heap(interior_loc, obj)
-#define shenandoah_assert_in_heap_or_null(interior_loc, obj)
+#define shenandoah_assert_in_heap_bounds(interior_loc, obj)
+#define shenandoah_assert_in_heap_bounds_or_null(interior_loc, obj)
 #define shenandoah_assert_in_correct_region(interior_loc, obj)
 
 #define shenandoah_assert_correct_if(interior_loc, obj, condition)

src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.inline.hpp

@@ -41,12 +41,12 @@ bool ShenandoahCollectionSet::is_in(ShenandoahHeapRegion* r) const {
 }
 
 bool ShenandoahCollectionSet::is_in(oop p) const {
-  shenandoah_assert_in_heap_or_null(nullptr, p);
+  shenandoah_assert_in_heap_bounds_or_null(nullptr, p);
   return is_in_loc(cast_from_oop<void*>(p));
 }
 
 bool ShenandoahCollectionSet::is_in_loc(void* p) const {
-  assert(p == nullptr || _heap->is_in(p), "Must be in the heap");
+  assert(p == nullptr || _heap->is_in_reserved(p), "Must be in the heap");
   uintx index = ((uintx) p) >> _region_size_bytes_shift;
   // no need to subtract the bottom of the heap from p,
   // _biased_cset_map is biased

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp

@@ -718,7 +718,7 @@ void ShenandoahEvacUpdateCleanupOopStorageRootsClosure::do_oop(oop* p) {
   const oop obj = RawAccess<>::oop_load(p);
   if (!CompressedOops::is_null(obj)) {
     if (!_mark_context->is_marked(obj)) {
-      shenandoah_assert_correct(p, obj);
+      // Note: The obj is dead here. Do not touch it, just clear.
       ShenandoahHeap::atomic_clear_oop(p, obj);
     } else if (_evac_in_progress && _heap->in_collection_set(obj)) {
       oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);

src/hotspot/share/gc/shenandoah/shenandoahForwarding.inline.hpp

@@ -32,7 +32,7 @@
 #include "runtime/javaThread.hpp"
 
 inline oop ShenandoahForwarding::get_forwardee_raw(oop obj) {
-  shenandoah_assert_in_heap(nullptr, obj);
+  shenandoah_assert_in_heap_bounds(nullptr, obj);
   return get_forwardee_raw_unchecked(obj);
 }
 

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -730,9 +730,18 @@ size_t ShenandoahHeap::initial_capacity() const {
 }
 
 bool ShenandoahHeap::is_in(const void* p) const {
-  HeapWord* heap_base = (HeapWord*) base();
-  HeapWord* last_region_end = heap_base + ShenandoahHeapRegion::region_size_words() * num_regions();
-  return p >= heap_base && p < last_region_end;
+  if (is_in_reserved(p)) {
+    if (is_full_gc_move_in_progress()) {
+      // Full GC move is running, we do not have a consistent region
+      // information yet. But we know the pointer is in heap.
+      return true;
+    }
+    // Now check if we point to a live section in active region.
+    ShenandoahHeapRegion* r = heap_region_containing(p);
+    return (r->is_active() && p < r->top());
+  } else {
+    return false;
+  }
 }
 
 void ShenandoahHeap::maybe_uncommit(double shrink_before, size_t shrink_until) {

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

@@ -493,6 +493,8 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {
 public:
   bool is_maximal_no_gc() const override shenandoah_not_implemented_return(false);
 
+  // Check the pointer is in active part of Java heap.
+  // Use is_in_reserved to check if object is within heap bounds.
   bool is_in(const void* p) const override;
 
   bool requires_barriers(stackChunkOop obj) const override;

src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp

@@ -122,7 +122,7 @@ void ShenandoahMarkBitMap::clear_range_large(MemRegion mr) {
 
 #ifdef ASSERT
 void ShenandoahMarkBitMap::check_mark(HeapWord* addr) const {
-  assert(ShenandoahHeap::heap()->is_in(addr),
+  assert(ShenandoahHeap::heap()->is_in_reserved(addr),
          "Trying to access bitmap " PTR_FORMAT " for address " PTR_FORMAT " not in the heap.",
          p2i(this), p2i(addr));
 }

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp

@@ -63,8 +63,10 @@ class ShenandoahMarkingContext : public CHeapObj<mtGC> {
   inline bool mark_weak(oop obj);
 
   // Simple versions of marking accessors, to be used outside of marking (e.g. no possible concurrent updates)
-  inline bool is_marked(oop) const;
+  inline bool is_marked(oop obj) const;
+  inline bool is_marked(HeapWord* raw_obj) const;
   inline bool is_marked_strong(oop obj) const;
+  inline bool is_marked_strong(HeapWord* raw_obj) const;
   inline bool is_marked_weak(oop obj) const;
 
   inline HeapWord* get_next_marked_addr(HeapWord* addr, HeapWord* limit) const;

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp

@@ -38,11 +38,19 @@ inline bool ShenandoahMarkingContext::mark_weak(oop obj) {
 }
 
 inline bool ShenandoahMarkingContext::is_marked(oop obj) const {
-  return allocated_after_mark_start(obj) || _mark_bit_map.is_marked(cast_from_oop<HeapWord *>(obj));
+  return is_marked(cast_from_oop<HeapWord*>(obj));
+}
+
+inline bool ShenandoahMarkingContext::is_marked(HeapWord* raw_obj) const {
+  return allocated_after_mark_start(raw_obj) || _mark_bit_map.is_marked(raw_obj);
 }
 
 inline bool ShenandoahMarkingContext::is_marked_strong(oop obj) const {
-  return allocated_after_mark_start(obj) || _mark_bit_map.is_marked_strong(cast_from_oop<HeapWord*>(obj));
+  return is_marked_strong(cast_from_oop<HeapWord*>(obj));
+}
+
+inline bool ShenandoahMarkingContext::is_marked_strong(HeapWord* raw_obj) const {
+  return allocated_after_mark_start(raw_obj) || _mark_bit_map.is_marked_strong(raw_obj);
 }
 
 inline bool ShenandoahMarkingContext::is_marked_weak(oop obj) const {