[9] RFR(S): 8179678: ArrayCopy with same src and dst can cause incorrect execution or compiler crash

[Vladimir Kozlov]

Call nodes have complicated graph of following control projections. You don't check for them. Why?
I think that was the reason that we scan memory edges instead of control when searching for calls.

Thanks,
Vladimir

On 5/19/17 5:14 AM, Roland Westrelin wrote:
>
> 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.
>