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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Mar 26 07:10:44 UTC 2019


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?

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

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.

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


More information about the hotspot-compiler-dev mailing list