On Fri, 23 Jan 2026 13:47:53 GMT, Volkan Yazici <[email protected]> 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
>
> I'd like to provide some help for reviewers:
> 
> 1. [JDK-8361842] (integrated in 655dc516c22) implemented changes for 
> `java.lang.StringCoding`
> 2. [JDK-8374210] (integrated in 7e18de137c3) reported regressions against 
> JDK-8361842, and used as the BACKOUT issue.
> 3. [JDK-8374582] (this PR) is the REDO of JDK-8361842, plus the fix for 
> regressions reported in JDK-8374210
> 
> That is, this PR starts with 3c466d372b7 (i.e, the revert of 7e18de137c3), 
> and continues with the fix, which is **the interesting part, and that can be 
> viewed by diff'ing 3c466d372b7...ff22857609d**. (ff22857609d is the last 
> commit as of date.)
> 
> [JDK-8361842]: https://bugs.openjdk.org/browse/JDK-8361842
> [JDK-8374210]: https://bugs.openjdk.org/browse/JDK-8374210
> [JDK-8374582]: https://bugs.openjdk.org/browse/JDK-8374582

Thanks a lot for your reviews @vy @chhagedorn @TobiHartmann!

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

PR Comment: https://git.openjdk.org/jdk/pull/29164#issuecomment-3858562112

Reply via email to