RFR: 8199739: Use HeapAccess when loading oops from static fields in javaClasses.cpp

[Kim Barrett]

> On Mar 17, 2018, at 6:39 AM, Roman Kennke <rkennke at redhat.com> wrote:
> 
> Am 17.03.2018 um 05:11 schrieb Kim Barrett:
>>> On Mar 16, 2018, at 10:39 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>> 
>>> Hi all,
>>> 
>>> Please review this patch to use HeapAccess<>::oop_load instead of oopDesc::load_decode_heap_oop when loading oops from static fields in javaClasses.cpp:
>>> 
>>> http://cr.openjdk.java.net/~stefank/8199739/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8199739
>>> 
>>> It's necessary to use HeapAccess<>::oop_load to inject load barriers for GCs that need them.
>>> 
>>> Thanks,
>>> StefanK
>> 
>> ------------------------------------------------------------------------------ 
>> src/hotspot/share/classfile/javaClasses.cpp.
>> 
>> 1870   address addr = ik->static_field_addr(static_unassigned_stacktrace_offset);
>> 1871   return HeapAccess<>::oop_load((HeapWord*)addr);
>> 
>> I'm not sure this is sufficient.  Isn't static_field_addr just
>> fundamentally broken for Shenandoah?
> 
> It is not totally broken (I think _addr() calls resolve() which kinda
> does what we need),

Current implementation of static_field_addr is:

address InstanceKlass::static_field_addr(int offset) { 
  assert(offset >= InstanceMirrorKlass::offset_of_static_fields(), "has already been adjusted"); 
  return (address)(offset + cast_from_oop<intptr_t>(java_mirror()));
}

java_mirror() calls OopHandle::resolve(), which presently doesn’t do anything wrto Access
(has OopHandle not yet been Accessorized?  Or is there some reason why it’s okay as is
that I’m not thinking of right now?)  Even if it did use Access, I would expect for Shenandoah
the access would just be a dereference of the oop* in the OopHandle, and would not chase
the Brooks pointer, since we’re not accessing fields in the mirror object at that point.

>  but the following would be perfect:
> 
>> but I think such an access needs to be 
>> HeapAccess<>::load_at(mirror, offset)
> 
> Thank you, Roman