[9] RFR(S): 8179678: ArrayCopy with same src and dst can cause incorrect execution or compiler crash
Roland Westrelin
rwestrel at redhat.com
Fri May 19 12:14:22 UTC 2017
I found 2 more bugs with this. Here is a new webrev:
http://cr.openjdk.java.net/~roland/8179678/webrev.02/
In the case of test3(), C2 now correctly finds that it can't move the
src[0] load above the arraycopy. Once the ArrayCopyNode is expanded, C2
tries to move the src[0] load above what's now the subgraph of the
expanded arraycopy. ArrayCopyNode::modifies() is how C2 knows whether it
can step over an arraycopy. ArrayCopyNode::modifies() covers the
expanded arraycopy case by looking for arraycopy stub calls along memory
edges from the MemBar that's at the end of the arraycopy subgraph. The
problem here is that ArrayCopyNode::modifies() looks for the stub calls
on the raw memory slice but in this particular case, the stubs are on
the slice of the array that's input to arraycopy because of this code in
PhaseMacroExpand::expand_arraycopy_node():
// This is where the memory effects are placed:
const TypePtr* adr_type = TypeAryPtr::get_array_body_type(dest_elem);
if (ac->_dest_type != TypeOopPtr::BOTTOM) {
adr_type = ac->_dest_type->add_offset(Type::OffsetBot)->is_ptr();
}
if (ac->_src_type != ac->_dest_type) {
adr_type = TypeRawPtr::BOTTOM;
}
C2 then sets the load memory edge to the MemBar memory input that causes
an unschedulable graph. I fixed this by changing
ArrayCopyNode::modifies() so it looks for arraycopy stubs along control
edges.
The last bug, is with test4(). In that case, it's legal for the src[0]
load to move above the arraycopy and the arraycopy is eliminated. In the
process, the code in PhaseMacroExpand::process_users_of_allocation()
sets the dest input of the ArrayCopy node to top and because src ==
dest, src to top as well but the logic there doesn't expect src to be
top.
Roland.
More information about the hotspot-compiler-dev
mailing list