[aarch64-port-dev ] RFR(s): 8171449" [aarch64] store_klass needs to use store release

[Andrew Haley]

Hi,

On 23/12/16 13:44, Thomas Schatzl wrote:
> On Fri, 2016-12-23 at 13:35 +0000, Andrew Haley wrote:
>> On 23/12/16 12:31, Thomas Schatzl wrote:
>>>
>>> I hope this answers your question.
>>>
>>> Given this I tend to think that there is no issue here after all,
>>> but
>>> please by all means, verify this.
>> OK.  I'm going to step through to see if that is what really happens.
>> I'm pretty sure that when I did step through I didn't see any NULLing
>> of the klass field, but I take it from your answer that the only time
>> this actually needs to happen is when an object is allocated by the
>> C++ runtime code, not by the JIT-generated code or the JIT's runtime
>> libraries, which only ever allocated in the young gen.
> 
> The important thing to consider here is that the FreeChunk data
> structure is overlaid over the future object. I.e. when
> FreeChunk::mark_not_free() is called, by nulling out _prev, it actually
> also nulls out the klass field of the future object.
> 
> There is actually a comment indicating exactly that in that method.

Yes, I see that code.

I think Derek is right, and there is no bug here.  Compiled code and
the template interpreter allocate in the young generation, and this
isn't visible to the collectors.  Whenever compiled code initializes
an object, the memory it overwrites is garbage.  When memory is
allocated in the old generation it's always in the runtime slow path,
and in that case there are plenty of memory fences when the object is
initialized.

We could argue that we should stick to this contract with the
collectors: whenever we write the klass field we should do so with a
store release.  But that would mean there are two or three fences for
every object allocation.

If we really did want to do this on AArch64, we could better by
zeroing the object first, writing the length field, then storing the
klass field with stlr.  One fence rather than two would be better than
what we do at the moment, which is storing the klass field then
zeroing the object.  But, as I said, I don't think that we need to.
Which is why I don't think we've seen this "bug" causing mysterious
crashes.

Andrew.