RFR: JDK-8222545: Safe klass asserts

[David Holmes]

Hi Roman,

On 17/04/2019 5:58 am, Roman Kennke wrote:
> Various code paths in oopDesc, Klass and their subclasses assert
> something that fetches the object's _klass field. With upcoming
> Shenandoah's changes this is not always safe and requires an additional
> indirection.
> 
> The trouble here is that we can, for example, call
> Klass::oop_oop_iterate() with a pre-resolved Klass*, instead of
> oopDesc::oop_iterate() which would call oopDesc::klass() on its own,
> which would be racy on some GC internal call paths, but we can't
> (currently) control some calls to klass() further down the call stack
> (all in asserts).
> 
> We'd also like a way to ensure that non-GC calls to klass() are sane.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8222545
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8222545/webrev.00/
> Testing:
> hotspot_gc_shenandoah with and without the prototype, hotspot/tier1
> 
> The change introduces only two ASSERT-level GC-interfaces, and afaict,
> this with JDK-8222537 will be all that we need for the upcoming
> elimination of forward pointers in Shenandoah. Notice that one assert in
> objArrayKlass is strengthened from is_array() to is_objArray(), but that
> seems only sane in that context.
> 
> Can I please get reviews?

This looks very awkward to me. Using:

Universe::heap()->safe_klass(obj)->is_objArray_klass()

instead of the obvious:

obj->is_objArray()

is very unintuitive. Can this not be handled inside is_objArray (and 
is_typeArray) ?

Thanks,
David

> Thanks,
> Roman
>