RFR: 8374582: [REDO] Move input validation checks to Java for java.lang.StringCoding intrinsics
ExE Boss
duke at openjdk.org
Tue Jan 20 18:54:14 UTC 2026
On Wed, 14 Jan 2026 10:05:23 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:
>> ## Issue
>>
>> This is a redo of [JDK-8361842](https://bugs.openjdk.org/browse/JDK-8361842) which was backed out by [JDK-8374210](https://bugs.openjdk.org/browse/JDK-8374210) due to C2-related regressions. The original change moved input validation checks for java.lang.StringCoding from the intrinsic to Java code (leaving the intrinsic check only with the `VerifyIntrinsicChecks` flag). Refer to the [original PR](https://github.com/openjdk/jdk/pull/25998) for details.
>>
>> This additional issue happens because, in some cases, for instance when the Java checking code is not inlined and we give an out-of-range constant as input, we fold the data path but not the control path and we crash in the backend.
>>
>> ## Causes
>>
>> The cause of this is that the out-of-range constant (e.g. -1) floats into the intrinsic and there (assuming the input is valid) we add a constraint to its type to positive integers (e.g. to compute the array address) which makes it top.
>>
>> ## Fix
>>
>> A possible fix is to introduce an opaque node (OpaqueGuardNode) similar to what we do in `must_be_not_null` for values that we know cannot be null:
>> https://github.com/openjdk/jdk/blob/ce721665cd61d9a319c667d50d9917c359d6c104/src/hotspot/share/opto/graphKit.cpp#L1484
>> This will temporarily add the range check to ensure that C2 figures that out-of-range values cannot reach the intrinsic. Then, during macro expansion, we replace the opaque node with the corresponding constant (true/false) in product builds such that the actually unneeded guards are folded and do not end up in the emitted code.
>>
>> # Testing
>>
>> * Tier 1-3+
>> * 2 JTReg tests added
>> * `TestRangeCheck.java` as regression test for the reported issue
>> * `TestOpaqueGuardNodes.java` to check that opaque guard nodes are added when parsing and removed at macro expansion
>
> test/hotspot/jtreg/compiler/intrinsics/string/TestRangeCheck.java line 58:
>
>> 56: // cut off the dead code. As a result, -1 is fed as input into the
>> 57: // StringCoding::countPositives0 intrinsic which is replaced by TOP and causes a
>> 58: // failure in the matcher.
>
> I'd appreciate it if we can be more elaborate for less C2-illiterate people like myself. 😇
>
> Suggestion:
>
> // Calling `StringCoding::countPositives`, which is a "front door"
> // to the `StringCoding::countPositives0` intrinsic.
> // `countPositives` validates its input using
> // `Preconditions::checkFromIndexSize`, which also maps to an
> // intrinsic. When `checkFromIndexSize` is not inlined, C2 does not
> // know about the explicit range checks, and does not cut off the
> // dead code. As a result, an invalid value (e.g., `-1`) can be fed
> // as input into the `countPositives0` intrinsic, got replaced
> // by TOP, and cause a failure in the matcher.
**Nit:** Using “get” here is grammatically better:
// intrinsic. When `checkFromIndexSize` is not inlined, C2 does not
// know about the explicit range checks, and does not cut off the
- // as input into the `countPositives0` intrinsic, got replaced
+ // as input into the `countPositives0` intrinsic, get replaced
// by TOP, and cause a failure in the matcher.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29164#discussion_r2690260434
More information about the security-dev
mailing list