8237632: Shenandoah: accept NULL fwdptr to cooperate with JVMTI and JFR

Reviewed-by: shade, rkennke

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

@@ -140,7 +140,7 @@ oop ShenandoahBarrierSet::load_reference_barrier_mutator_work(oop obj, T* load_a
   assert(ShenandoahLoadRefBarrier, "should be enabled");
   shenandoah_assert_in_cset(load_addr, obj);
 
-  oop fwd = resolve_forwarded_not_null(obj);
+  oop fwd = resolve_forwarded_not_null_mutator(obj);
   if (obj == fwd) {
     assert(_heap->is_gc_in_progress_mask(ShenandoahHeap::EVACUATION | ShenandoahHeap::TRAVERSAL),
            "evac should be in progress");
@@ -173,7 +173,7 @@ oop ShenandoahBarrierSet::load_reference_barrier_mutator_work(oop obj, T* load_a
       size_t count = 0;
       while ((cur < r->top()) && ctx->is_marked(oop(cur)) && (count++ < max)) {
         oop cur_oop = oop(cur);
-        if (cur_oop == resolve_forwarded_not_null(cur_oop)) {
+        if (cur_oop == resolve_forwarded_not_null_mutator(cur_oop)) {
           _heap->evacuate_object(cur_oop, thread);
         }
         cur = cur + cur_oop->size();

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

@@ -84,6 +84,7 @@ public:
   virtual void on_thread_detach(Thread* thread);
 
   static inline oop resolve_forwarded_not_null(oop p);
+  static inline oop resolve_forwarded_not_null_mutator(oop p);
   static inline oop resolve_forwarded(oop p);
 
   template <DecoratorSet decorators, typename T>

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

@@ -49,6 +49,10 @@ inline oop ShenandoahBarrierSet::resolve_forwarded(oop p) {
   }
 }
 
+inline oop ShenandoahBarrierSet::resolve_forwarded_not_null_mutator(oop p) {
+  return ShenandoahForwarding::get_forwardee_mutator(p);
+}
+
 inline void ShenandoahBarrierSet::enqueue(oop obj) {
   shenandoah_assert_not_forwarded_if(NULL, obj, _heap->is_concurrent_traversal_in_progress());
   assert(_satb_mark_queue_set.is_active(), "only get here when SATB active");

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

@@ -34,6 +34,10 @@ public:
    */
   static inline oop get_forwardee(oop obj);
 
+  /* Gets forwardee from the given object. Only from mutator thread.
+   */
+  static inline oop get_forwardee_mutator(oop obj);
+
   /* Returns the raw value from forwardee slot.
    */
   static inline HeapWord* get_forwardee_raw(oop obj);

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

@@ -28,6 +28,7 @@
 #include "gc/shenandoah/shenandoahAsserts.hpp"
 #include "gc/shenandoah/shenandoahForwarding.hpp"
 #include "oops/markWord.inline.hpp"
+#include "runtime/thread.hpp"
 
 inline HeapWord* ShenandoahForwarding::get_forwardee_raw(oop obj) {
   shenandoah_assert_in_heap(NULL, obj);
@@ -35,11 +36,32 @@ inline HeapWord* ShenandoahForwarding::get_forwardee_raw(oop obj) {
 }
 
 inline HeapWord* ShenandoahForwarding::get_forwardee_raw_unchecked(oop obj) {
+  // JVMTI and JFR code use mark words for marking objects for their needs.
+  // On this path, we can encounter the "marked" object, but with NULL
+  // fwdptr. That object is still not forwarded, and we need to return
+  // the object itself.
   markWord mark = obj->mark_raw();
   if (mark.is_marked()) {
-    return (HeapWord*) mark.clear_lock_bits().to_pointer();
+    HeapWord* fwdptr = (HeapWord*) mark.clear_lock_bits().to_pointer();
+    if (fwdptr != NULL) {
+      return fwdptr;
+    }
+  }
+  return cast_from_oop<HeapWord*>(obj);
+}
+
+inline oop ShenandoahForwarding::get_forwardee_mutator(oop obj) {
+  // Same as above, but mutator thread cannot ever see NULL forwardee.
+  shenandoah_assert_correct(NULL, obj);
+  assert(Thread::current()->is_Java_thread(), "Must be a mutator thread");
+
+  markWord mark = obj->mark_raw();
+  if (mark.is_marked()) {
+    HeapWord* fwdptr = (HeapWord*) mark.clear_lock_bits().to_pointer();
+    assert(fwdptr != NULL, "Forwarding pointer is never null here");
+    return cast_to_oop(fwdptr);
   } else {
-    return cast_from_oop<HeapWord*>(obj);
+    return obj;
   }
 }
 

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

@@ -1230,28 +1230,20 @@ class ObjectIterateScanRootClosure : public BasicOopIterateClosure {
 private:
   MarkBitMap* _bitmap;
   Stack<oop,mtGC>* _oop_stack;
+  ShenandoahHeap* const _heap;
+  ShenandoahMarkingContext* const _marking_context;
 
   template <class T>
   void do_oop_work(T* p) {
     T o = RawAccess<>::oop_load(p);
     if (!CompressedOops::is_null(o)) {
       oop obj = CompressedOops::decode_not_null(o);
-      oop fwd = (oop) ShenandoahForwarding::get_forwardee_raw_unchecked(obj);
-      if (fwd == NULL) {
-        // There is an odd interaction with VM_HeapWalkOperation, see jvmtiTagMap.cpp.
-        //
-        // That operation walks the reachable objects on its own, storing the marking
-        // wavefront in the object marks. When it is done, it calls the CollectedHeap
-        // to iterate over all objects to clean up the mess. When it reaches here,
-        // the Shenandoah fwdptr resolution code encounters the marked objects with
-        // NULL forwardee. Trying to act on that would crash the VM. Or fail the
-        // asserts, should we go for resolve_forwarded_pointer(obj).
-        //
-        // Therefore, we have to dodge it by doing the raw access to forwardee, and
-        // assuming the object had no forwardee, if that thing is NULL.
-      } else {
-        obj = fwd;
+      if (_heap->is_concurrent_root_in_progress() && !_marking_context->is_marked(obj)) {
+        // There may be dead oops in weak roots in concurrent root phase, do not touch them.
+        return;
       }
+      obj = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
+
       assert(oopDesc::is_oop(obj), "must be a valid oop");
       if (!_bitmap->is_marked(obj)) {
         _bitmap->mark(obj);
@@ -1261,7 +1253,8 @@ private:
   }
 public:
   ObjectIterateScanRootClosure(MarkBitMap* bitmap, Stack<oop,mtGC>* oop_stack) :
-    _bitmap(bitmap), _oop_stack(oop_stack) {}
+    _bitmap(bitmap), _oop_stack(oop_stack), _heap(ShenandoahHeap::heap()),
+    _marking_context(_heap->marking_context()) {}
   void do_oop(oop* p)       { do_oop_work(p); }
   void do_oop(narrowOop* p) { do_oop_work(p); }
 };
@@ -1307,13 +1300,7 @@ void ShenandoahHeap::object_iterate(ObjectClosure* cl) {
   ShenandoahHeapIterationRootScanner rp;
   ObjectIterateScanRootClosure oops(&_aux_bit_map, &oop_stack);
 
-  // When concurrent root is in progress, weak roots may contain dead oops, they should not be used
-  // for root scanning.
-  if (is_concurrent_root_in_progress()) {
-    rp.strong_roots_do(&oops);
-  } else {
-    rp.roots_do(&oops);
-  }
+  rp.roots_do(&oops);
 
   // Work through the oop stack to traverse heap.
   while (! oop_stack.is_empty()) {

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

@@ -281,17 +281,3 @@ void ShenandoahRootAdjuster::roots_do(uint worker_id, OopClosure* oops) {
    _weak_roots.oops_do<OopClosure>(oops, 0);
    _dedup_roots.oops_do(&always_true, oops, 0);
  }
-
- void ShenandoahHeapIterationRootScanner::strong_roots_do(OopClosure* oops) {
-   assert(Thread::current()->is_VM_thread(), "Only by VM thread");
-   // Must use _claim_none to avoid interfering with concurrent CLDG iteration
-   CLDToOopClosure clds(oops, ClassLoaderData::_claim_none);
-   MarkingCodeBlobClosure code(oops, !CodeBlobToOopClosure::FixRelocations);
-   ShenandoahParallelOopsDoThreadClosure tc_cl(oops, &code, NULL);
-   ResourceMark rm;
-
-   _serial_roots.oops_do(oops, 0);
-   _vm_roots.oops_do(oops, 0);
-   _cld_roots.always_strong_cld_do(&clds, 0);
-   _thread_roots.threads_do(&tc_cl, 0);
- }

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

@@ -273,7 +273,6 @@ public:
   ShenandoahHeapIterationRootScanner();
 
   void roots_do(OopClosure* cl);
-  void strong_roots_do(OopClosure* cl);
 };
 
 // Evacuate all roots at a safepoint