[14] RFR (S): 8231430: C2: Memory stomp in max_array_length() for T_ILLEGAL type

[Vladimir Ivanov]

Thanks for the review, Vladimir.

> May be we should have permanent guarantee() in 
> TypeAryPtr::max_array_length() for all types which we don't expect to 
> see and not temporary assert().

Makes sense. What do you think about the following?

   http://cr.openjdk.java.net/~vlivanov/8231430/webrev.01/

Best regards,
Vladimir Ivanov

> On 11/27/19 5:54 AM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8231430/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8231430
>>
>> There's a memory stomp happening in max_array_length() for T_ILLEGAL 
>> type. T_ILLEGAL type arises as an element basic type for a merge of 2 
>> primitive arrays (bottom[]). max_array_length() does some input 
>> normalization (T_ILLEGAL => T_BYTE), but first it acquires a reference 
>> to the a cache slot which is out-of-bounds (T_ILLEGAL = 99 vs 
>> T_CONFLICT = 19).
>>
>> I was able to reproduce the problem as a corruption of one of the OOPs 
>> in Universe::_mirrors array which happened to be put close enough to 
>> max_array_length_cache in memory.
>>
>> I propose to completely remove the cache. 
>> arrayOopDesc::max_array_length() doesn't look too expensive and the 
>> method is not used on a hot path anywhere.
>>
>> Also, I put an assert for T_VOID, T_CONFLICT, T_NARROWKLASS cases, but 
>> left the logic there (=> T_BYTE) to get more testing before removing 
>> them.
>>
>> Testing: hs-precheckin-comp, tier1-5.
>>
>> Best regards,
>> Vladimir Ivanov