8325372: Shenandoah: SIGSEGV crash in unnecessary_acquire due to LoadStore split through phi

Reviewed-by: shade, rkennke, thartmann
This commit is contained in:
Roland Westrelin 2024-02-23 10:09:06 +00:00
parent 93a2e773a5
commit 5d414da504
4 changed files with 100 additions and 1 deletions
src/hotspot/share
test/hotspot/jtreg/gc/shenandoah/compiler

@ -1734,11 +1734,26 @@ bool ShenandoahBarrierC2Support::identical_backtoback_ifs(Node* n, PhaseIdealLoo
return true;
}
bool ShenandoahBarrierC2Support::merge_point_safe(Node* region) {
for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) {
Node* n = region->fast_out(i);
if (n->is_LoadStore()) {
// Splitting a LoadStore node through phi, causes it to lose its SCMemProj: the split if code doesn't have support
// for a LoadStore at the region the if is split through because that's not expected to happen (LoadStore nodes
// should be between barrier nodes). It does however happen with Shenandoah though because barriers can get
// expanded around a LoadStore node.
return false;
}
}
return true;
}
void ShenandoahBarrierC2Support::merge_back_to_back_tests(Node* n, PhaseIdealLoop* phase) {
assert(is_heap_stable_test(n), "no other tests");
if (identical_backtoback_ifs(n, phase)) {
Node* n_ctrl = n->in(0);
if (phase->can_split_if(n_ctrl)) {
if (phase->can_split_if(n_ctrl) && merge_point_safe(n_ctrl)) {
IfNode* dom_if = phase->idom(n_ctrl)->as_If();
if (is_heap_stable_test(n)) {
Node* gc_state_load = n->in(1)->in(1)->in(1)->in(1);

@ -65,6 +65,7 @@ private:
static void test_in_cset(Node*& ctrl, Node*& not_cset_ctrl, Node* val, Node* raw_mem, PhaseIdealLoop* phase);
static void move_gc_state_test_out_of_loop(IfNode* iff, PhaseIdealLoop* phase);
static void merge_back_to_back_tests(Node* n, PhaseIdealLoop* phase);
static bool merge_point_safe(Node* region);
static bool identical_backtoback_ifs(Node *n, PhaseIdealLoop* phase);
static void fix_ctrl(Node* barrier, Node* region, const MemoryGraphFixer& fixer, Unique_Node_List& uses, Unique_Node_List& uses_to_ignore, uint last, PhaseIdealLoop* phase);
static IfNode* find_unswitching_candidate(const IdealLoopTree *loop, PhaseIdealLoop* phase);

@ -3408,6 +3408,7 @@ Node *MemBarNode::Ideal(PhaseGVN *phase, bool can_reshape) {
my_mem = load_node;
} else {
assert(my_mem->unique_out() == this, "sanity");
assert(!trailing_load_store(), "load store node can't be eliminated");
del_req(Precedent);
phase->is_IterGVN()->_worklist.push(my_mem); // remove dead node later
my_mem = nullptr;

@ -0,0 +1,82 @@
/*
* Copyright (c) 2024, Red Hat, Inc. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/**
* @test
* @bug 8325372
* @summary fusion of heap stable test causes GetAndSet node to be removed
* @requires vm.gc.Shenandoah
* @modules java.base/jdk.internal.misc:+open
*
* @run main/othervm -XX:+UseShenandoahGC -XX:-BackgroundCompilation TestUnsafeLoadStoreMergedHeapStableTests
*/
import jdk.internal.misc.Unsafe;
import java.lang.reflect.Field;
public class TestUnsafeLoadStoreMergedHeapStableTests {
static final jdk.internal.misc.Unsafe UNSAFE = Unsafe.getUnsafe();
static long F_OFFSET;
static class A {
Object f;
}
static {
try {
Field fField = A.class.getDeclaredField("f");
F_OFFSET = UNSAFE.objectFieldOffset(fField);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
static Object testHelper(boolean flag, Object o, long offset, Object x) {
if (flag) {
return UNSAFE.getAndSetObject(o, offset, x);
}
return null;
}
static Object field;
static Object test1(boolean flag, Object o, long offset) {
return testHelper(flag, null, offset, field);
}
static Object test2(Object o, long offset) {
return UNSAFE.getAndSetObject(o, offset, field);
}
static public void main(String[] args) {
A a = new A();
for (int i = 0; i < 20_000; i++) {
testHelper(true, a, F_OFFSET, null);
test1(false, a, F_OFFSET);
test2(a, F_OFFSET);
}
}
}