Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions src/hotspot/share/opto/macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2320,12 +2320,7 @@ void PhaseMacroExpand::expand_unlock_node(UnlockNode *unlock) {
// No need for a null check on unlock

// Make the merge point
Node *region;
Node *mem_phi;

region = new RegionNode(3);
// create a Phi for the memory state
mem_phi = new PhiNode( region, Type::MEMORY, TypeRawPtr::BOTTOM);
Node *region = new RegionNode(3);

FastUnlockNode *funlock = new FastUnlockNode( ctrl, obj, box );
funlock = transform_later( funlock )->as_FastUnlock();
Expand Down Expand Up @@ -2354,12 +2349,15 @@ void PhaseMacroExpand::expand_unlock_node(UnlockNode *unlock) {
transform_later(region);
_igvn.replace_node(_callprojs.fallthrough_proj, region);

Node *memproj = transform_later(new ProjNode(call, TypeFunc::Memory) );
mem_phi->init_req(1, memproj );
mem_phi->init_req(2, mem);
transform_later(mem_phi);

_igvn.replace_node(_callprojs.fallthrough_memproj, mem_phi);
if (_callprojs.fallthrough_memproj != nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we not have to hook up the memory input to the fall through projection if it does not exist in the first place?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify the question?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code before assumed that fallthrough_memproj is always not null. So does the rest of the code also expect this? If so, then we should perhaps use mem for that purpose. This is related to @dean-long's question below. There is an invariant that is being violated and your fix should take into account why it is violated.

// create a Phi for the memory state
Node *mem_phi = new PhiNode( region, Type::MEMORY, TypeRawPtr::BOTTOM);
Node *memproj = transform_later(new ProjNode(call, TypeFunc::Memory));
mem_phi->init_req(1, memproj);
mem_phi->init_req(2, mem);
transform_later(mem_phi);
_igvn.replace_node(_callprojs.fallthrough_memproj, mem_phi);
}
}

void PhaseMacroExpand::expand_subtypecheck_node(SubTypeCheckNode *check) {
Expand Down
53 changes: 53 additions & 0 deletions test/hotspot/jtreg/compiler/c2/Test8370502.java
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation in Java files should be 4 spaces.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed another test's format style. Now I can see that there is a mixture of 2 and 4 space Test*.java files.

Fixing.

Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. 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 8370502
* @summary Do not segfault while adding node to IGVN worklist
*
* @run main/othervm -Xbatch compiler.c2.Test8370502
*/

package compiler.c2;

public class Test8370502 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by comment: Please rename the test to something more descriptive. We don't use bug numbers for test names anymore.

public static void main(String[] args) {
int[] a = new int[0]; // test valid only when size is 0.
for (int i = 0; i < Integer.valueOf(10000); i++)
try {
test(a, 0);
} catch (ArrayIndexOutOfBoundsException e) {
}
}

static void test(int[] a, int invar) {
for (int i = 0; i < 1;) {
a[i] = 0;
synchronized (Test8370502.class) {
}
for (int j = 0; Integer.valueOf(j) < 1;)
j = 0;
}
}
}