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

[Volker Simonis]

Hi Florian, Martin,

thanks for looking at the change.

I think Martin is right, we either get an object allocated at
0x700000000 right with the first allocation after System.gc() or it is
unlikely that we'll get it at all. The loops are actually a leftover
from my first experiments with other GC's like G1, but in the end I
decided to explicitly use SerialGC because it was the simplest and
most reliably way of allocation an object at the desired address.

Please find the new webrev here:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8221083.v1/

Thank you and best regards,
Volker

On Wed, Mar 20, 2019 at 12:24 PM Doerr, Martin <martin.doerr at sap.com> wrote:
>
> Hi Volker,
>
> I think the 2 nested loops are not really needed.
> If you use System.gc() + new String you should get it allocated at the desired location.
> I believe this is sufficient with the flags you are using.
>
> The test seems to test interpreter, C1, and C2 with the loop limit of 30_000. I like that.
>
> The C1 fix looks good.
>
> Thanks for finding and fixing this bug.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Florian Weimer
> Sent: Dienstag, 19. März 2019 22:48
> To: Volker Simonis <volker.simonis at gmail.com>
> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(S): 8221083: [ppc64] Wrong oop compare in C1-generated code
>
> * Volker Simonis:
>
> > 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.
>
>   79         for (int i = 0; i < 10; i++) {
>   80             System.gc();
>   81             for (int j = 0; j < 1024; j++) {
>   82                 s = new String("I'm not null!!!");
>   83                 if (WB.getObjectAddress(s) == 0x700000000L) break;
>   84             }
>   85             if (WB.getObjectAddress(s) == 0x700000000L) {
>   86                 System.out.println("Got object at address 0x700000000");
>   87                 break;
>   88             }
>   89         }
>
> I think this could use a labeled loop, like this:
>
>         GC_TESTS: for (int i = 0; i < 10; i++) {
>             System.gc();
>             for (int j = 0; j < 1024; j++) {
>                 s = new String("I'm not null!!!");
>                 if (WB.getObjectAddress(s) == 0x700000000L) {
>                     System.out.println("Got object at address 0x700000000");
>                     break GC_TESTS;
>                 }
>             }
>         }
>
> (Untested.)
>
> On the other hand, neither this version or yours properly detects when
> s is *not* actually the object with the desired address, in which case
> the test objective fails to materialize, I think.