On Wed, 7 May 2025 16:36:09 GMT, Chen Liang <li...@openjdk.org> wrote:
>> While the compiler does not allow invalid queries to flow into
>> `SwitchBootstraps:typeSwitch`, a library user could do that and `typeSwitch`
>> does not prevent such usage pattern errors resulting in erroneous evaluation.
>>
>> For example this is not valid Java (and protected) by javac:
>>
>>
>> byte b = 1;
>> switch (b) {
>> case String s -> System.out.println("How did we get here? byte is " +
>> s.getClass());
>> }
>>
>>
>> but this is a valid call (and not protected):
>>
>>
>> CallSite shortSwitch = SwitchBootstraps.typeSwitch(
>> MethodHandles.lookup(),
>> "",
>> MethodType.methodType(int.class, short.class, int.class), // models
>> (short, int) -> int
>> String.class);
>>
>>
>> The `SwitchBootstraps.typeSwitch` returns wrong result since the code was
>> reasoning erroneously that this pair was unconditionally exact.
>>
>> This PR proposes to add the safety check in unconditional exactness which
>> will return false in erroneous pairs and then the actual check will be
>> delegated to `instanceof`. For the case of erroneous pairs with primitive
>> `boolean`s there is a check in the beginning of the type switch skeleton.
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 553:
>
>> 551: continue;
>> 552: }
>> 553: else if (unconditionalExactnessCheck(selectorType,
>> classLabel)) {
>
> Can we merge this into `isNotValidPair(...) ||
> unconditionalExactnessCheck(...)` then do nothing? The next label is already
> in theory immediately bound to the instruction after goto.
The empty body means that we unconditionally return the index `case 0 -> /*no
if is generated*/ return 0;`.
I confirmed that by merging, the following test would not pass. It returns
erroneously 0, instead of 1.
testPrimitiveType((byte) 1, byte.class,0, 1, boolean.class, byte.class);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25090#discussion_r2078206536
