RFR(xs): java.lang.Class.isPrimitive() (C1) returns wrong result if Klass* is aligned to 32bit

[Thomas Stüfe]

Just occurred to me that my proposed fix could break non-x86. I have to
think this through a bit more. I'd still be thankful for feedback.

Cheers, Thomas

On Fri, Oct 25, 2019, 16:57 Thomas Stüfe <thomas.stuefe at gmail.com> wrote:

>
> Hello,
>
> Could I please have opinions and reviews for the following fix.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8233019
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8233019-c1-isprimitive/webrev.00/webrev/
>
>
> While working with the prototype of the new metaspace allocator
> (JDK-8221173), I saw weird errors which looked like
> java.lang.Class.isPrimitive() returning sometimes incorrectly true for
> boolean[].class, which is not a primitive.
>
> The bug seems to only happen in c1.
>
> boolean[].class is the very first Klass* ever to be created in the life of
> a VM. For JDK-8221173 I removed metachunk headers from their payload. One
> unintended side effect is that the first Klass* is now allocated directly
> at the base of class space. If the class space base is aligned to 32bit,
> the Klass* for boolean[].class is so too, and all lower 32bits are zero.
>
> That seems to trigger what I think is a bug in C1, in isPrimitive. There,
> the Klass* pointer gets loaded from the mirror, and then compared with zero
> (class objects for primitives have no associated Klass*, so isPrimitive()
> returns true if that Klass* pointer is NULL).
>
> But to me it seems that this comparison is done with 32bit cmp:
>
> -------------
>
>  6512 Compiled method (c1)     822   31       1
> java.lang.invoke.MethodTypeForm::canonicalize (233 bytes)
> ..<snip>..
>
>  6544  ;;  block B2 [9, 13]
>  6545   0x00007f2ca8e76f3f:   mov    0x50(%rsi),%rdi              ;
> implicit exception: dispatches to 0x00007f2ca8e771ec
>  6546   0x00007f2ca8e76f43:   cmp    $0x0,%edi      <<<< 32bit?
>  6547   0x00007f2ca8e76f46:   mov    $0x0,%edi
>  6548   0x00007f2ca8e76f4b:   jne    0x00007f2ca8e76f56
>  6549   0x00007f2ca8e76f51:   mov    $0x1,%edi
>  ;*invokevirtual isPrimitive {reexecute=0 rethrow=0 return_oop=0}
>  6550                                                             ; -
> java.lang.invoke.MethodTypeForm::canonicalize at 10 (line 263)
>
> -------------
>
> and therefore returns true for Klass pointers like 0x900000000.
>
> My fix makes the comparison 64bit.
>
> Would that be the correct way to fix it? Please note that I am new to the
> compiler and this is my first fix in c1.
>
> At least on my system the fix seems to solve the error, and isPrimitive
> now uses a 64bit compare, and the test errors are gone:
>
> 6544  ;;  block B2 [9, 13]
>  6545   0x00007f7750e76f3f:   mov    0x50(%rsi),%rdi              ;
> implicit exception: dispatches to 0x00007f7750e771ec
>  6546   0x00007f7750e76f43:   cmp    $0x0,%rdi       <<<<<
>  6547   0x00007f7750e76f47:   mov    $0x0,%edi
>  6548   0x00007f7750e76f4c:   jne    0x00007f7750e76f57
>  6549   0x00007f7750e76f52:   mov    $0x1,%edi
>  ;*invokevirtual isPrimitive {reexecute=0 rethrow=0 return_oop=0}
>  6550                                                             ; -
> java.lang.invoke.MethodTypeForm::canonicalize at 10 (line 263)
>
>
> Thanks, Thomas
>
>