RFR(S): 8235762: JVM crash in SWPointer during C2 compilation
Tobias Hartmann
tobias.hartmann at oracle.com
Wed Dec 11 12:50:00 UTC 2019
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.patch
>
> 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