Skip to content

Commit

Permalink
8268362: [REDO] C2 crash when compile negative Arrays.copyOf length a…
Browse files Browse the repository at this point in the history
…fter loop

Backport-of: 22ebd19
  • Loading branch information
TheRealMDoerr committed Jul 15, 2021
1 parent 8008d33 commit 2252165
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/callnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,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 @@ -1180,15 +1180,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 @@ -3734,16 +3750,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 @@ -336,6 +336,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 @@ -4592,6 +4592,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) {
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 @@ -4623,6 +4653,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");
}
}
}

1 comment on commit 2252165

@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.