(S) 8176518: [9] C2: Invalid ImplicitNullChecks with non-protected heap base

[Doerr, Martin]

Hi,

I have experimented on x86 and I get a very similar graph when using -XX:ObjectAlignmentInBytes=16 (prevents oop decoding matched into operands) in addition to the flags below under [1].
If -XX:+UseLargePages (which is used to skip the heap base protection) is supported on Windows, the problem exists there as well.

As the graph pattern on PPC64 is the same as on x86, I believe that there's nothing wrong except the missing check for narrow_oop_use_implicit_null_checks(), which is added by the webrev below.
The remaining question is if there's a better test than "base->as_Mach()->ideal_Opcode() == Op_DecodeN".

I will also need a sponsor for this change, please.

Thanks and best regards,
Martin



From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Doerr, Martin
Sent: Freitag, 10. März 2017 18:49
To: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
Subject: RFR: (S) 8176518: [9] C2: Invalid ImplicitNullChecks with non-protected heap base

Hi,

we have observed that C2 generates implicit null checks for write accesses to the heap base even though it was not protected (narrow_oop_use_implicit_null_checks = false).
The problem showed up on AIX with -XX:HeapBaseMinAddress=32g. ReservedHeapSpace::establish_noaccess_prefix protects the base area on almost all platforms, but not on AIX (in some case).
PhaseCFG::implicit_null_check needs to skip the transformation in this case.

The problem can be reproduced by the simple test program below under [1]. The null pointer exception is just missing when running with the given parameters.

The problem can be prevented by applying this patch:
http://cr.openjdk.java.net/~mdoerr/8176518_C2_NullCheck/webrev.00/

Especially, the case "base->is_Mach() && base->as_Mach()->ideal_Opcode() == Op_DecodeN" gets hit. I'm not sure if this is allowed at that place. Maybe something went wrong before.
The node "base" is a storeI and "addr"="base" is a decodeN node (the "index" retrieved by mach->memory_inputs is NULL).

The patch helps, but is there a better way to fix the problem?

Thanks and best regards,
Martin



[1] Reproduction case:

/jdk/bin/java -XX:HeapBaseMinAddress=32g -XX:-TieredCompilation -Xbatch TestWriteNPE

public class TestWriteNPE{

    TestWriteNPE instance = null;
    int i = 0;

    public void bogus_test(int value) {
        instance.i = value;
    }


    static final int loop_cnt=100000;

    public static void main(String args[]){
        TestWriteNPE xyz=new TestWriteNPE();
        xyz.instance = xyz;
        long duration = System.nanoTime();
        for (int x=0; x<loop_cnt; x++) xyz.bogus_test(x);
        duration = System.nanoTime() - duration;
        System.out.println("value: " + xyz.i + " (duration: " + duration/loop_cnt + ")");
        xyz.instance = null;
        try {
            xyz.bogus_test(0);
        } catch (NullPointerException np) {
            System.out.println("Got NPE:");
            np.printStackTrace();
        }
    }

}

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20170314/bf1dc098/attachment.html>