RFR: JDK-8222545: Safe klass asserts

[Roman Kennke]

To add a little more detail, I could move the change up into is_objArray(), but I don't want to expose it to any non-assert paths. Therefore I could do 2 different impls there, guarded by #ifdef ASSERT but I don't think it's a good idea to behave differently under ASSERT, that kindof defeats the point of assert, right?

What do you think ?

Roman


Am 17. April 2019 18:59:09 MESZ schrieb Roman Kennke <rkennke at redhat.com>:
>>> 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) ?
>
>Not really. Then it would get exposed to many more code paths, most of 
>which don't actually need it/don't want it, and many of which are 
>outside of asserts, and rely on the usual klass() with the sanity
>assert 
>there instead. I am open for suggestions, but it would have to be 
>restricted to ASSERT code IMO, and ideally with as few as possible GC 
>interface additions.
>
>Roman

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190418/aafadb0b/attachment.htm>