Skip to content
Permalink
Browse files
8263361: Incorrect arraycopy stub selected by C2 for SATB collectors
Reviewed-by: yan
Backport-of: 5d87a21
  • Loading branch information
Sergey Nazarkin authored and Yuri Nesterenko committed Jul 28, 2021
1 parent dd3d55e commit 4d40fee765c11fa8fabd70d3e1adc85a5fd250cd
Showing 3 changed files with 42 additions and 32 deletions.
@@ -4396,8 +4396,8 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) {
set_control(is_obja);
obj = access_resolve(obj, ACCESS_READ);
// 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, false);
// Clones are always tightly coupled.
ArrayCopyNode* ac = ArrayCopyNode::make(this, true, obj, intcon(0), alloc_obj, intcon(0), obj_length, true, false);
ac->set_cloneoop();
Node* n = _gvn.transform(ac);
assert(n == ac, "cannot disappear");
@@ -286,8 +286,9 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
transform_later(slow_region);
}

Node* original_dest = dest;
bool dest_uninitialized = false;
Node* original_dest = dest;
bool dest_needs_zeroing = false;
bool acopy_to_uninitialized = false;

// See if this is the initialization of a newly-allocated array.
// If so, we will take responsibility here for initializing it to zero.
@@ -299,25 +300,34 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
&& basic_elem_type != T_CONFLICT // avoid corner case
&& !src->eqv_uncast(dest)
&& alloc != NULL
&& _igvn.find_int_con(alloc->in(AllocateNode::ALength), 1) > 0
&& alloc->maybe_set_complete(&_igvn)) {
// "You break it, you buy it."
InitializeNode* init = alloc->initialization();
assert(init->is_complete(), "we just did this");
init->set_complete_with_arraycopy();
assert(dest->is_CheckCastPP(), "sanity");
assert(dest->in(0)->in(0) == init, "dest pinned");
adr_type = TypeRawPtr::BOTTOM; // all initializations are into raw memory
// From this point on, every exit path is responsible for
// initializing any non-copied parts of the object to zero.
// Also, if this flag is set we make sure that arraycopy interacts properly
// with G1, eliding pre-barriers. See CR 6627983.
dest_uninitialized = true;
&& _igvn.find_int_con(alloc->in(AllocateNode::ALength), 1) > 0) {
assert(ac->is_alloc_tightly_coupled(), "sanity");
// acopy to uninitialized tightly coupled allocations
// needs zeroing outside the copy range
// and the acopy itself will be to uninitialized memory
acopy_to_uninitialized = true;
if (alloc->maybe_set_complete(&_igvn)) {
// "You break it, you buy it."
InitializeNode* init = alloc->initialization();
assert(init->is_complete(), "we just did this");
init->set_complete_with_arraycopy();
assert(dest->is_CheckCastPP(), "sanity");
assert(dest->in(0)->in(0) == init, "dest pinned");
adr_type = TypeRawPtr::BOTTOM; // all initializations are into raw memory
// From this point on, every exit path is responsible for
// initializing any non-copied parts of the object to zero.
// Also, if this flag is set we make sure that arraycopy interacts properly
// with G1, eliding pre-barriers. See CR 6627983.
dest_needs_zeroing = true;
} else {
// dest_need_zeroing = false;
}
} else {
// No zeroing elimination here.
alloc = NULL;
//original_dest = dest;
//dest_uninitialized = false;
// No zeroing elimination needed here.
alloc = NULL;
acopy_to_uninitialized = false;
//original_dest = dest;
//dest_needs_zeroing = false;
}

uint alias_idx = C->get_alias_index(adr_type);
@@ -351,11 +361,11 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
Node* checked_value = NULL;

if (basic_elem_type == T_CONFLICT) {
assert(!dest_uninitialized, "");
assert(!dest_needs_zeroing, "");
Node* cv = generate_generic_arraycopy(ctrl, &mem,
adr_type,
src, src_offset, dest, dest_offset,
copy_length, dest_uninitialized);
copy_length, acopy_to_uninitialized);
if (cv == NULL) cv = intcon(-1); // failure (no stub available)
checked_control = *ctrl;
checked_i_o = *io;
@@ -376,7 +386,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
}

// copy_length is 0.
if (dest_uninitialized) {
if (dest_needs_zeroing) {
assert(!local_ctrl->is_top(), "no ctrl?");
Node* dest_length = alloc->in(AllocateNode::ALength);
if (copy_length->eqv_uncast(dest_length)
@@ -411,7 +421,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
result_memory->init_req(zero_path, local_mem->memory_at(alias_idx));
}

if (!(*ctrl)->is_top() && dest_uninitialized) {
if (!(*ctrl)->is_top() && dest_needs_zeroing) {
// We have to initialize the *uncopied* part of the array to zero.
// The copy destination is the slice dest[off..off+len]. The other slices
// are dest_head = dest[0..off] and dest_tail = dest[off+len..dest.length].
@@ -452,7 +462,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
didit = generate_block_arraycopy(&local_ctrl, &local_mem, local_io,
adr_type, basic_elem_type, alloc,
src, src_offset, dest, dest_offset,
dest_size, dest_uninitialized);
dest_size, acopy_to_uninitialized);
if (didit) {
// Present the results of the block-copying fast call.
result_region->init_req(bcopy_path, local_ctrl);
@@ -540,7 +550,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
adr_type,
dest_elem_klass,
src, src_offset, dest, dest_offset,
ConvI2X(copy_length), dest_uninitialized);
ConvI2X(copy_length), acopy_to_uninitialized);
if (cv == NULL) cv = intcon(-1); // failure (no stub available)
checked_control = local_ctrl;
checked_i_o = *io;
@@ -568,7 +578,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
generate_unchecked_arraycopy(&local_ctrl, &local_mem,
adr_type, copy_type, disjoint_bases,
src, src_offset, dest, dest_offset,
ConvI2X(copy_length), dest_uninitialized);
ConvI2X(copy_length), acopy_to_uninitialized);

// Present the results of the fast call.
result_region->init_req(fast_path, local_ctrl);
@@ -657,7 +667,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
// Generate the slow path, if needed.
local_mem->set_memory_at(alias_idx, slow_mem);

if (dest_uninitialized) {
if (dest_needs_zeroing) {
generate_clear_array(local_ctrl, local_mem,
adr_type, dest, basic_elem_type,
intcon(0), NULL,
@@ -516,8 +516,8 @@ StubRoutines::select_arraycopy_function(BasicType t, bool aligned, bool disjoint
name = #xxx_arraycopy; \
return StubRoutines::xxx_arraycopy(); }

#define RETURN_STUB_PARM(xxx_arraycopy, parm) { \
name = #xxx_arraycopy; \
#define RETURN_STUB_PARM(xxx_arraycopy, parm) { \
name = parm ? #xxx_arraycopy "_uninit": #xxx_arraycopy; \
return StubRoutines::xxx_arraycopy(parm); }

switch (t) {

1 comment on commit 4d40fee

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 4d40fee Jul 28, 2021

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.