On Mon, 21 Jul 2025 13:28:00 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:

>> 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...
>
> Volkan Yazici has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix bit shifting

Minor: could you please add a bug id to the `@bug` annotations in tests?

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

PR Comment: https://git.openjdk.org/jdk/pull/25998#issuecomment-3097286276

Reply via email to