RFR 8225573: Shenandoah: Enhance ShenandoahVerifier to ensure roots to-space invariant

[Zhengyu Gu]

On 6/18/19 2:46 PM, Aleksey Shipilev wrote:
> On 6/18/19 6:57 PM, Zhengyu Gu wrote:
>> On 6/11/19 1:21 PM, Zhengyu Gu wrote:
>>> This patch adds a new API to Shenandoah verifier, the verifies all roots only contain to-space
>>> references. This API should help concurrent root processing work, to catch bad references in roots.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225573
>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8225573/webrev.00/8225573
>>
>> Sorry, bad link. Should be:
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8225573/webrev.00/
> 
> Looks okay.
> 
> *) The mix of SA::assert_* and SA::print_failure irritates me. I think it is cleaner to stick with
> one way of doing things. "assert" kinda implies in can be turned off (even though it does not in
> this case). For example:
> 
> class ShenandoahVerifyInToSpaceClosure : public OopClosure {
> private:
>    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);
> 
>        ShenandoahHeap* heap = ShenandoahHeap::heap_no_check();
> 
>        if (!heap->marking_context()->is_marked(obj)) {
>          ShenandoahAsserts::print_failure(ShenandoahAsserts::_safe_all, obj, p, NULL,
>                  "Verify Roots In To-Space", "Should be marked", __FILE__, __LINE__);
>        }
> 
>        if (heap->in_collection_set(obj)) {
>          ShenandoahAsserts::print_failure(ShenandoahAsserts::_safe_all, obj, p, NULL,
>                  "Verify Roots In To-Space", "Should not be in collection set", __FILE__, __LINE__);
>        }
> 
>        oop fwd = (oop) ShenandoahForwarding::get_forwardee_raw_unchecked(obj);
>        if (!oopDesc::equals_raw(obj, fwd)) {
>          ShenandoahAsserts::print_failure(ShenandoahAsserts::_safe_all, obj, p, NULL,
>                  "Verify Roots In To-Space", "Should not be forwarded", __FILE__, __LINE__);
>        }
>      }
>    }

Took this version and pushed.

Thanks,

-Zhengyu

> 
> -Aleksey
>