> Validate input in `java.lang.StringCoding` intrinsic Java wrappers, improve > their documentation, enhance the checks in the associated IR or assembly > code, and adapt them to cause VM crash on invalid input. > > ## Implementation notes > > The goal of the associated umbrella issue > [JDK-8156534](https://bugs.openjdk.org/browse/JDK-8156534) is to, for > `java.lang.String*` classes, > > 1. Move `@IntrinsicCandidate`-annotated `public` methods<sup>1</sup> (in Java > code) to `private` ones, and wrap them with a `public` ["front door" > method](https://github.com/openjdk/jdk/pull/24982#discussion_r2087493446) > 2. Since we moved the `@IntrinsicCandidate` annotation to a new method, > intrinsic mappings – i.e., associated `do_intrinsic()` calls in > `vmIntrinsics.hpp` – need to be updated too > 3. Add necessary input validation (range, null, etc.) checks to the newly > created public front door method > 4. Place all input validation checks in the intrinsic code (add if missing!) > behind a `VerifyIntrinsicChecks` VM flag > > Following preliminary work needs to be carried out as well: > > 1. Add a new `VerifyIntrinsicChecks` VM flag > 2. Update `generate_string_range_check` to produce a `HaltNode`. That is, > crash the VM if `VerifyIntrinsicChecks` is set and a Java wrapper fails to > spot an invalid input. > > <sup>1</sup> `@IntrinsicCandidate`-annotated constructors are not subject to > this change, since they are a special case. > > ## Functional and performance tests > > - `tier1` (which includes `test/hotspot/jtreg/compiler/intrinsics/string`) > passes on several platforms. Further tiers will be executed after integrating > reviewer feedback. > > - Performance impact is still actively monitored using > `test/micro/org/openjdk/bench/java/lang/String{En,De}code.java`, among other > tests. If you have suggestions on benchmarks, please share in the comments. > > ## Verification of the VM crash > > I've tested the VM crash scenario as follows: > > 1. Created the following test program: > > public class StrIntri { > public static void main(String[] args) { > Exception lastException = null; > for (int i = 0; i < 1_000_000; i++) { > try { > > jdk.internal.access.SharedSecrets.getJavaLangAccess().countPositives(new > byte[]{1,2,3}, 2, 5); > } catch (Exception exception) { > lastException = exception; > } > } > if (lastException != null) { > lastException.printStackTrace(); > } else { > System.out.println("completed"); > } > } > } > ...
Volkan Yazici has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 23 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into strIntrinCheck - Make `StringCoding` encoding intrinsics lenient - Fix `encodeISOArray` bounds checks and Javadoc - Disable `TestVerifyIntrinsicChecks` for GraalVM - Relax target array capacity check for intrinsic Java wrappers It's not possible to determine the required capacity of the target array in constant time, as Unicode code points may occupy either one or two `char` values. As a result, existing implementations typically invoke encoding methods in a loop, handling each unmappable character on a case-by-case basis. For an example, see `sun.nio.cs.DoubleByte.Encoder::encode`. - Fix out-of-bounds in `sun.nio.cs.SingleByte.Encoder::encodeArrayLoop` - Replace casting with `as_Region()` in `generate_string_range_check` - Duplicate affected tests with `-XX:+VerifyIntrinsicChecks` variants - Fix compiler error in `generate_string_range_check` - Add test verifying the effectiveness of `VerifyIntrinsicChecks` - ... and 13 more: https://git.openjdk.org/jdk/compare/c179d680...f69374fb ------------- Changes: - all: https://git.openjdk.org/jdk/pull/25998/files - new: https://git.openjdk.org/jdk/pull/25998/files/db1ed388..f69374fb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25998&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25998&range=07-08 Stats: 13609 lines in 414 files changed: 10649 ins; 1054 del; 1906 mod Patch: https://git.openjdk.org/jdk/pull/25998.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25998/head:pull/25998 PR: https://git.openjdk.org/jdk/pull/25998
