RFR: 8361842: Move input validation checks to Java for String-related intrinsics [v3]
Volkan Yazici
vyazici at openjdk.org
Tue Jul 15 20:24:04 UTC 2025
> 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 12 additional commits since the last revision:
- Merge remote-tracking branch 'upstream/master' into strIntrinCheck
- Apply review feedback (styling changes)
- Minimize the number of touched lines in `vmIntrinsics.hpp`
- Remove Markdown-styling in comments
- Improve wording of the `VerifyIntrinsicChecks` flag
- Merge remote-tracking branch 'upstream/master' into strIntrinCheck
- Fix `EUC_JP.java.template` broken due to `encodeASCII` rename
- Remove `StringCodingCountPositives`, `String{En,De}code` already cover our cases
This reverts commit 196fc5d406851b8e7070c97ac53ca59c4615aad9.
- Improve intrinsics in `StringCoding`
- Add `StringCodingCountPositives` benchmark
- ... and 2 more: https://git.openjdk.org/jdk/compare/b424f31d...85f19864
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/25998/files
- new: https://git.openjdk.org/jdk/pull/25998/files/7c042b35..85f19864
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=25998&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=25998&range=01-02
Stats: 7104 lines in 199 files changed: 2886 ins; 3418 del; 800 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
More information about the graal-dev
mailing list