infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-02-04 23:48:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

8159611: C2: ArrayCopy elimination skips required parameter checks

Browse Source 
Reviewed-by: kvn, zmajo, thartmann
This commit is contained in:
Volker Simonis 2016-10-06 18:51:24 +02:00
parent 7bc6ecfba8
commit fcdc3eac28
6 changed files with 146 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
View File

@ -3187,7 +3187,6 @@ void LIR_Assembler::emit_arraycopy(LIR_OpArrayCopy* op) {
if (flags & LIR_OpArrayCopy::length_positive_check) {
__ testl(length, length);
__ jcc(Assembler::less, *stub->entry());
__ jcc(Assembler::zero, *stub->continuation());
}
#ifdef _LP64

6
hotspot/src/share/vm/opto/arraycopynode.cpp
View File

@ -26,9 +26,10 @@
#include "opto/arraycopynode.hpp"
#include "opto/graphKit.hpp"
ArrayCopyNode::ArrayCopyNode(Compile* C, bool alloc_tightly_coupled)
ArrayCopyNode::ArrayCopyNode(Compile* C, bool alloc_tightly_coupled, bool has_negative_length_guard)
: CallNode(arraycopy_type(), NULL, TypeRawPtr::BOTTOM),
_alloc_tightly_coupled(alloc_tightly_coupled),
_has_negative_length_guard(has_negative_length_guard),
_kind(None),
_arguments_validated(false),
_src_type(TypeOopPtr::BOTTOM),
@ -45,10 +46,11 @@ ArrayCopyNode* ArrayCopyNode::make(GraphKit* kit, bool may_throw,
Node* dest, Node* dest_offset,
Node* length,
bool alloc_tightly_coupled,
bool has_negative_length_guard,
Node* src_klass, Node* dest_klass,
Node* src_length, Node* dest_length) {
ArrayCopyNode* ac = new ArrayCopyNode(kit->C, alloc_tightly_coupled);
ArrayCopyNode* ac = new ArrayCopyNode(kit->C, alloc_tightly_coupled, has_negative_length_guard);
Node* prev_mem = kit->set_predefined_input_for_runtime_call(ac);
ac->init_req(ArrayCopyNode::Src, src);

6
hotspot/src/share/vm/opto/arraycopynode.hpp
View File

@ -58,6 +58,7 @@ private:
// the arraycopy is not parsed yet so doesn't exist when
// LibraryCallKit::tightly_coupled_allocation() is called.
bool _alloc_tightly_coupled;
bool _has_negative_length_guard;
bool _arguments_validated;
@ -82,7 +83,7 @@ private:
return TypeFunc::make(domain, range);
}
ArrayCopyNode(Compile* C, bool alloc_tightly_coupled);
ArrayCopyNode(Compile* C, bool alloc_tightly_coupled, bool has_negative_length_guard);
intptr_t get_length_if_constant(PhaseGVN *phase) const;
int get_count(PhaseGVN *phase) const;
@ -133,6 +134,7 @@ public:
Node* dest, Node* dest_offset,
Node* length,
bool alloc_tightly_coupled,
bool has_negative_length_guard,
Node* src_klass = NULL, Node* dest_klass = NULL,
Node* src_length = NULL, Node* dest_length = NULL);
@ -162,6 +164,8 @@ public:
bool is_alloc_tightly_coupled() const { return _alloc_tightly_coupled; }
bool has_negative_length_guard() const { return _has_negative_length_guard; }
static bool may_modify(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase, ArrayCopyNode*& ac);
bool modifies(intptr_t offset_lo, intptr_t offset_hi, PhaseTransform* phase, bool must_modify);

19
hotspot/src/share/vm/opto/library_call.cpp
View File

@ -4046,7 +4046,7 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
if (!stopped()) {
newcopy = new_array(klass_node, length, 0); // no arguments to push
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, original, start, newcopy, intcon(0), moved, true,
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, original, start, newcopy, intcon(0), moved, true, false,
load_object_klass(original), klass_node);
if (!is_copyOfRange) {
ac->set_copyof(validated);
@ -4566,7 +4566,7 @@ void LibraryCallKit::copy_to_clone(Node* obj, Node* alloc_obj, Node* obj_size, b
const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
ArrayCopyNode* ac = ArrayCopyNode::make(this, false, src, NULL, dest, NULL, countx, false);
ArrayCopyNode* ac = ArrayCopyNode::make(this, false, src, NULL, dest, NULL, countx, false, false);
ac->set_clonebasic();
Node* n = _gvn.transform(ac);
if (n == ac) {
@ -4703,7 +4703,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) {
set_control(is_obja);
// Generate a direct call to the right arraycopy function(s).
Node* alloc = tightly_coupled_allocation(alloc_obj, NULL);
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, obj, intcon(0), alloc_obj, intcon(0), obj_length, alloc != NULL);
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, obj, intcon(0), alloc_obj, intcon(0), obj_length, alloc != NULL, false);
ac->set_cloneoop();
Node* n = _gvn.transform(ac);
assert(n == ac, "cannot disappear");
@ -5100,6 +5100,8 @@ bool LibraryCallKit::inline_arraycopy() {
trap_bci = alloc->jvms()->bci();
}
bool negative_length_guard_generated = false;
if (!C->too_many_traps(trap_method, trap_bci, Deoptimization::Reason_intrinsic) &&
can_emit_guards &&
!src->is_top() && !dest->is_top()) {
@ -5132,6 +5134,15 @@ bool LibraryCallKit::inline_arraycopy() {
load_array_length(dest),
slow_region);
// (6) length must not be negative.
// This is also checked in generate_arraycopy() during macro expansion, but
// we also have to check it here for the case where the ArrayCopyNode will
// be eliminated by Escape Analysis.
if (EliminateAllocations) {
generate_negative_guard(length, slow_region);
negative_length_guard_generated = true;
}
// (9) each element of an oop array must be assignable
Node* src_klass = load_object_klass(src);
Node* dest_klass = load_object_klass(dest);
@ -5159,7 +5170,7 @@ bool LibraryCallKit::inline_arraycopy() {
return true;
}
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, src, src_offset, dest, dest_offset, length, alloc != NULL,
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, src, src_offset, dest, dest_offset, length, alloc != NULL, negative_length_guard_generated,
// Create LoadRange and LoadKlass nodes for use during macro expansion here
// so the compiler has a chance to eliminate them: during macro expansion,
// we have to set their control (CastPP nodes are eliminated).

9
hotspot/src/share/vm/opto/macroArrayCopy.cpp
View File

@ -1154,7 +1154,10 @@ void PhaseMacroExpand::expand_arraycopy_node(ArrayCopyNode *ac) {
// Call StubRoutines::generic_arraycopy stub.
Node* mem = generate_arraycopy(ac, NULL, &ctrl, merge_mem, &io,
TypeRawPtr::BOTTOM, T_CONFLICT,
src, src_offset, dest, dest_offset, length);
src, src_offset, dest, dest_offset, length,
// If a negative length guard was generated for the ArrayCopyNode,
// the length of the array can never be negative.
false, ac->has_negative_length_guard());
// Do not let reads from the destination float above the arraycopy.
// Since we cannot type the arrays, we don't know which slices
@ -1258,5 +1261,7 @@ void PhaseMacroExpand::expand_arraycopy_node(ArrayCopyNode *ac) {
generate_arraycopy(ac, alloc, &ctrl, merge_mem, &io,
adr_type, dest_elem,
src, src_offset, dest, dest_offset, length,
false, false, slow_region);
// If a negative length guard was generated for the ArrayCopyNode,
// the length of the array can never be negative.
false, ac->has_negative_length_guard(), slow_region);
}

115
hotspot/test/compiler/escapeAnalysis/TestArrayCopy.java Normal file
View File

@ -0,0 +1,115 @@
/*
* Copyright (c) 2016, SAP SE 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 8159611
* @summary The elimination of System.arraycopy by EscapeAnalysis prevents
* an IndexOutOfBoundsException from being thrown if the arraycopy
* is called with a negative length argument.
* @modules java.base/jdk.internal.misc
* @library /testlibrary /test/lib
* @build sun.hotspot.WhiteBox
* @run driver ClassFileInstaller sun.hotspot.WhiteBox
* sun.hotspot.WhiteBox$WhiteBoxPermission
*
* @run main/othervm
* -Xbootclasspath/a:.
* -XX:+UnlockDiagnosticVMOptions
* -XX:+WhiteBoxAPI
* -XX:-UseOnStackReplacement
* compiler.escapeAnalysis.TestArrayCopy
*
* @author Volker Simonis
*/
package compiler.escapeAnalysis;
import sun.hotspot.WhiteBox;
import java.lang.reflect.Method;
public class TestArrayCopy {
private static final WhiteBox WB = WhiteBox.getWhiteBox();
// DST_LEN Must be const, otherwise EliminateAllocations won't work
static final int DST_LEN = 4;
static final int SRC_LEN = 8;
public static boolean do_test1(Object src, int src_pos, int dst_pos, int cpy_len) {
try {
System.arraycopy(src, src_pos, new Object[DST_LEN], dst_pos, cpy_len);
return false;
} catch (IndexOutOfBoundsException e) {
return true;
}
}
public static int do_test2(Object src, int src_pos, int dst_pos, int cpy_len) {
try {
System.arraycopy(src, src_pos, new Object[DST_LEN], dst_pos, cpy_len);
return 0;
} catch (IndexOutOfBoundsException e) {
return 1;
} catch (ArrayStoreException e) {
return 2;
}
}
static final int COUNT = 100_000;
static final int[] src_pos = { 0, -1, -1, 0, 0, 0, 1, 1, 1, 1, 1 };
static final int[] dst_pos = { 0, -1, 0, -1, 0, 1, 0, 1, 1, 1, 1 };
static final int[] cpy_len = { 0, 0, 0, 0, -1, -1, -1, -1, 8, 4, 2 };
public static void main(String args[]) throws Exception {
int length = args.length > 0 ? Integer.parseInt(args[0]) : -1;
int[] int_arr = new int[SRC_LEN];
Object[] obj_arr = new Object[SRC_LEN];
Method test1 = TestArrayCopy.class.getMethod("do_test1", Object.class, int.class, int.class, int.class);
Method test2 = TestArrayCopy.class.getMethod("do_test2", Object.class, int.class, int.class, int.class);
for (int i = 0; i < src_pos.length; i++) {
int sp = src_pos[i];
int dp = dst_pos[i];
int cl = cpy_len[i];
String version1 = String.format("System.arraycopy(Object[8], %d, new Object[%d], %d, %d)", sp, DST_LEN, dp, cl);
String version2 = String.format("System.arraycopy(int[8], %d, new Object[%d], %d, %d)", sp, DST_LEN, dp, cl);
System.out.format("Testing " + version1 + "\nand " + version2).flush();
for (int x = 0; x < COUNT; x++) {
if (!do_test1(obj_arr, sp, dp, cl) &&
(sp < 0 || dp < 0 || cl < 0 || (sp + cl >= SRC_LEN) || (dp + cl >= DST_LEN))) {
throw new RuntimeException("Expected IndexOutOfBoundsException for " + version1);
}
int res = do_test2(int_arr, sp, dp, cl);
if (res == 0 || res == 1) {
throw new RuntimeException("Expected ArrayStoreException for " + version2);
}
}
WB.deoptimizeMethod(test1);
WB.clearMethodState(test1);
WB.deoptimizeMethod(test2);
WB.clearMethodState(test2);
}
}
}