RFR(S): 8235762: JVM crash in SWPointer during C2 compilation
Yangfei (Felix)
felix.yang at huawei.com
Wed Dec 11 12:55:31 UTC 2019
Hi Tobias,
I didn't noticed JDK-8235700 when I started to fix this bug several days ago.
Sure, I will prepare a webrev. Comments are welcome.
Thanks,
Felix
> -----Original Message-----
> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> Sent: Wednesday, December 11, 2019 8:50 PM
> To: Yangfei (Felix) <felix.yang at huawei.com>;
> hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR(S): 8235762: JVM crash in SWPointer during C2 compilation
>
> Hi Felix,
>
> is this a duplicate of JDK-8235700?
>
> Please create a webrev and also include the regression test.
>
> Thanks,
> Tobias
>
> On 11.12.19 13:39, Yangfei (Felix) wrote:
> > Hi,
> >
> > Please review this patch fixing a crash in C2 superword transform phase.
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8235762
> >
> > This is similar to JDK-8229694, but a little bit more complex.
> > The JVM crashes in the testcase when trying to dereference mem at [9]
> which is NULL.
> > This happens when converting packs into vector nodes in SuperWord::output()
> by reading an unexpected NULL value with SuperWord::align_to_ref() [8].
> > The corresponding field _align_to_ref is set to NULL at [7] since
> best_align_to_mem_ref was assigned NULL just before at [6].
> > _packset contained two packs and there were no memory operations left
> > to be processed (memops is empty). As a result
> SuperWord::find_align_to_ref() will return NULL since it cannot find an
> alignment from two different types of mem operations.
> >
> > The loop in Test::vMeth is unrolled two times. As a result, main loop
> contains the following 12 memory operations:
> >
> > 954 LoadI === 633 985 955 [[ 953 ]] @int[int:>=0]:exact+any
> *, idx=6; #int !orig=320 !jvms: Test::vMeth @ bci:13
> > 950 StoreI === 981 985 951 953 [[ 946 948 ]]
> @int[int:>=0]:exact+any *, idx=6; Memory: @int[int:>=0]:NotNull:exact+any
> *, idx=6; !orig=342 !jvms: Test::vMeth @ bci:16
> > 948 LoadI === 647 950 949 [[ 947 ]] @int[int:>=0]:exact+any
> *, idx=6; #int !orig=369 !jvms: Test::vMeth @ bci:23
> > 946 StoreI === 981 950 949 947 [[ 320 342 ]]
> @int[int:>=0]:exact+any *, idx=6; Memory: @int[int:>=0]:NotNull:exact+any
> *, idx=6; !orig=391,989 !jvms: Test::vMeth @ bci:26
> > 320 LoadI === 633 946 318 [[ 321 ]] @int[int:>=0]:exact+any
> *, idx=6; #int !jvms: Test::vMeth @ bci:13
> > 342 StoreI === 981 946 340 321 [[ 391 369 ]]
> @int[int:>=0]:exact+any *, idx=6; Memory: @int[int:>=0]:NotNull:exact+any
> *, idx=6; !jvms: Test::vMeth @ bci:16
> > 369 LoadI === 647 342 367 [[ 370 ]] @int[int:>=0]:exact+any
> *, idx=6; #int !jvms: Test::vMeth @ bci:23
> > 391 StoreI === 981 342 367 370 [[ 772 985 ]]
> @int[int:>=0]:exact+any *, idx=6; Memory: @int[int:>=0]:NotNull:exact+any
> *, idx=6; !orig=989 !jvms: Test::vMeth @ bci:26
> >
> > 958 StoreB === 981 986 959 10 [[ 470 ]]
> @byte[int:>=0]:exact+any *, idx=10; Memory:
> @byte[int:>=0]:NotNull:exact+any *, idx=10; !orig=470,[990] !jvms:
> Test::vMeth @ bci:44
> > 470 StoreB === 981 958 468 10 [[ 774 986 ]]
> @byte[int:>=0]:exact+any *, idx=10; Memory:
> @byte[int:>=0]:NotNull:exact+any *, idx=10; !orig=[990] !jvms: Test::vMeth @
> bci:44
> >
> > 961 StoreL === 981 984 962 12 [[ 431 ]]
> @long[int:>=0]:exact+any *, idx=8; Memory:
> @long[int:>=0]:NotNull:exact+any *, idx=8; !orig=431,[991] !jvms: Test::vMeth
> @ bci:35
> > 431 StoreL === 981 961 429 12 [[ 776 984 ]]
> @long[int:>=0]:exact+any *, idx=8; Memory:
> @long[int:>=0]:NotNull:exact+any *, idx=8; !orig=[991] !jvms: Test::vMeth @
> bci:35
> >
> >
> > Consider the while loop at [1]:
> > iter1:
> > memops.size() = 12 mem_ref = 342 alignment: 342=0, 946=0, 391=4,
> > 950=4 create_pack=true _packset: <342, 950> best_align_to_mem_ref
> =
> > 342
> > iter2:
> > memops.size() = 8 mem_ref = 431 alignment: 431=0, 961=8
> > create_pack=true _packset: <342, 950>, <431, 961>
> > best_align_to_mem_ref = 342
> > iter3:
> > memops.size() = 6 mem_ref = 470 alignment: 470=0,958=1
> > create_pack=true _packset: <342, 950>, <431, 961>, <470, 958>
> > best_align_to_mem_ref = 342
> > iter4:
> > memops.size() = 4 mem_ref = 320 alignment: 320=0, 954=4
> > create_pack=true _packset: <342, 950>, <431, 961>, <470, 958>, <320,
> > 954> best_align_to_mem_ref = 342
> > iter5:
> > memops.size() = 2 mem_ref = 369 alignment: 369=0, 948=4
> > create_pack=false _packset: <431, 961>, <470, 958>
> > best_align_to_mem_ref = NULL (<342, 950> and <320, 954> are removed
> > from _packset at [4])
> >
> > For iter5, create_pack is set to false at [2]. As a result, the two memory
> operations in memops are poped at [3]. And 431 and 470 are pushed to
> memops at [5].
> >
> > Then memops for call find_align_to_ref at [6] only contains 431 and 470 which
> are different in type. As a result, find_align_to_ref returns NULL.
> >
> > [1]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l582
> > [2]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l637
> > [3]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l684
> > [4]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l693
> > [5]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l715
> > [6]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l717
> > [7]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l742
> > [8]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l2346
> > [9]
> > http://hg.openjdk.java.net/jdk/jdk/file/1a7175456d29/src/hotspot/share
> > /opto/superword.cpp#l3626
> >
> > Proposed fix chooses one alignment from the remaining packs ignoring
> whether the memory operations are comparable or not.
> > This is currently under testing:
> > https://bugs.openjdk.java.net/secure/attachment/86058/fix-superword.pa
> > tch
> >
> > diff -r 6cf6761c444e src/hotspot/share/opto/superword.cpp
> > --- a/src/hotspot/share/opto/superword.cpp Wed Dec 11 12:12:39 2019
> > +0100
> > +++ b/src/hotspot/share/opto/superword.cpp Wed Dec 11 20:17:10
> 2019 +0800
> > @@ -576,12 +576,13 @@
> > }
> > Node_List align_to_refs;
> > + int max_idx;
> > int best_iv_adjustment = 0;
> > MemNode* best_align_to_mem_ref = NULL;
> > while (memops.size() != 0) {
> > // Find a memory reference to align to.
> > - MemNode* mem_ref = find_align_to_ref(memops);
> > + MemNode* mem_ref = find_align_to_ref(memops, max_idx);
> > if (mem_ref == NULL) break;
> > align_to_refs.push(mem_ref);
> > int iv_adjustment = get_iv_adjustment(mem_ref); @@ -699,34
> > +700,36 @@
> > // Put memory ops from remaining packs back on memops list for
> > // the best alignment search.
> > uint orig_msize = memops.size();
> > - if (_packset.length() == 1 && orig_msize == 0) {
> > - // If there are no remaining memory ops and only 1 pack we
> have only one choice
> > - // for the alignment
> > - Node_List* p = _packset.at(0);
> > - assert(p->size() > 0, "sanity");
> > + for (int i = 0; i < _packset.length(); i++) {
> > + Node_List* p = _packset.at(i);
> > MemNode* s = p->at(0)->as_Mem();
> > assert(!same_velt_type(s, mem_ref), "sanity");
> > - best_align_to_mem_ref = s;
> > - } else {
> > - for (int i = 0; i < _packset.length(); i++) {
> > - Node_List* p = _packset.at(i);
> > - MemNode* s = p->at(0)->as_Mem();
> > - assert(!same_velt_type(s, mem_ref), "sanity");
> > - memops.push(s);
> > + memops.push(s);
> > + }
> > + best_align_to_mem_ref = find_align_to_ref(memops, max_idx);
> > + if (best_align_to_mem_ref == NULL) {
> > + if (TraceSuperWord) {
> > + tty->print_cr("SuperWord::find_adjacent_refs():
> > + best_align_to_mem_ref == NULL");
> > }
> > - best_align_to_mem_ref = find_align_to_ref(memops);
> > - if (best_align_to_mem_ref == NULL) {
> > - if (TraceSuperWord) {
> > - tty->print_cr("SuperWord::find_adjacent_refs():
> best_align_to_mem_ref == NULL");
> > + if (_packset.length() > 0) {
> > + if (orig_msize == 0) {
> > + best_align_to_mem_ref =
> memops.at(max_idx)->as_Mem();
> > + } else {
> > + for (int i = 0; i < orig_msize; i++) {
> > + memops.remove(0);
> > + }
> > + best_align_to_mem_ref = find_align_to_ref(memops,
> max_idx);
> > + assert(best_align_to_mem_ref == NULL, "sanity");
> > + best_align_to_mem_ref =
> memops.at(max_idx)->as_Mem();
> > }
> > - break;
> > }
> > - best_iv_adjustment =
> get_iv_adjustment(best_align_to_mem_ref);
> > -
> NOT_PRODUCT(find_adjacent_refs_trace_1(best_align_to_mem_ref,
> best_iv_adjustment);)
> > - // Restore list.
> > - while (memops.size() > orig_msize)
> > - (void)memops.pop();
> > + break;
> > }
> > + best_iv_adjustment =
> get_iv_adjustment(best_align_to_mem_ref);
> > +
> NOT_PRODUCT(find_adjacent_refs_trace_1(best_align_to_mem_ref,
> best_iv_adjustment);)
> > + // Restore list.
> > + while (memops.size() > orig_msize)
> > + (void)memops.pop();
> > }
> > } // unaligned memory accesses
> > @@ -761,7 +764,7 @@
> > // Find a memory reference to align the loop induction variable to.
> > // Looks first at stores then at loads, looking for a memory reference
> > // with the largest number of references similar to it.
> > -MemNode* SuperWord::find_align_to_ref(Node_List &memops) {
> > +MemNode* SuperWord::find_align_to_ref(Node_List &memops, int &idx) {
> > GrowableArray<int> cmp_ct(arena(), memops.size(), memops.size(), 0);
> > // Count number of comparable memory ops @@ -848,6 +851,7 @@
> > }
> > #endif
> > + idx = max_idx;
> > if (max_ct > 0) {
> > #ifdef ASSERT
> > if (TraceSuperWord) {
> > diff -r 6cf6761c444e src/hotspot/share/opto/superword.hpp
> > --- a/src/hotspot/share/opto/superword.hpp Wed Dec 11 12:12:39
> 2019 +0100
> > +++ b/src/hotspot/share/opto/superword.hpp Wed Dec 11 20:17:10
> 2019 +0800
> > @@ -408,7 +408,7 @@
> > void print_loop(bool whole);
> > #endif
> > // Find a memory reference to align the loop induction variable to.
> > - MemNode* find_align_to_ref(Node_List &memops);
> > + MemNode* find_align_to_ref(Node_List &memops, int &idx);
> > // Calculate loop's iv adjustment for this memory ops.
> > int get_iv_adjustment(MemNode* mem);
> > // Can the preloop align the reference to position zero in the vector?
> >
> >
> > Thanks,
> > Felix
> >
More information about the hotspot-compiler-dev
mailing list