8337981: ShenandoahHeap::is_in should check for alive regions

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

@@ -699,7 +699,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

@@ -710,9 +710,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::op_uncommit(double shrink_before, size_t shrink_until) {

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

@@ -469,6 +469,8 @@ class ShenandoahHeap : public CollectedHeap {
 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 {

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

@@ -83,10 +83,21 @@ static volatile T* reference_referent_addr(oop reference) {
   return (volatile T*)java_lang_ref_Reference::referent_addr_raw(reference);
 }
 
+inline oop reference_coop_decode_raw(narrowOop v) {
+  return CompressedOops::is_null(v) ? nullptr : CompressedOops::decode_raw(v);
+}
+
+inline oop reference_coop_decode_raw(oop v) {
+  return v;
+}
+
+// Raw referent, it can be dead. You cannot treat it as oop without additional safety
+// checks, this is why it is HeapWord*. The decoding uses a special-case inlined
+// CompressedOops::decode method that bypasses normal oop-ness checks.
 template <typename T>
-static oop reference_referent(oop reference) {
-  T heap_oop = Atomic::load(reference_referent_addr<T>(reference));
-  return CompressedOops::decode(heap_oop);
+static HeapWord* reference_referent_raw(oop reference) {
+  T raw_oop = Atomic::load(reference_referent_addr<T>(reference));
+  return cast_from_oop<HeapWord*>(reference_coop_decode_raw(raw_oop));
 }
 
 static void reference_clear_referent(oop reference) {
@@ -278,8 +289,8 @@ bool ShenandoahReferenceProcessor::should_discover(oop reference, ReferenceType
 
 template <typename T>
 bool ShenandoahReferenceProcessor::should_drop(oop reference, ReferenceType type) const {
-  const oop referent = reference_referent<T>(reference);
-  if (referent == nullptr) {
+  HeapWord* raw_referent = reference_referent_raw<T>(reference);
+  if (raw_referent == nullptr) {
     // Reference has been cleared, by a call to Reference.enqueue()
     // or Reference.clear() from the application, which means we
     // should drop the reference.
@@ -289,9 +300,9 @@ bool ShenandoahReferenceProcessor::should_drop(oop reference, ReferenceType type
   // Check if the referent is still alive, in which case we should
   // drop the reference.
   if (type == REF_PHANTOM) {
-    return ShenandoahHeap::heap()->complete_marking_context()->is_marked(referent);
+    return ShenandoahHeap::heap()->complete_marking_context()->is_marked(raw_referent);
   } else {
-    return ShenandoahHeap::heap()->complete_marking_context()->is_marked_strong(referent);
+    return ShenandoahHeap::heap()->complete_marking_context()->is_marked_strong(raw_referent);
   }
 }
 
@@ -303,7 +314,7 @@ void ShenandoahReferenceProcessor::make_inactive(oop reference, ReferenceType ty
     // next field. An application can't call FinalReference.enqueue(), so there is
     // no race to worry about when setting the next field.
     assert(reference_next<T>(reference) == nullptr, "Already inactive");
-    assert(ShenandoahHeap::heap()->marking_context()->is_marked(reference_referent<T>(reference)), "only make inactive final refs with alive referents");
+    assert(ShenandoahHeap::heap()->marking_context()->is_marked(reference_referent_raw<T>(reference)), "only make inactive final refs with alive referents");
     reference_set_next(reference, reference);
   } else {
     // Clear referent
@@ -376,8 +387,8 @@ oop ShenandoahReferenceProcessor::drop(oop reference, ReferenceType type) {
   log_trace(gc, ref)("Dropped Reference: " PTR_FORMAT " (%s)", p2i(reference), reference_type_name(type));
 
 #ifdef ASSERT
-  oop referent = reference_referent<T>(reference);
-  assert(referent == nullptr || ShenandoahHeap::heap()->marking_context()->is_marked(referent),
+  HeapWord* raw_referent = reference_referent_raw<T>(reference);
+  assert(raw_referent == nullptr || ShenandoahHeap::heap()->marking_context()->is_marked(raw_referent),
          "only drop references with alive referents");
 #endif
 

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

@@ -51,13 +51,6 @@ static bool is_instance_ref_klass(Klass* k) {
   return k->is_instance_klass() && InstanceKlass::cast(k)->reference_type() != REF_NONE;
 }
 
-class ShenandoahIgnoreReferenceDiscoverer : public ReferenceDiscoverer {
-public:
-  virtual bool discover_reference(oop obj, ReferenceType type) {
-    return true;
-  }
-};
-
 class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
 private:
   const char* _phase;
@@ -68,6 +61,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
   ShenandoahLivenessData* _ld;
   void* _interior_loc;
   oop _loc;
+  ReferenceIterationMode _ref_mode;
 
 public:
   ShenandoahVerifyOopClosure(ShenandoahVerifierStack* stack, MarkBitMap* map, ShenandoahLivenessData* ld,
@@ -82,10 +76,20 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
     _loc(nullptr) {
     if (options._verify_marked == ShenandoahVerifier::_verify_marked_complete_except_references ||
         options._verify_marked == ShenandoahVerifier::_verify_marked_disable) {
-      set_ref_discoverer_internal(new ShenandoahIgnoreReferenceDiscoverer());
+      // Unknown status for Reference.referent field. Do not touch it, it might be dead.
+      // Normally, barriers would prevent us from seeing the dead referents, but verifier
+      // runs with barriers disabled.
+      _ref_mode = DO_FIELDS_EXCEPT_REFERENT;
+    } else {
+      // Otherwise do all fields.
+      _ref_mode = DO_FIELDS;
     }
   }
 
+  ReferenceIterationMode reference_iteration_mode() override {
+    return _ref_mode;
+  }
+
 private:
   void check(ShenandoahAsserts::SafeLevel level, oop obj, bool test, const char* label) {
     if (!test) {
@@ -119,8 +123,8 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
     // that failure report would not try to touch something that was not yet verified to be
     // safe to process.
 
-    check(ShenandoahAsserts::_safe_unknown, obj, _heap->is_in(obj),
-              "oop must be in heap");
+    check(ShenandoahAsserts::_safe_unknown, obj, _heap->is_in_reserved(obj),
+              "oop must be in heap bounds");
     check(ShenandoahAsserts::_safe_unknown, obj, is_object_aligned(obj),
               "oop must be aligned");
 
@@ -177,8 +181,8 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
     ShenandoahHeapRegion* fwd_reg = nullptr;
 
     if (obj != fwd) {
-      check(ShenandoahAsserts::_safe_oop, obj, _heap->is_in(fwd),
-             "Forwardee must be in heap");
+      check(ShenandoahAsserts::_safe_oop, obj, _heap->is_in_reserved(fwd),
+             "Forwardee must be in heap bounds");
       check(ShenandoahAsserts::_safe_oop, obj, !CompressedOops::is_null(fwd),
              "Forwardee is set");
       check(ShenandoahAsserts::_safe_oop, obj, is_object_aligned(fwd),
@@ -195,6 +199,9 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
 
       fwd_reg = _heap->heap_region_containing(fwd);
 
+      check(ShenandoahAsserts::_safe_oop, obj, fwd_reg->is_active(),
+            "Forwardee should be in active region");
+
       // Verify that forwardee is not in the dead space:
       check(ShenandoahAsserts::_safe_oop, obj, !fwd_reg->is_humongous(),
              "Should have no humongous forwardees");
@@ -309,8 +316,8 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
     _loc = nullptr;
   }
 
-  virtual void do_oop(oop* p) { do_oop_work(p); }
-  virtual void do_oop(narrowOop* p) { do_oop_work(p); }
+  virtual void do_oop(oop* p) override { do_oop_work(p); }
+  virtual void do_oop(narrowOop* p) override { do_oop_work(p); }
 };
 
 class ShenandoahCalculateRegionStatsClosure : public ShenandoahHeapRegionClosure {