# HG changeset patch # User zgu # Date 1582749145 18000 # Wed Feb 26 15:32:25 2020 -0500 # Node ID a5f3d7016a3cfc582b5c17436c05197802111b5d # Parent 695c6b0986c390485cfbc9277b96bac49ebc5e34 8237632: Shenandoah fails some vmTestbase_nsk_jvmti tests with "Forwardee must point to a heap address" diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.cpp @@ -140,7 +140,7 @@ 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 @@ 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(); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp @@ -84,6 +84,7 @@ 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 diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp @@ -49,6 +49,10 @@ } } +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"); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahForwarding.hpp b/src/hotspot/share/gc/shenandoah/shenandoahForwarding.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahForwarding.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahForwarding.hpp @@ -34,6 +34,10 @@ */ 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); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahForwarding.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahForwarding.inline.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahForwarding.inline.hpp +++ b/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_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(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(obj); + return obj; } } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp @@ -1230,28 +1230,20 @@ private: MarkBitMap* _bitmap; Stack* _oop_stack; + ShenandoahHeap* const _heap; + ShenandoahMarkingContext* const _marking_context; template 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 @@ } public: ObjectIterateScanRootClosure(MarkBitMap* bitmap, Stack* 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 @@ 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()) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.cpp @@ -281,17 +281,3 @@ _weak_roots.oops_do(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); - } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp --- a/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahRootProcessor.hpp @@ -273,7 +273,6 @@ ShenandoahHeapIterationRootScanner(); void roots_do(OopClosure* cl); - void strong_roots_do(OopClosure* cl); }; // Evacuate all roots at a safepoint