RFR: JDK-8301491: C2: java.lang.StringUTF16::indexOfChar intrinsic called with negative character argument

[Damon Fenacci]

On Thu, 16 Feb 2023 07:54:48 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> The `java.lang.StringUTF16::indexOfChar` is supposed to return -1 for characters with value `< 0`. Its intrinsic methods don't always do so.
>> https://github.com/openjdk/jdk/blob/96c50a3486e3b6cdce7f8fb409d015b289770811/src/java.base/share/classes/java/lang/StringUTF16.java#L535
>> 
>> The intrinsic methods expect the `int` character being passed to be `>= 0`. Unfortunately this is not enforced in the Java part (`indexOf` only checks for the upper bound):
>> https://github.com/openjdk/jdk/blob/96c50a3486e3b6cdce7f8fb409d015b289770811/src/java.base/share/classes/java/lang/StringUTF16.java#L430
>> The intrinsic methods assume that only the lower 16 bits are used (0 <=  `ch` <= 0xFFFF) and either don't care about the upper 16 bits or implicitly mask them, e.g. for aarch64:
>> https://github.com/openjdk/jdk/blob/96c50a3486e3b6cdce7f8fb409d015b289770811/src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp#L502-L503 or avx:
>> https://github.com/openjdk/jdk/blob/96c50a3486e3b6cdce7f8fb409d015b289770811/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L3039
>> 
>> On the other hand, the Java method `indexOfCharUnsafe` makes this check implicitly by comparing a `char` with an `int`;
>> https://github.com/openjdk/jdk/blob/96c50a3486e3b6cdce7f8fb409d015b289770811/src/java.base/share/classes/java/lang/StringUTF16.java#L542-L544
>> 
>> As there doesn't seem to be a good reason to call `indexOfChar` with `ch < 0`, it seems reasonable to add a check in `LibraryCallKit::inline_string_indexOfChar` that triggers a deoptimization if it fails.
>
> Looks good to me.

@TobiHartmann @vnkozlov thanks a lot for your reviews!

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

PR: https://git.openjdk.org/jdk/pull/12538