Skip to content

Commit

Permalink
8322996: BoxLockNode creation fails with assert(reg < CHUNK_SIZE) fai…
Browse files Browse the repository at this point in the history
…led: sanity

Reviewed-by: rcastanedalo, kvn
  • Loading branch information
Daniel Lundén authored and robcasloz committed Jan 29, 2024
1 parent f0bae79 commit 69586e7
Show file tree
Hide file tree
Showing 6 changed files with 265 additions and 14 deletions.
7 changes: 5 additions & 2 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, 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
Expand Down Expand Up @@ -3468,7 +3468,10 @@ FastLockNode* GraphKit::shared_lock(Node* obj) {
assert(dead_locals_are_killed(), "should kill locals before sync. point");

// Box the stack location
Node* box = _gvn.transform(new BoxLockNode(next_monitor()));
Node* box = new BoxLockNode(next_monitor());
// Check for bailout after new BoxLockNode
if (failing()) { return nullptr; }
box = _gvn.transform(box);
Node* mem = reset_memory();

FastLockNode * flock = _gvn.transform(new FastLockNode(0, obj, box) )->as_FastLock();
Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/share/opto/locknode.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2024, 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
Expand Down Expand Up @@ -44,6 +44,10 @@ BoxLockNode::BoxLockNode( int slot ) : Node( Compile::current()->root() ),
init_class_id(Class_BoxLock);
init_flags(Flag_rematerialize);
OptoReg::Name reg = OptoReg::stack2reg(_slot);
if (!RegMask::can_represent(reg, Compile::current()->sync_stack_slots())) {
Compile::current()->record_method_not_compilable("must be able to represent all monitor slots in reg mask");
return;
}
_inmask.Insert(reg);
}

Expand Down
9 changes: 7 additions & 2 deletions src/hotspot/share/opto/parse1.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, 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
Expand Down Expand Up @@ -224,7 +224,10 @@ void Parse::load_interpreter_state(Node* osr_buf) {
Node *monitors_addr = basic_plus_adr(osr_buf, osr_buf, (max_locals+mcnt*2-1)*wordSize);
for (index = 0; index < mcnt; index++) {
// Make a BoxLockNode for the monitor.
Node *box = _gvn.transform(new BoxLockNode(next_monitor()));
Node *box = new BoxLockNode(next_monitor());
// Check for bailout after new BoxLockNode
if (failing()) { return; }
box = _gvn.transform(box);


// Displaced headers and locked objects are interleaved in the
Expand Down Expand Up @@ -1260,6 +1263,8 @@ void Parse::do_method_entry() {
kill_dead_locals();
// Build the FastLockNode
_synch_lock = shared_lock(lock_obj);
// Check for bailout in shared_lock
if (failing()) { return; }
}

// Feed profiling data for parameters to the type system so it can
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/opto/parse2.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2024, 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
Expand Down Expand Up @@ -2778,6 +2778,7 @@ void Parse::do_one_bytecode() {
}

#ifndef PRODUCT
if (failing()) { return; }
constexpr int perBytecode = 6;
if (C->should_print_igv(perBytecode)) {
IdealGraphPrinter* printer = C->igv_printer();
Expand Down
16 changes: 8 additions & 8 deletions src/hotspot/share/opto/regmask.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, 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
Expand Down Expand Up @@ -29,6 +29,7 @@
#include "opto/optoreg.hpp"
#include "utilities/count_leading_zeros.hpp"
#include "utilities/count_trailing_zeros.hpp"
#include "utilities/globalDefinitions.hpp"

class LRG;

Expand Down Expand Up @@ -359,16 +360,15 @@ class RegMask {
static const RegMask Empty; // Common empty mask
static const RegMask All; // Common all mask

static bool can_represent(OptoReg::Name reg) {
// NOTE: -1 in computation reflects the usage of the last
// bit of the regmask as an infinite stack flag and
// -7 is to keep mask aligned for largest value (VecZ).
return (int)reg < (int)(CHUNK_SIZE - 1);
static bool can_represent(OptoReg::Name reg, unsigned int size = 1) {
// NOTE: MAX2(1U,size) in computation reflects the usage of the last
// bit of the regmask as an infinite stack flag.
return (int)reg < (int)(CHUNK_SIZE - MAX2(1U,size));
}
static bool can_represent_arg(OptoReg::Name reg) {
// NOTE: -SlotsPerVecZ in computation reflects the need
// NOTE: SlotsPerVecZ in computation reflects the need
// to keep mask aligned for largest value (VecZ).
return (int)reg < (int)(CHUNK_SIZE - SlotsPerVecZ);
return can_represent(reg, SlotsPerVecZ);
}
};

Expand Down
238 changes: 238 additions & 0 deletions test/hotspot/jtreg/compiler/locks/TestNestedSynchronize.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,238 @@
/*
* Copyright (c) 2024, 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 8322996
* @summary Ensure no assert error in C2 with deeply nested synchronize
* statements.
* @run main/othervm -XX:CompileCommand=compileonly,compiler.locks.TestNestedSynchronize::test
* -Xcomp
* compiler.locks.TestNestedSynchronize
*/

package compiler.locks;

public class TestNestedSynchronize {

public static void main(String[] args) {
test();
}

public static void test() {

synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {
synchronized (TestNestedSynchronize.class) {

}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}

1 comment on commit 69586e7

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.