Skip to content
This repository was archived by the owner on Sep 2, 2022. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/callnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ Node *AllocateArrayNode::make_ideal_length(const TypeOopPtr* oop_type, PhaseTran
InitializeNode* init = initialization();
assert(init != NULL, "initialization not found");
length = new CastIINode(length, narrow_length_type);
length->set_req(0, init->proj_out_or_null(0));
length->set_req(TypeFunc::Control, init->proj_out_or_null(TypeFunc::Control));
}
}

Expand Down
37 changes: 22 additions & 15 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1184,15 +1184,31 @@ Node* GraphKit::load_array_length(Node* array) {
Node *r_adr = basic_plus_adr(array, arrayOopDesc::length_offset_in_bytes());
alen = _gvn.transform( new LoadRangeNode(0, immutable_memory(), r_adr, TypeInt::POS));
} else {
alen = alloc->Ideal_length();
Node* ccast = alloc->make_ideal_length(_gvn.type(array)->is_oopptr(), &_gvn);
if (ccast != alen) {
alen = _gvn.transform(ccast);
}
alen = array_ideal_length(alloc, _gvn.type(array)->is_oopptr(), false);
}
return alen;
}

Node* GraphKit::array_ideal_length(AllocateArrayNode* alloc,
const TypeOopPtr* oop_type,
bool replace_length_in_map) {
Node* length = alloc->Ideal_length();
if (replace_length_in_map == false || map()->find_edge(length) >= 0) {
Node* ccast = alloc->make_ideal_length(oop_type, &_gvn);
if (ccast != length) {
// do not transfrom ccast here, it might convert to top node for
// negative array length and break assumptions in parsing stage.
_gvn.set_type_bottom(ccast);
record_for_igvn(ccast);
if (replace_length_in_map) {
replace_in_map(length, ccast);
}
return ccast;
}
}
return length;
}

//------------------------------do_null_check----------------------------------
// Helper function to do a NULL pointer check. Returned value is
// the incoming address with NULL casted away. You are allowed to use the
Expand Down Expand Up @@ -3967,16 +3983,7 @@ Node* GraphKit::new_array(Node* klass_node, // array klass (maybe variable)

Node* javaoop = set_output_for_allocation(alloc, ary_type, deoptimize_on_exception);

// Cast length on remaining path to be as narrow as possible
if (map()->find_edge(length) >= 0) {
Node* ccast = alloc->make_ideal_length(ary_type, &_gvn);
if (ccast != length) {
_gvn.set_type_bottom(ccast);
record_for_igvn(ccast);
replace_in_map(length, ccast);
}
}

array_ideal_length(alloc, ary_type, true);
return javaoop;
}

Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/opto/graphKit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@ class GraphKit : public Phase {
Node* load_object_klass(Node* object);
// Find out the length of an array.
Node* load_array_length(Node* array);
// Cast array allocation's length as narrow as possible.
// If replace_length_in_map is true, replace length with CastIINode in map.
// This method is invoked after creating/moving ArrayAllocationNode or in load_array_length
Node* array_ideal_length(AllocateArrayNode* alloc,
const TypeOopPtr* oop_type,
bool replace_length_in_map);


// Helper function to do a NULL pointer check or ZERO check based on type.
Expand Down
32 changes: 32 additions & 0 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4443,6 +4443,36 @@ void LibraryCallKit::arraycopy_move_allocation_here(AllocateArrayNode* alloc, No
Node* alloc_mem = alloc->in(TypeFunc::Memory);
C->gvn_replace_by(callprojs.fallthrough_ioproj, alloc->in(TypeFunc::I_O));
C->gvn_replace_by(init->proj_out(TypeFunc::Memory), alloc_mem);

// The CastIINode created in GraphKit::new_array (in AllocateArrayNode::make_ideal_length) must stay below
// the allocation (i.e. is only valid if the allocation succeeds):
// 1) replace CastIINode with AllocateArrayNode's length here
// 2) Create CastIINode again once allocation has moved (see below) at the end of this method
//
// Multiple identical CastIINodes might exist here. Each GraphKit::load_array_length() call will generate
// new separate CastIINode (arraycopy guard checks or any array length use between array allocation and ararycopy)
Node* init_control = init->proj_out(TypeFunc::Control);
Node* alloc_length = alloc->Ideal_length();
#ifdef ASSERT
Node* prev_cast = NULL;
#endif
for (uint i = 0; i < init_control->outcnt(); i++) {
Node* init_out = init_control->raw_out(i);
if (init_out->is_CastII() && init_out->in(TypeFunc::Control) == init_control && init_out->in(1) == alloc_length) {
#ifdef ASSERT
if (prev_cast == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment why multiply CastII could be here (because each load_array_length() call in guard checks will generate now separate CastII nodes). All these CastII nodes are replaced here with one: original alloc_length.

I am wondering if will affect result of guard checks in runtime.

Copy link
Author

Choose a reason for hiding this comment

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

done.

Would you share more detail about your concern for "if will affect result of guard checks in runtime."?

prev_cast = init_out;
} else {
if (prev_cast->cmp(*init_out) == false) {
prev_cast->dump();
init_out->dump();
assert(false, "not equal CastIINode");
}
}
#endif
C->gvn_replace_by(init_out, alloc_length);
}
}
C->gvn_replace_by(init->proj_out(TypeFunc::Control), alloc->in(0));

// move the allocation here (after the guards)
Expand Down Expand Up @@ -4474,6 +4504,8 @@ void LibraryCallKit::arraycopy_move_allocation_here(AllocateArrayNode* alloc, No
dest->set_req(0, control());
Node* destx = _gvn.transform(dest);
assert(destx == dest, "where has the allocation result gone?");

array_ideal_length(alloc, ary_type, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright (C) 2021 THL A29 Limited, a Tencent company. 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 8268362
* @requires vm.compiler2.enabled & vm.debug
* @summary C2 using negative array length as index, using a.length.
* AllocateArrayNode::make_ideal_length create CastIINode to not negative range.
* Apply transform in GraphKit::load_array_length will covert array load index type to top.
* This cause assert in Parse::array_addressing, it expect index type is int.
* @run main/othervm -XX:-PrintCompilation compiler.arraycopy.TestNegArrayLengthAsIndex1
*/

package compiler.arraycopy;
public class TestNegArrayLengthAsIndex1 {

public static void main(String[] args) throws Exception {
for (int i = 0; i < 10000; i++) {
foo();
}
}

static int foo() {
int minusOne = -1;
int[] a = null;
try {
a = new int[minusOne];
} catch (NegativeArraySizeException e) {
return 0;
}
return a[a.length - 1];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright (C) 2021 THL A29 Limited, a Tencent company. 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 8268362
* @requires vm.compiler2.enabled & vm.debug
* @summary C2 using negative array length as index, using array allocation length.
* This assertion is triggered by 8267904.
* @run main/othervm compiler.arraycopy.TestNegArrayLengthAsIndex2
*/

package compiler.arraycopy;
public class TestNegArrayLengthAsIndex2 {

public static void main(String[] args) throws Exception {
for (int i = 0; i < 10000; i++) {
foo();
}
}

static int foo() {
int minusOne = -1;
int[] a = null;
try {
a = new int[minusOne];
} catch (NegativeArraySizeException e) {
return 0;
}
return a[minusOne - 1];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright (C) 2021 THL A29 Limited, a Tencent company. 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 8267904
* @requires vm.compiler2.enabled
* @summary C2 inline array_copy move CastIINode(Array Length) before allocation cause crash.
* @run main/othervm compiler.arraycopy.TestNegativeArrayCopyAfterLoop
*/

package compiler.arraycopy;
import java.util.Arrays;

class test {
public static int exp_count = 0;
public int in1 = -4096;
test (){
try {
short sha4[] = new short[1012];
for (int i = 0; i < sha4.length; i++) {
sha4[i] = 9;
}
Arrays.copyOf(sha4, in1);
} catch (Exception ex) {
exp_count++;
}
}
}

public class TestNegativeArrayCopyAfterLoop {
public static void main(String[] args) {
for (int i = 0; i < 20000; i++) {
new test();
}
if (test.exp_count == 20000) {
System.out.println("TEST PASSED");
}
}
}