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

[Vladimir Kozlov]

Okay, this seems fine.

I will run testing on this change and sponsor if it pass.

Thanks,
Vladimir

On 5/22/17 9:26 AM, Roland Westrelin wrote:
>
>> 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.
>
> Catch and CatchProj?
>
> The logic I changed handles an array copy, once it is expanded, whose
> destination is a non escaping array.
>
> Escape analysis only considers the destination of "validated" array
> copies as non escaping:
>
> PointsToNode::EscapeState es = PointsToNode::ArgEscape;
> if (call->is_ArrayCopy()) {
>   ArrayCopyNode* ac = call->as_ArrayCopy();
>   if (ac->is_clonebasic() ||
>       ac->is_arraycopy_validated() ||
>       ac->is_copyof_validated() ||
>       ac->is_copyofrange_validated()) {
>     es = PointsToNode::NoEscape;
>   }
> }
>
> An array copy is "validated" when LibraryCallKit::inline_arraycopy()
> inserted guards to validate that this arraycopy is a "good" arraycopy.
>
> In PhaseMacroExpand::generate_arraycopy() the stub calls that are
> inserted are all connected through a single ProjNode to the result
> Region except:
>
> 1- the slow arraycopy call which has Catch and CatchProj nodes. But with
> validated arraycopies, there shouldn't be a slow arraycopy alone but at
> least one stub call on one of the control path.
>
> 2- a checkcast arraycopy for which the return of the call needs to be
> checked: checkcast arraycopy are explicitly skipped for validated
> arraycopies.
>
> 3- if the copy size is null, PhaseMacroExpand::generate_arraycopy()
> would generate no call but that special case should be handled by
> ArrayCopyNode::Ideal.
>
> 4- A generic arraycopy (if C2 can't tell whether inputs to arraycopy are
> arrays).
>
> I found that 4 can happen eventhough given the arraycopy is validated,
> it could be avoided. So here is a new webrev which sets src_elem =
> dest_elem in PhaseMacroExpand::expand_arraycopy_node() when dest_elem is
> known and the arraycopy is validated.
>
> http://cr.openjdk.java.net/~roland/8179678/webrev.03/
>
> Roland.
>