RFR: 8328702: C2: Crash during parsing because sub type check is not folded

[Christian Hagedorn]

On Wed, 27 Mar 2024 14:37:05 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> The test case shows a problem where data is folded during parsing while control is not. This leaves the graph in a broken state and we fail with an assertion.
>> 
>> We have the following (pseudo) code for some class `X`:
>> 
>> o = flag ? new Object[] : new byte[]; 
>> if (o instanceof X) {
>>    X x = (X)o; // checkcast
>> }
>> 
>> For the `checkcast`, C2 knows that the type of `o` is some kind of array, i.e. type `[bottom`. But this cannot be a sub type of `X`. Therefore, the `CheckCastPP` node created for the `checkcast` result is replaced by top by the type system. However, the `SubTypeCheckNode` for the `checkcast` is not folded and the graph is broken.
>> 
>> The problem of not folding the `SubTypeCheckNode` can be traced back to `SubTypeCheckNode::sub` calling `static_subtype_check()` when transforming the node after it's creation. `static_subtype_check()` should detect that the sub type check is always wrong here:
>> https://github.com/openjdk/jdk/blob/d0a265039a36292d87b249af0e8977982e5acc7b/src/hotspot/share/opto/compile.cpp#L4454-L4460
>> 
>> But it does not because these two checks return the following:
>> 1. Check: is `o` a sub type of `X`? -> returns no, so far so good.
>> 2. Check: _could_ `o` be a sub type of `X`? -> returns no which is wrong! `[bottom` is only a sub type of `Object` and can never be a subtype of `X`
>> 
>> In `maybe_java_subtype_of_helper_for_arr()`, we wrongly conclude that any array with a base element type `bottom` _could_ be a sub type of anything:
>> https://github.com/openjdk/jdk/blob/d0a265039a36292d87b249af0e8977982e5acc7b/src/hotspot/share/opto/type.cpp#L6462-L6465
>> But this is only true if the super class is also an array class - but not if `other` (super klass) is an instance klass as in this case.
>> 
>> The fix for this is to first check the immediately following check which handles the case of comparing an array klass to an instance klass: An array klass can only ever be a sub class of an instance klass if it's the `Object` class. But in our case, we have `X` and this would return false:
>> 
>> https://github.com/openjdk/jdk/blob/d0a265039a36292d87b249af0e8977982e5acc7b/src/hotspot/share/opto/type.cpp#L6466-L6468
>> 
>> The very same problem can also be triggered with `X` being an interface instead. There are tests for both these cases.
>> 
>> #### Additionally Required Fix
>> When running with `-XX:+ExpandSubTypeCheckAtParseTime`, we eagerly expand the sub type check during parsing and therefore do not emit a `SubTypeCheckNode`. Wh...
>
> src/hotspot/share/opto/type.cpp line 6465:
> 
>> 6463:   }
>> 6464:   if (this_one->is_instance_type(other)) {
>> 6465:     return other->klass()->equals(ciEnv::current()->Object_klass()) && other->_interfaces->intersection_with(this_one->_interfaces)->eq(other->_interfaces);
> 
> `TypeInterfaces` has a `contains` method that does `intersection_with` + `eq`. Could we use it here? i.e. `this_one->_interfaces->contains(other->_interfaces)`

That would definitely be better but I've seen that there are three other uses of `intersection_with` + `eq`. We should probably update them all together but not sure if I should squeeze this in here. Should I follow up with an RFE?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18512#discussion_r1541342059