RFR: 8267904: C2 crash when compile negative Arrays.copyOf length after loop [v4]

[Roland Westrelin]

On Wed, 2 Jun 2021 01:03:57 GMT, Hui Shi <hshi at openjdk.org> wrote:

>> C2 crash when Arrays.copyOf has a negative length after a loop. This happens in release and debug build. Test and hs_err are in JBS.
>> 
>> Crash reason is:
>> - CastIINode is created in GraphKit::new_array (in AllocateArrayNode::make_ideal_length), Cast array lenght  to range [0, maxint-2]. This is safe when allocation is success and CastIINode 's input control is InitializeNode's proj control.
>> - In LibraryCallKit::inline_arraycopy, InitializeNode's proj control's use nodes' control is replaced with AllocateArrayNode's input control (in LibraryCallKit::arraycopy_move_allocation_here). This is necessary to move allocation after array copy checks. But this also includes CastIINode.
>> 
>>    C->gvn_replace_by(init->proj_out(TypeFunc::Control), alloc->in(0));
>> 
>> - CastIINode's control is also adjust to AllocateArrayNode's input control, which is illegal state in laster IGVN phase, casting a negative to [0, maxint-2].
>> - This cause control and nodes after loop become top and removed. The previous loop has no fall-through edge and crash.
>> 
>> Fix is:
>> - In LibraryCallKit::arraycopy_move_allocation_here
>>     - Before replacing init->proj_out(TypeFunc::Control) in, find and replace  CastIINode nodes with original array length.
>>     - After move allocation node, create CastIINode again if necessary.
>> 
>> Before fix:  node 250 is CastII which should be after InitializeNode.
>> ![image](https://user-images.githubusercontent.com/70356247/119938428-f7fa4e80-bfbe-11eb-925e-c239620c73f3.png)
>> 
>> After fix: all arry copy check is performed on original array length node 203
>> ![image](https://user-images.githubusercontent.com/70356247/119938532-2415cf80-bfbf-11eb-98c6-76e6b19b691f.png)
>> 
>> New test test/hotspot/jtreg/compiler/c2/TestNegativeArrayCopyAfterLoop.java is added and pass.
>> Tests performs on Linux X64 and no regression
>> - Tier1/2/3/hotspot_all_no_apps on release and fastdebug build.
>> - Tier1/2/3 with option "-XX:-TieredCompilation -Xbatch" on fastdebug build
>
> Hui Shi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add test case

src/hotspot/share/opto/graphKit.cpp line 3987:

> 3985:       ccast = _gvn.transform(ccast);
> 3986:       record_for_igvn(ccast);
> 3987:       replace_in_map(length, ccast);

_gvn.set_type_bottom(ccast);
is not needed then. 
record_for_igvn(ccast);
is likely not needed either. No sure the reason there was no call to transform() in the first place.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4238