## 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

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

Commit messages:
 - JDK-8374852: revert unchanged tests
 - JDK-8374852: shorten line lenght in test
 - JDK-8374852: revert comment change
 - JDK-8374852: correct  comment and make more concise
 - Update test/hotspot/jtreg/compiler/intrinsics/string/TestRangeCheck.java
 - JDK-8374852: fix generate_limit_guard opaque handling and remove unneeded 
positive flag
 - JDK-8374852: remove compileonly
 - JDK-8374852: remove VerifyIntrinsicChecks and refactor opaque flag
 - JDK-8374852: add forgotten opaque guard node handling in clone_iff
 - JDK-8374852: 120 max char for comment
 - ... and 8 more: https://git.openjdk.org/jdk/compare/6d1bfdf7...ff228576

Changes: https://git.openjdk.org/jdk/pull/29164/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=29164&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8374582
  Stats: 435 lines in 28 files changed: 328 ins; 20 del; 87 mod
  Patch: https://git.openjdk.org/jdk/pull/29164.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29164/head:pull/29164

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

Reply via email to