[urgent] RFR (XS): 8166207: Wrong use of Copy::conjoint_oops_atomic in global mark stack

[Kim Barrett]

> On Sep 19, 2016, at 11:13 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi,
> 
> On Mon, 2016-09-19 at 17:07 +0200, Thomas Schatzl wrote:
>> Hi all,
>> 
>>   can I have reviews for this change that does not
>> use Copy::conjoint_oops_atomic() to copy oops outside of the java
>> heap
>> introduced in JDK-8159422?
>> 
>> This is because on some platforms the method is implemented and only
>> ever used (apart from this one) to copy oops (oop references) *on the
>> heap*.
>> 
>> Their size may vary depending on UseCompressedOops. This crashes (or
>> asserts) on 64 bit arm platforms because their implementation of this
>> method directly uses this. Which means that in effect the code only
>> copies half of the intended data.
>> Other implementations use 
> 
>   ... overloading, not the flag, to accidentally (I think) get the
> correct behavior.

There are two overloads, one for oop*, one for narrowOop*.  It is up to the caller to deal with UseCompressedOops dispatching.

Unfortunately:

- linux_aarch64 pd_conjoint_oops_atomic asserts !UseCompressedOops.  That’s a bug.

- There is also a closed port implementing pd_conjoint_oops_atomic with a dispatch on UseCompressedOops, which is also a bug.

All the other ports implement pd_conjoint_oops_atomic properly.

I don’t know if there might be any fannout from fixing the two buggy pd_conjoint_oops_atomic, since I haven’t examined all callers.  Hopefully not, but without review and test…

Given that, I’m okay with the proposed change for 8166207.

> 
>> The suggested fix is to revert to the use of
>> Copy::conjoint_memory_atomic to copy data as suggested in an early
>> version of the JDK-8159422, and in a follow-up look what the intended
>> meaning of this method actually is and eventually fix the methods.
>> 
>> This will remove a lot of noise on the arm64 nightlies.
>> 
>> Thanks go to Dean Long for finding the root problem first.
>> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8166207
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8166207/webrev/
>> Testing:
>> passes jprt, waiting for rbt to pick up crashing tests with it
> 
> Still did not start. I am however very confident that this is the
> correct fix.
> 
> Thanks,
>   Thomas