RFR(S): 8159611: C2: ArrayCopy elimination skips required parameter checks

[Tobias Hartmann]

Hi Volker,

thanks for taking care of this issue!

Did you check what happens if the allocation is not eliminated and macro expansion phase emits another negative guard? Are the checks merged?

I would prefer brackets around the if body but you don't need to send another webrev:
 if (EliminateAllocations) {
   generate_negative_guard(length, slow_region);
 }

Best regards,
Tobias

On 12.08.2016 21:13, Volker Simonis wrote:
> Hi,
> 
> can I please have a review and sponsor for the following fix:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2016/8159611
> https://bugs.openjdk.java.net/browse/JDK-8159611
> 
> 
> We are inserting several checks for the arguments of
> System.arraycopy() in LibraryCallKit::inline_arraycopy() before
> intensifying the call in LibraryCallKit::inline_arraycopy. However the
> check for the 'length' argument of arracopy is postponed to the macro
> expansion phase in PhaseMacroExpand::generate_arraycopy().
> 
> But if we are running with EscapeAnalysis and EliminateAllocations,
> the array allocations inside a call to System.arraycopy() may get
> eliminated and thus the complete call to System.arraycopy() will be
> removed (see PhaseMacroExpand::process_users_of_allocation). In this
> case the extra 'length' check won't be added by
> PhaseMacroExpand::generate_arraycopy() any more because macro
> expansion happens after the elimination of macro nodes.
> 
> In such a case it may happen that System.arraycopy() will silently
> accept an invalid (i.e. negative) 'length' parameter, although it
> should actually throw an ArrayOutOfBounds exception.
> 
> The fix is simple: also insert a check for the length field in
> LibraryCallKit::inline_arraycopy() if we are running with
> EliminateAllocations.
> 
> Regards,
> Volker
>