RFR(S): 8235762: JVM crash in SWPointer during C2 compilation
Yangfei (Felix)
felix.yang at huawei.com
Wed Dec 11 12:39:46 UTC 2019
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