RFR: 8224671: AArch64: mauve System.arraycopy test failure

[Andrew Dinn]

Oh, nice catch.

The patch looks good for head and for all backports so count it as reviewed.

Are those the only cases where we need a variant with an unsigned int
argument?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

On 23/05/2019 13:18, Andrew Haley wrote:
> Well, this one was a doozy.
> 
> Mauve is an old Java test suite, from many years ago. Thankfully
> though, AdoptOpenJDK still run it, and we received a surprising report
> about a test failure in System.arraycopy().
> 
> It turns out that in C1-compiled code an ArrayStoreException is
> wrongly thrown as an ArrayIndexOutOfBoundsException. After a lot of
> debugging, the bug turned out to be the calculation of ~x, a bitwise
> inversion:
> 
>      __ eonw(rscratch1, r0, 0);
> 
> This looks reasonable enough: EOR r0 with ~0. But it's wrong: the EONW
> instruction has no immediate form. So how are we not getting an
> assembly-time error? This turns out to be a misfeature of C++ combined
> with the nasty way we declare registers in HotSpot.
> 
> When C++ processes the overloads for eonw() it looks first for an
> exact match of the immediate operand then applies the default integer
> conversions. One of these silently (!) converts 0 to a NULL
> pointer. Unfortunately (very unfortunately) the register declaration
> for r0 is a NULL pointer, so
> 
>      __ eonw(rscratch1, r0, 0);
> 
> generates
> 
>      eonw x8, r0, r0
> 
> which always sets r0 to -1, which causes the ArrayIndexOutOfBoundsException.
> 
> The fix is to use ZR instead of r0.
> 
> In order to make sure that this never happens again I've provided a
> declaration for the immediate form of these instructions. It will
> generate a compile-time error if this mistake ever happens again.
> 
> http://cr.openjdk.java.net/~aph/8224671/jdk-test.changeset
> 
> We'll need backports for all releases.
>