RFR(S): 8221083: [ppc64] Wrong oop compare in C1-generated code

[Volker Simonis]

On Tue, Mar 26, 2019 at 8:10 AM Lindenmaier, Goetz
<goetz.lindenmaier at sap.com> wrote:
>
> Hi Volker,
>
> so basically the code handled pointers (Object references) the
> same as integers and other 32-bit values, because they take up
> on Java 'slot'?  Is that what "is_single_cpu()" means?
>

Yes, exactly. "is_single_cpu()" means that it uses one Java 'slot' but
if it's an object or an array we still need to use a 64-bit compare.

> If so, the fix looks good and complete.  Reviewed.

Thanks.

>
> For the test, while it is extremely unlikely that it ever fails,
> (now after the code has been fixed),
> I think it should throw a RuntimeException with a message
> like "wrong compare resulting in assumption of reference being null"
> or the like, instead of throwing a NPE.
> Don't need a new webrev in case you want to adapt this.
>

Done.

> Best regards,
>   Goetz.
>
> > -----Original Message-----
> > From: hotspot-compiler-dev <hotspot-compiler-dev-
> > bounces at openjdk.java.net> On Behalf Of Volker Simonis
> > Sent: Dienstag, 19. März 2019 19:54
> > To: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
> > Subject: RFR(S): 8221083: [ppc64] Wrong oop compare in C1-generated code
> >
> > Hi,
> >
> > can I please have a review for the following small ppc64-only C1 patch
> > which fixes a nasty, day-one problem which only recently popped up
> > more frequently:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2019/8221083/
> > https://bugs.openjdk.java.net/browse/JDK-8221083
> >
> > The C1 generated code for comparing two oops erroneously emits a
> > 32-bit instead of an 64-bit compare instruction. Because oops are only
> > compared for equality/inequality, this bug only becomes manifests for
> > oops which are equal in their 32 least-significant bits but unequal
> > otherwise. This means the two oops have to be exactly 4GB apart from
> > each other in the heap or their 32 least significant bits have to be
> > zero when compared to 'null'.
> >
> > This makes the occurrence of this bug extremely unlikely, but when it
> > happens, the consequences are usually a semantically wrong program
> > execution and not a crash, which makes it very hard to detect.
> >
> > The regression test reproduces the issue by allocation an object at an
> > address with the 32-bit least significant bits being zero and comperes
> > it with another null object.
> >
> > The fix also removes some adjacent code which has never been used (and
> > tested) until now.
> >
> > Thank you and best regards,
> > Volker