From ac5df9f3a779110111c0e400ec10b9bf94a65c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 26 Jun 2025 12:34:22 +0200 Subject: [PATCH 01/32] Move `StringCoding::countPositives` checks from C++ to Java --- src/hotspot/share/classfile/vmIntrinsics.hpp | 2 +- src/hotspot/share/opto/c2_globals.hpp | 3 ++ src/hotspot/share/opto/library_call.cpp | 45 ++++++++++++------- src/hotspot/share/opto/library_call.hpp | 12 +++-- .../share/classes/java/lang/StringCoding.java | 10 ++++- 5 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/hotspot/share/classfile/vmIntrinsics.hpp b/src/hotspot/share/classfile/vmIntrinsics.hpp index 5be372075ed65..77d2b718c7fde 100644 --- a/src/hotspot/share/classfile/vmIntrinsics.hpp +++ b/src/hotspot/share/classfile/vmIntrinsics.hpp @@ -415,7 +415,7 @@ class methodHandle; \ do_class(java_lang_StringCoding, "java/lang/StringCoding") \ do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \ - do_name( countPositives_name, "countPositives") \ + do_name( countPositives_name, "countPositives0") \ do_signature(countPositives_signature, "([BII)I") \ \ do_class(sun_nio_cs_iso8859_1_Encoder, "sun/nio/cs/ISO_8859_1$Encoder") \ diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index ca76114af3166..eafe1557c3468 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -659,6 +659,9 @@ product(bool, PrintIntrinsics, false, DIAGNOSTIC, \ "prints attempted and successful inlining of intrinsics") \ \ + develop(bool, VerifyIntrinsicRangeChecks, false, \ + "Enable range checks in intrinsics") \ + \ develop(bool, StressReflectiveCode, false, \ "Use inexact types at allocations, etc., to test reflection") \ \ diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 94f047dc115e8..19110e7ba8792 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -851,7 +851,8 @@ void LibraryCallKit::set_result(RegionNode* region, PhiNode* value) { // or null if it is obvious that the slow path can never be taken. // Also, if region and the slow control are not null, the slow edge // is appended to the region. -Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_prob) { +Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_prob, + bool halt) { if (stopped()) { // Already short circuited. return nullptr; @@ -872,8 +873,13 @@ Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_ return nullptr; } - if (region != nullptr) + if (halt) { + Node *frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); + Node* halt = _gvn.transform(new HaltNode(if_slow, frame, "unexpected guard failure in intrinsic")); + C->root()->add_req(halt); + } else if (region != nullptr) { region->add_req(if_slow); + } Node* if_fast = _gvn.transform(new IfFalseNode(iff)); set_control(if_fast); @@ -889,14 +895,15 @@ inline Node* LibraryCallKit::generate_fair_guard(Node* test, RegionNode* region) } inline Node* LibraryCallKit::generate_negative_guard(Node* index, RegionNode* region, - Node* *pos_index) { + Node* *pos_index, + bool halt) { if (stopped()) return nullptr; // already stopped if (_gvn.type(index)->higher_equal(TypeInt::POS)) // [0,maxint] return nullptr; // index is already adequately typed Node* cmp_lt = _gvn.transform(new CmpINode(index, intcon(0))); Node* bol_lt = _gvn.transform(new BoolNode(cmp_lt, BoolTest::lt)); - Node* is_neg = generate_guard(bol_lt, region, PROB_MIN); + Node* is_neg = generate_guard(bol_lt, region, PROB_MIN, halt); if (is_neg != nullptr && pos_index != nullptr) { // Emulate effect of Parse::adjust_map_after_if. Node* ccast = new CastIINode(control(), index, TypeInt::POS); @@ -922,7 +929,8 @@ inline Node* LibraryCallKit::generate_negative_guard(Node* index, RegionNode* re inline Node* LibraryCallKit::generate_limit_guard(Node* offset, Node* subseq_length, Node* array_length, - RegionNode* region) { + RegionNode* region, + bool halt) { if (stopped()) return nullptr; // already stopped bool zero_offset = _gvn.type(offset) == TypeInt::ZERO; @@ -933,12 +941,16 @@ inline Node* LibraryCallKit::generate_limit_guard(Node* offset, last = _gvn.transform(new AddINode(last, offset)); Node* cmp_lt = _gvn.transform(new CmpUNode(array_length, last)); Node* bol_lt = _gvn.transform(new BoolNode(cmp_lt, BoolTest::lt)); - Node* is_over = generate_guard(bol_lt, region, PROB_MIN); + Node* is_over = generate_guard(bol_lt, region, PROB_MIN, halt); return is_over; } // Emit range checks for the given String.value byte array -void LibraryCallKit::generate_string_range_check(Node* array, Node* offset, Node* count, bool char_count) { +void LibraryCallKit::generate_string_range_check(Node* array, + Node* offset, + Node* count, + bool char_count, + bool halt) { if (stopped()) { return; // already stopped } @@ -950,10 +962,10 @@ void LibraryCallKit::generate_string_range_check(Node* array, Node* offset, Node } // Offset and count must not be negative - generate_negative_guard(offset, bailout); - generate_negative_guard(count, bailout); + generate_negative_guard(offset, bailout, nullptr, halt); + generate_negative_guard(count, bailout, nullptr, halt); // Offset + count must not exceed length of array - generate_limit_guard(offset, count, load_array_length(array), bailout); + generate_limit_guard(offset, count, load_array_length(array), bailout, halt); if (bailout->req() > 1) { PreserveJVMState pjvms(this); @@ -1128,13 +1140,14 @@ bool LibraryCallKit::inline_countPositives() { Node* offset = argument(1); Node* len = argument(2); - ba = must_be_not_null(ba, true); - - // Range checks - generate_string_range_check(ba, offset, len, false); - if (stopped()) { - return true; + if (VerifyIntrinsicRangeChecks) { + ba = must_be_not_null(ba, true); + generate_string_range_check(ba, offset, len, false, true); + if (stopped()) { + return true; + } } + Node* ba_start = array_element_address(ba, offset, T_BYTE); Node* result = new CountPositivesNode(control(), memory(TypeAryPtr::BYTES), ba_start, len); set_result(_gvn.transform(result)); diff --git a/src/hotspot/share/opto/library_call.hpp b/src/hotspot/share/opto/library_call.hpp index a622eefa24816..8826b33613266 100644 --- a/src/hotspot/share/opto/library_call.hpp +++ b/src/hotspot/share/opto/library_call.hpp @@ -127,17 +127,21 @@ class LibraryCallKit : public GraphKit { virtual int reexecute_sp() { return _reexecute_sp; } // Helper functions to inline natives - Node* generate_guard(Node* test, RegionNode* region, float true_prob); + Node* generate_guard(Node* test, RegionNode* region, float true_prob, + bool halt = false); Node* generate_slow_guard(Node* test, RegionNode* region); Node* generate_fair_guard(Node* test, RegionNode* region); Node* generate_negative_guard(Node* index, RegionNode* region, // resulting CastII of index: - Node* *pos_index = nullptr); + Node* *pos_index = nullptr, + bool halt = false); Node* generate_limit_guard(Node* offset, Node* subseq_length, Node* array_length, - RegionNode* region); + RegionNode* region, + bool halt = false); void generate_string_range_check(Node* array, Node* offset, - Node* length, bool char_count); + Node* length, bool char_count, + bool halt = false); Node* current_thread_helper(Node* &tls_output, ByteSize handle_offset, bool is_immutable); Node* generate_current_thread(Node* &tls_output); diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index c02af28c37d8b..e217b2c21b8fd 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -28,6 +28,8 @@ import jdk.internal.vm.annotation.IntrinsicCandidate; +import java.util.Objects; + /** * Utility class for string encoding and decoding. */ @@ -86,8 +88,14 @@ public static boolean hasNegatives(byte[] ba, int off, int len) { * a value that is less than or equal to the index of the first negative byte * in the range. */ - @IntrinsicCandidate public static int countPositives(byte[] ba, int off, int len) { + Objects.requireNonNull(ba, "ba"); + Objects.checkFromIndexSize(off, len, ba.length); + return countPositives0(ba, off, len); + } + + @IntrinsicCandidate + private static int countPositives0(byte[] ba, int off, int len) { int limit = off + len; for (int i = off; i < limit; i++) { if (ba[i] < 0) { From 149882416a956dec728a964c150b826dd589908f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 26 Jun 2025 17:40:57 +0200 Subject: [PATCH 02/32] Apply review feedback --- src/hotspot/share/opto/c2_globals.hpp | 4 +- src/hotspot/share/opto/library_call.cpp | 43 +++++++++---------- src/hotspot/share/opto/library_call.hpp | 9 ++-- .../share/classes/java/lang/StringCoding.java | 2 +- 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index eafe1557c3468..2ca82f9687e91 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -659,8 +659,8 @@ product(bool, PrintIntrinsics, false, DIAGNOSTIC, \ "prints attempted and successful inlining of intrinsics") \ \ - develop(bool, VerifyIntrinsicRangeChecks, false, \ - "Enable range checks in intrinsics") \ + develop(bool, VerifyIntrinsicChecks, false, \ + "Verify that Java level checks in intrinsics work as expected") \ \ develop(bool, StressReflectiveCode, false, \ "Use inexact types at allocations, etc., to test reflection") \ diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 19110e7ba8792..86fa61703724d 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -851,8 +851,7 @@ void LibraryCallKit::set_result(RegionNode* region, PhiNode* value) { // or null if it is obvious that the slow path can never be taken. // Also, if region and the slow control are not null, the slow edge // is appended to the region. -Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_prob, - bool halt) { +Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_prob) { if (stopped()) { // Already short circuited. return nullptr; @@ -873,13 +872,8 @@ Node* LibraryCallKit::generate_guard(Node* test, RegionNode* region, float true_ return nullptr; } - if (halt) { - Node *frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); - Node* halt = _gvn.transform(new HaltNode(if_slow, frame, "unexpected guard failure in intrinsic")); - C->root()->add_req(halt); - } else if (region != nullptr) { + if (region != nullptr) region->add_req(if_slow); - } Node* if_fast = _gvn.transform(new IfFalseNode(iff)); set_control(if_fast); @@ -895,15 +889,14 @@ inline Node* LibraryCallKit::generate_fair_guard(Node* test, RegionNode* region) } inline Node* LibraryCallKit::generate_negative_guard(Node* index, RegionNode* region, - Node* *pos_index, - bool halt) { + Node* *pos_index) { if (stopped()) return nullptr; // already stopped if (_gvn.type(index)->higher_equal(TypeInt::POS)) // [0,maxint] return nullptr; // index is already adequately typed Node* cmp_lt = _gvn.transform(new CmpINode(index, intcon(0))); Node* bol_lt = _gvn.transform(new BoolNode(cmp_lt, BoolTest::lt)); - Node* is_neg = generate_guard(bol_lt, region, PROB_MIN, halt); + Node* is_neg = generate_guard(bol_lt, region, PROB_MIN); if (is_neg != nullptr && pos_index != nullptr) { // Emulate effect of Parse::adjust_map_after_if. Node* ccast = new CastIINode(control(), index, TypeInt::POS); @@ -929,8 +922,7 @@ inline Node* LibraryCallKit::generate_negative_guard(Node* index, RegionNode* re inline Node* LibraryCallKit::generate_limit_guard(Node* offset, Node* subseq_length, Node* array_length, - RegionNode* region, - bool halt) { + RegionNode* region) { if (stopped()) return nullptr; // already stopped bool zero_offset = _gvn.type(offset) == TypeInt::ZERO; @@ -941,7 +933,7 @@ inline Node* LibraryCallKit::generate_limit_guard(Node* offset, last = _gvn.transform(new AddINode(last, offset)); Node* cmp_lt = _gvn.transform(new CmpUNode(array_length, last)); Node* bol_lt = _gvn.transform(new BoolNode(cmp_lt, BoolTest::lt)); - Node* is_over = generate_guard(bol_lt, region, PROB_MIN, halt); + Node* is_over = generate_guard(bol_lt, region, PROB_MIN); return is_over; } @@ -962,16 +954,23 @@ void LibraryCallKit::generate_string_range_check(Node* array, } // Offset and count must not be negative - generate_negative_guard(offset, bailout, nullptr, halt); - generate_negative_guard(count, bailout, nullptr, halt); + generate_negative_guard(offset, bailout); + generate_negative_guard(count, bailout); // Offset + count must not exceed length of array - generate_limit_guard(offset, count, load_array_length(array), bailout, halt); + generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { - PreserveJVMState pjvms(this); - set_control(_gvn.transform(bailout)); - uncommon_trap(Deoptimization::Reason_intrinsic, - Deoptimization::Action_maybe_recompile); + if (halt) { + Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); + Node* bailoutN = _gvn.transform(bailout); + Node* halt = _gvn.transform(new HaltNode(bailoutN, frame, "unexpected guard failure in intrinsic")); + C->root()->add_req(halt); + } else { + PreserveJVMState pjvms(this); + set_control(_gvn.transform(bailout)); + uncommon_trap(Deoptimization::Reason_intrinsic, + Deoptimization::Action_maybe_recompile); + } } } @@ -1140,7 +1139,7 @@ bool LibraryCallKit::inline_countPositives() { Node* offset = argument(1); Node* len = argument(2); - if (VerifyIntrinsicRangeChecks) { + if (VerifyIntrinsicChecks) { ba = must_be_not_null(ba, true); generate_string_range_check(ba, offset, len, false, true); if (stopped()) { diff --git a/src/hotspot/share/opto/library_call.hpp b/src/hotspot/share/opto/library_call.hpp index 8826b33613266..2b2dd162061e8 100644 --- a/src/hotspot/share/opto/library_call.hpp +++ b/src/hotspot/share/opto/library_call.hpp @@ -127,18 +127,15 @@ class LibraryCallKit : public GraphKit { virtual int reexecute_sp() { return _reexecute_sp; } // Helper functions to inline natives - Node* generate_guard(Node* test, RegionNode* region, float true_prob, - bool halt = false); + Node* generate_guard(Node* test, RegionNode* region, float true_prob); Node* generate_slow_guard(Node* test, RegionNode* region); Node* generate_fair_guard(Node* test, RegionNode* region); Node* generate_negative_guard(Node* index, RegionNode* region, // resulting CastII of index: - Node* *pos_index = nullptr, - bool halt = false); + Node* *pos_index = nullptr); Node* generate_limit_guard(Node* offset, Node* subseq_length, Node* array_length, - RegionNode* region, - bool halt = false); + RegionNode* region); void generate_string_range_check(Node* array, Node* offset, Node* length, bool char_count, bool halt = false); diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index e217b2c21b8fd..7c2efd313c870 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2024, Alibaba Group Holding Limited. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * From 196fc5d406851b8e7070c97ac53ca59c4615aad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 4 Jul 2025 16:34:21 +0200 Subject: [PATCH 03/32] Add `StringCodingCountPositives` benchmark --- make/test/BuildMicrobenchmark.gmk | 1 + .../java/lang/StringCodingCountPositives.java | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java diff --git a/make/test/BuildMicrobenchmark.gmk b/make/test/BuildMicrobenchmark.gmk index 347ca44d25f39..573ea475d57e9 100644 --- a/make/test/BuildMicrobenchmark.gmk +++ b/make/test/BuildMicrobenchmark.gmk @@ -88,6 +88,7 @@ $(eval $(call SetupJavaCompilation, BUILD_JDK_MICROBENCHMARK, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVAC_FLAGS := \ + --add-exports java.base/jdk.internal.access=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile.components=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile.impl=ALL-UNNAMED \ --add-exports java.base/jdk.internal.event=ALL-UNNAMED \ diff --git a/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java b/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java new file mode 100644 index 0000000000000..c839dbeb23616 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.lang; + +import jdk.internal.access.JavaLangAccess; +import jdk.internal.access.SharedSecrets; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.CompilerControl; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Warmup; + +import java.nio.charset.StandardCharsets; +import java.util.concurrent.TimeUnit; + +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 5, time = 3) +@Measurement(iterations = 5, time = 3) +@Fork(value = 2, jvmArgs = {"--add-exports=java.base/jdk.internal.access=ALL-UNNAMED"}) +public class StringCodingCountPositives { + + public static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); + + private static final byte[] BUFFER = """ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam ac sem eu + urna egestas placerat. Etiam finibus ipsum nulla, non mattis dolor cursus a. + Nulla nec nisl consectetur, lacinia neque id, accumsan ante. Curabitur et + sapien in magna porta ultricies. Sed vel pellentesque nibh. Pellentesque dictum + dignissim diam eu ultricies. Class aptent taciti sociosqu ad litora torquent + per conubia nostra, per inceptos himenaeos. Suspendisse erat diam, fringilla + sed massa sed, posuere viverra orci. Suspendisse tempor libero non gravida + efficitur. Vivamus lacinia risus non orci viverra, at consectetur odio laoreet. + Suspendisse potenti.""".getBytes(StandardCharsets.UTF_8); + + @Benchmark + @CompilerControl(CompilerControl.Mode.DONT_INLINE) + public int countPositives() { + return JLA.countPositives(BUFFER, 0, BUFFER.length); + } + +} From 9932dd34612256adef193b445e4eadefad62984f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 9 Jul 2025 12:26:29 +0200 Subject: [PATCH 04/32] Improve intrinsics in `StringCoding` --- .../cpu/aarch64/macroAssembler_aarch64.cpp | 12 ++- .../cpu/riscv/c2_MacroAssembler_riscv.cpp | 12 ++- src/hotspot/cpu/x86/macroAssembler_x86.cpp | 66 ++++++++----- src/hotspot/share/classfile/vmIntrinsics.hpp | 16 ++-- src/hotspot/share/opto/library_call.cpp | 23 ++++- .../share/classes/java/lang/String.java | 2 +- .../share/classes/java/lang/StringCoding.java | 95 ++++++++++++++++--- .../share/classes/java/lang/System.java | 5 +- .../jdk/internal/access/JavaLangAccess.java | 23 +++-- .../share/classes/sun/nio/cs/CESU_8.java | 4 +- .../share/classes/sun/nio/cs/DoubleByte.java | 4 +- .../share/classes/sun/nio/cs/ISO_8859_1.java | 52 +++++----- .../share/classes/sun/nio/cs/SingleByte.java | 2 +- .../share/classes/sun/nio/cs/US_ASCII.java | 2 +- .../share/classes/sun/nio/cs/UTF_8.java | 2 +- 15 files changed, 217 insertions(+), 103 deletions(-) diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index cbd941397f305..167ab3cdd100e 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -6421,10 +6421,14 @@ void MacroAssembler::fill_words(Register base, Register cnt, Register value) // Intrinsic for // -// - sun/nio/cs/ISO_8859_1$Encoder.implEncodeISOArray -// return the number of characters copied. -// - java/lang/StringUTF16.compress -// return index of non-latin1 character if copy fails, otherwise 'len'. +// - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// Encodes `char[]` to `byte[]` in ISO-8859-1 +// +// - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// Encodes `byte[]` (containing UTF-16) to `byte[]` in ISO-8859-1 +// +// - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) +// Encodes `char[]` to `byte[]` in ASCII // // This version always returns the number of characters copied, and does not // clobber the 'len' register. A successful copy will complete with the post- diff --git a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp index ce13ebde74f9b..8725deec8ab67 100644 --- a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp +++ b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp @@ -2825,10 +2825,14 @@ void C2_MacroAssembler::char_array_compress_v(Register src, Register dst, Regist // Intrinsic for // -// - sun/nio/cs/ISO_8859_1$Encoder.implEncodeISOArray -// return the number of characters copied. -// - java/lang/StringUTF16.compress -// return index of non-latin1 character if copy fails, otherwise 'len'. +// - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// Encodes `char[]` to `byte[]` in ISO-8859-1 +// +// - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// Encodes `byte[]` (containing UTF-16) to `byte[]` in ISO-8859-1 +// +// - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) +// Encodes `char[]` to `byte[]` in ASCII // // This version always returns the number of characters copied. A successful // copy will complete with the post-condition: 'res' == 'len', while an diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp index c401863d7cdc8..4faace822435d 100644 --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp @@ -6011,32 +6011,46 @@ void MacroAssembler::evpbroadcast(BasicType type, XMMRegister dst, Register src, } } -// encode char[] to byte[] in ISO_8859_1 or ASCII - //@IntrinsicCandidate - //private static int implEncodeISOArray(byte[] sa, int sp, - //byte[] da, int dp, int len) { - // int i = 0; - // for (; i < len; i++) { - // char c = StringUTF16.getChar(sa, sp++); - // if (c > '\u00FF') - // break; - // da[dp++] = (byte)c; - // } - // return i; - //} - // - //@IntrinsicCandidate - //private static int implEncodeAsciiArray(char[] sa, int sp, - // byte[] da, int dp, int len) { - // int i = 0; - // for (; i < len; i++) { - // char c = sa[sp++]; - // if (c >= '\u0080') - // break; - // da[dp++] = (byte)c; - // } - // return i; - //} +// Encode given `char[]`/`byte[]` to `byte[]` in ISO_8859_1 or ASCII +// +// @IntrinsicCandidate +// int sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0( +// char[] sa, int sp, byte[] da, int dp, int len) { +// int i = 0; +// for (; i < len; i++) { +// char c = sa[sp++]; +// if (c > '\u00FF') +// break; +// da[dp++] = (byte) c; +// } +// return i; +// } +// +// @IntrinsicCandidate +// int java.lang.StringCoding.encodeISOArray0( +// byte[] sa, int sp, byte[] da, int dp, int len) { +// int i = 0; +// for (; i < len; i++) { +// char c = StringUTF16.getChar(sa, sp++); +// if (c > '\u00FF') +// break; +// da[dp++] = (byte) c; +// } +// return i; +// } +// +// @IntrinsicCandidate +// int java.lang.StringCoding.encodeAsciiArray0( +// char[] sa, int sp, byte[] da, int dp, int len) { +// int i = 0; +// for (; i < len; i++) { +// char c = sa[sp++]; +// if (c >= '\u0080') +// break; +// da[dp++] = (byte) c; +// } +// return i; +// } void MacroAssembler::encode_iso_array(Register src, Register dst, Register len, XMMRegister tmp1Reg, XMMRegister tmp2Reg, XMMRegister tmp3Reg, XMMRegister tmp4Reg, diff --git a/src/hotspot/share/classfile/vmIntrinsics.hpp b/src/hotspot/share/classfile/vmIntrinsics.hpp index 77d2b718c7fde..000ce560ef523 100644 --- a/src/hotspot/share/classfile/vmIntrinsics.hpp +++ b/src/hotspot/share/classfile/vmIntrinsics.hpp @@ -414,19 +414,19 @@ class methodHandle; do_signature(Preconditions_checkLongIndex_signature, "(JJLjava/util/function/BiFunction;)J") \ \ do_class(java_lang_StringCoding, "java/lang/StringCoding") \ - do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \ - do_name( countPositives_name, "countPositives0") \ - do_signature(countPositives_signature, "([BII)I") \ + do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \ + do_name( countPositives_name, "countPositives0") \ + do_signature(countPositives_signature, "([BII)I") \ \ do_class(sun_nio_cs_iso8859_1_Encoder, "sun/nio/cs/ISO_8859_1$Encoder") \ - do_intrinsic(_encodeISOArray, sun_nio_cs_iso8859_1_Encoder, encodeISOArray_name, encodeISOArray_signature, F_S) \ - do_name( encodeISOArray_name, "implEncodeISOArray") \ + do_intrinsic(_encodeISOArray, sun_nio_cs_iso8859_1_Encoder, encodeISOArray_name, encodeISOArray_signature, F_S) \ + do_name( encodeISOArray_name, "encodeISOArray0") \ do_signature(encodeISOArray_signature, "([CI[BII)I") \ \ - do_intrinsic(_encodeByteISOArray, java_lang_StringCoding, encodeISOArray_name, indexOfI_signature, F_S) \ + do_intrinsic(_encodeByteISOArray, java_lang_StringCoding, encodeISOArray_name, indexOfI_signature, F_S) \ \ - do_intrinsic(_encodeAsciiArray, java_lang_StringCoding, encodeAsciiArray_name, encodeISOArray_signature, F_S) \ - do_name( encodeAsciiArray_name, "implEncodeAsciiArray") \ + do_intrinsic(_encodeAsciiArray, java_lang_StringCoding, encodeAsciiArray_name, encodeISOArray_signature, F_S) \ + do_name( encodeAsciiArray_name, "encodeAsciiArray0") \ \ do_class(java_math_BigInteger, "java/math/BigInteger") \ do_intrinsic(_multiplyToLen, java_math_BigInteger, multiplyToLen_name, multiplyToLen_signature, F_S) \ diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 86fa61703724d..d658c10e8821e 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -1128,6 +1128,7 @@ bool LibraryCallKit::inline_array_equals(StrIntrinsicNode::ArgEnc ae) { //------------------------------inline_countPositives------------------------------ +// int java.lang.StringCoding#countPositives0(byte[] ba, int off, int len) bool LibraryCallKit::inline_countPositives() { if (too_many_traps(Deoptimization::Reason_intrinsic)) { return false; @@ -6119,6 +6120,9 @@ CallStaticJavaNode* LibraryCallKit::get_uncommon_trap_from_success_proj(Node* no } //-------------inline_encodeISOArray----------------------------------- +// int sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// int java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) +// int java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) // encode char[] to byte[] in ISO_8859_1 or ASCII bool LibraryCallKit::inline_encodeISOArray(bool ascii) { assert(callee()->signature()->size() == 5, "encodeISOArray has 5 parameters"); @@ -6129,8 +6133,14 @@ bool LibraryCallKit::inline_encodeISOArray(bool ascii) { Node *dst_offset = argument(3); Node *length = argument(4); - src = must_be_not_null(src, true); - dst = must_be_not_null(dst, true); + // Cast source & target arrays to not-null + if (VerifyIntrinsicChecks) { + src = must_be_not_null(src, true); + dst = must_be_not_null(dst, true); + if (stopped()) { + return true; + } + } const TypeAryPtr* src_type = src->Value(&_gvn)->isa_aryptr(); const TypeAryPtr* dst_type = dst->Value(&_gvn)->isa_aryptr(); @@ -6147,6 +6157,15 @@ bool LibraryCallKit::inline_encodeISOArray(bool ascii) { return false; } + // Check source & target bounds + if (VerifyIntrinsicChecks) { + generate_string_range_check(src, src_offset, length, src_elem == T_BYTE, true); + generate_string_range_check(dst, dst_offset, length, false, true); + if (stopped()) { + return true; + } + } + Node* src_start = array_element_address(src, src_offset, T_CHAR); Node* dst_start = array_element_address(dst, dst_offset, dst_elem); // 'src_start' points to src array + scaled offset diff --git a/src/java.base/share/classes/java/lang/String.java b/src/java.base/share/classes/java/lang/String.java index 52af7dfd50376..d51f08ace14c6 100644 --- a/src/java.base/share/classes/java/lang/String.java +++ b/src/java.base/share/classes/java/lang/String.java @@ -1036,7 +1036,7 @@ private static byte[] encode8859_1(byte coder, byte[] val, boolean doReplace) { int sp = 0; int sl = len; while (sp < sl) { - int ret = StringCoding.implEncodeISOArray(val, sp, dst, dp, len); + int ret = StringCoding.encodeISOArray(val, sp, dst, dp, len); sp = sp + ret; dp = dp + ret; if (ret != len) { diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 7c2efd313c870..6a1496293cc1b 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -26,9 +26,14 @@ package java.lang; +import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; -import java.util.Objects; +import java.util.function.BiFunction; + +import static java.util.Objects.requireNonNull; +import static jdk.internal.util.Preconditions.AIOOBE_FORMATTER; +import static jdk.internal.util.Preconditions.checkFromIndexSize; /** * Utility class for string encoding and decoding. @@ -40,7 +45,7 @@ private StringCoding() { } /** * Count the number of leading non-zero ascii chars in the range. */ - public static int countNonZeroAscii(String s) { + static int countNonZeroAscii(String s) { byte[] value = s.value(); if (s.isLatin1()) { return countNonZeroAsciiLatin1(value, 0, value.length); @@ -52,7 +57,7 @@ public static int countNonZeroAscii(String s) { /** * Count the number of non-zero ascii chars in the range. */ - public static int countNonZeroAsciiLatin1(byte[] ba, int off, int len) { + private static int countNonZeroAsciiLatin1(byte[] ba, int off, int len) { int limit = off + len; for (int i = off; i < limit; i++) { if (ba[i] <= 0) { @@ -65,7 +70,7 @@ public static int countNonZeroAsciiLatin1(byte[] ba, int off, int len) { /** * Count the number of leading non-zero ascii chars in the range. */ - public static int countNonZeroAsciiUTF16(byte[] ba, int off, int strlen) { + private static int countNonZeroAsciiUTF16(byte[] ba, int off, int strlen) { int limit = off + strlen; for (int i = off; i < limit; i++) { char c = StringUTF16.charAt(ba, i); @@ -76,7 +81,7 @@ public static int countNonZeroAsciiUTF16(byte[] ba, int off, int strlen) { return strlen; } - public static boolean hasNegatives(byte[] ba, int off, int len) { + static boolean hasNegatives(byte[] ba, int off, int len) { return countPositives(ba, off, len) != len; } @@ -87,10 +92,16 @@ public static boolean hasNegatives(byte[] ba, int off, int len) { * bytes in the range. If there are negative bytes, the implementation must return * a value that is less than or equal to the index of the first negative byte * in the range. + * + * @param ba a byte array + * @param off the index of the first byte to start reading from + * @param len the total number of bytes to read + * @throws NullPointerException if {@code ba} is null + * @throws ArrayIndexOutOfBoundsException if the provided sub-range is + * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ - public static int countPositives(byte[] ba, int off, int len) { - Objects.requireNonNull(ba, "ba"); - Objects.checkFromIndexSize(off, len, ba.length); + static int countPositives(byte[] ba, int off, int len) { + checkFromIndexSize(off, len, requireNonNull(ba, "ba").length, AIOOBE_FORMATTER); return countPositives0(ba, off, len); } @@ -105,29 +116,83 @@ private static int countPositives0(byte[] ba, int off, int len) { return len; } + /** + * Encodes as many ISO-8859-1 codepoints as possible from the source byte + * array containing characters encoded in UTF-16, into the destination byte + * array, assuming that the encoding is ISO-8859-1 compatible. + * + * @apiNote + * + * {@code sa} denotes the {@code byte[]} backing a {@link String}. When + * {@linkplain String#COMPACT_STRINGS compact strings} are disabled, a + * {@code char} is always represented by 2 bytes, i.e., + * encoded in UTF-16. When enabled, if the content is ISO-8859-1, a + * {@code char} is represented by 1 byte; otherwise again by 2 bytes. + *

+ * This method assumes that {@code sa} is encoded in UTF-16, and hence, + * each {@code char} maps to 2 bytes. + *

+ * {@code sp} is encoded in ISO-8859-1. There each {@code byte} corresponds + * to a {@code char}. + *

+ * + * @param sa the source byte array containing characters encoded in UTF-16 + * @param sp the index of the byte (not character!) from the source array to start reading from + * @param da the target byte array + * @param dp the index of the target array to start writing to + * @param len the total number of characters (not bytes!) to be encoded + * @return the total number of characters (not bytes!) successfully encoded + * @throws NullPointerException if any of the provided arrays is null + * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are + * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} + */ + static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { + checkFromIndexSize(sp, len << 1, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); + checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + return encodeISOArray0(sa, sp, da, dp, len); + } + @IntrinsicCandidate - public static int implEncodeISOArray(byte[] sa, int sp, - byte[] da, int dp, int len) { + private static int encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = StringUTF16.getChar(sa, sp++); if (c > '\u00FF') break; - da[dp++] = (byte)c; + da[dp++] = (byte) c; } return i; } + /** + * Encodes as many ASCII codepoints as possible from the source + * character array into the destination byte array, assuming that + * the encoding is ASCII compatible. + * + * @param sa the source character array + * @param sp the index of the source array to start reading from + * @param da the target byte array + * @param dp the index of the target array to start writing to + * @param len the total number of characters to be encoded + * @return the total number of characters successfully encoded + * @throws NullPointerException if any of the provided arrays is null + * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are + * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} + */ + static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { + checkFromIndexSize(sp, len, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); + checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + return encodeAsciiArray0(sa, sp, da, dp, len); + } + @IntrinsicCandidate - public static int implEncodeAsciiArray(char[] sa, int sp, - byte[] da, int dp, int len) - { + private static int encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = sa[sp++]; if (c >= '\u0080') break; - da[dp++] = (byte)c; + da[dp++] = (byte) c; } return i; } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index 0175558d31348..2026726dc4329 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -55,7 +55,6 @@ import java.util.ResourceBundle; import java.util.Set; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import java.util.function.Supplier; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; @@ -2153,8 +2152,8 @@ public int decodeASCII(byte[] src, int srcOff, char[] dst, int dstOff, int len) return String.decodeASCII(src, srcOff, dst, dstOff, len); } - public int uncheckedEncodeASCII(char[] src, int srcOff, byte[] dst, int dstOff, int len) { - return StringCoding.implEncodeAsciiArray(src, srcOff, dst, dstOff, len); + public int encodeASCII(char[] sa, int sp, byte[] da, int dp, int len) { + return StringCoding.encodeAsciiArray(sa, sp, da, dp, len); } public InputStream initialSystemIn() { diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java index e8343274caca7..e4972c83890f0 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java @@ -45,6 +45,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; +import java.util.function.BiFunction; import java.util.stream.Stream; import jdk.internal.loader.NativeLibraries; @@ -417,15 +418,21 @@ public interface JavaLangAccess { PrintStream initialSystemErr(); /** - * Encodes as many ASCII codepoints as possible from the source array into - * the destination byte array, assuming that the encoding is ASCII - * compatible. - *

- * WARNING: This method does not perform any bound checks. + * Encodes as many ASCII codepoints as possible from the source + * character array into the destination byte array, assuming that + * the encoding is ASCII compatible. * - * @return the number of bytes successfully encoded, or 0 if none - */ - int uncheckedEncodeASCII(char[] src, int srcOff, byte[] dst, int dstOff, int len); + * @param sa the source character array + * @param sp the index of the source array to start reading from + * @param da the target byte array + * @param dp the index of the target array to start writing to + * @param len the total number of characters to be encoded + * @return the total number of characters successfully encoded + * @throws NullPointerException if any of the provided arrays is null + * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are + * {@linkplain jdk.internal.util.Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} + */ + int encodeASCII(char[] sa, int sp, byte[] da, int dp, int len); /** * Set the cause of Throwable diff --git a/src/java.base/share/classes/sun/nio/cs/CESU_8.java b/src/java.base/share/classes/sun/nio/cs/CESU_8.java index a9a25e151ad11..9b907bcbc65b2 100644 --- a/src/java.base/share/classes/sun/nio/cs/CESU_8.java +++ b/src/java.base/share/classes/sun/nio/cs/CESU_8.java @@ -446,7 +446,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, int dl = dst.arrayOffset() + dst.limit(); // Handle ASCII-only prefix - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); sp += n; dp += n; @@ -551,7 +551,7 @@ public int encode(char[] sa, int sp, int len, byte[] da) { int dp = 0; // Handle ASCII-only prefix - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(len, da.length)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(len, da.length)); sp += n; dp += n; diff --git a/src/java.base/share/classes/sun/nio/cs/DoubleByte.java b/src/java.base/share/classes/sun/nio/cs/DoubleByte.java index 4738f51515b7f..165e1e21c0fab 100644 --- a/src/java.base/share/classes/sun/nio/cs/DoubleByte.java +++ b/src/java.base/share/classes/sun/nio/cs/DoubleByte.java @@ -600,7 +600,7 @@ protected CoderResult encodeArrayLoop(CharBuffer src, ByteBuffer dst) { try { if (isASCIICompatible) { - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(dl - dp, sl - sp)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(dl - dp, sl - sp)); sp += n; dp += n; } @@ -686,7 +686,7 @@ public int encode(char[] src, int sp, int len, byte[] dst) { int dp = 0; int sl = sp + len; if (isASCIICompatible) { - int n = JLA.uncheckedEncodeASCII(src, sp, dst, dp, len); + int n = JLA.encodeASCII(src, sp, dst, dp, len); sp += n; dp += n; } diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 39215bfa93d29..88b828e6ed8a5 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -31,13 +31,17 @@ import java.nio.charset.CharsetDecoder; import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; -import java.util.Objects; +import java.util.function.BiFunction; import jdk.internal.access.JavaLangAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; +import static java.util.Objects.requireNonNull; +import static jdk.internal.util.Preconditions.AIOOBE_FORMATTER; +import static jdk.internal.util.Preconditions.checkFromIndexSize; + public class ISO_8859_1 extends Charset implements HistoricallyNamedCharset @@ -142,41 +146,39 @@ public boolean isLegalReplacement(byte[] repl) { private final Surrogate.Parser sgp = new Surrogate.Parser(); - // Method possible replaced with a compiler intrinsic. - private static int encodeISOArray(char[] sa, int sp, - byte[] da, int dp, int len) { - if (len <= 0) { - return 0; - } - encodeISOArrayCheck(sa, sp, da, dp, len); - return implEncodeISOArray(sa, sp, da, dp, len); + /** + * Encodes as many ISO-8859-1 codepoints as possible from the source + * character array into the destination byte array, assuming that + * the encoding is ISO-8859-1 compatible. + * + * @param sa the source character array + * @param sp the index of the source array to start reading from + * @param da the target byte array + * @param dp the index of the target array to start writing to + * @param len the total number of characters to be encoded + * @return the total number of characters successfully encoded + * @throws NullPointerException if any of the provided arrays is null + * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are + * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} + */ + private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { + checkFromIndexSize(sp, len, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); + checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + return encodeISOArray0(sa, sp, da, dp, len); } @IntrinsicCandidate - private static int implEncodeISOArray(char[] sa, int sp, - byte[] da, int dp, int len) - { + private static int encodeISOArray0(char[] sa, int sp, byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = sa[sp++]; if (c > '\u00FF') break; - da[dp++] = (byte)c; + da[dp++] = (byte) c; } return i; } - private static void encodeISOArrayCheck(char[] sa, int sp, - byte[] da, int dp, int len) { - Objects.requireNonNull(sa); - Objects.requireNonNull(da); - Preconditions.checkIndex(sp, sa.length, Preconditions.AIOOBE_FORMATTER); - Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); - - Preconditions.checkIndex(sp + len - 1, sa.length, Preconditions.AIOOBE_FORMATTER); - Preconditions.checkIndex(dp + len - 1, da.length, Preconditions.AIOOBE_FORMATTER); - } - private CoderResult encodeArrayLoop(CharBuffer src, ByteBuffer dst) { @@ -196,7 +198,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, int slen = sl - sp; int len = (dlen < slen) ? dlen : slen; try { - int ret = encodeISOArray(sa, sp, da, dp, len); + int ret = len <= 0 ? 0 : encodeISOArray(sa, sp, da, dp, len); sp = sp + ret; dp = dp + ret; if (ret != len) { diff --git a/src/java.base/share/classes/sun/nio/cs/SingleByte.java b/src/java.base/share/classes/sun/nio/cs/SingleByte.java index d802cc85aa8af..8efa6b295ff0f 100644 --- a/src/java.base/share/classes/sun/nio/cs/SingleByte.java +++ b/src/java.base/share/classes/sun/nio/cs/SingleByte.java @@ -217,7 +217,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, ByteBuffer dst) { int len = Math.min(dl - dp, sl - sp); if (isASCIICompatible) { - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, len); + int n = JLA.encodeASCII(sa, sp, da, dp, len); sp += n; dp += n; len -= n; diff --git a/src/java.base/share/classes/sun/nio/cs/US_ASCII.java b/src/java.base/share/classes/sun/nio/cs/US_ASCII.java index bb84ab1bd4b42..3886978d2093d 100644 --- a/src/java.base/share/classes/sun/nio/cs/US_ASCII.java +++ b/src/java.base/share/classes/sun/nio/cs/US_ASCII.java @@ -159,7 +159,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, assert (dp <= dl); dp = (dp <= dl ? dp : dl); - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); sp += n; dp += n; diff --git a/src/java.base/share/classes/sun/nio/cs/UTF_8.java b/src/java.base/share/classes/sun/nio/cs/UTF_8.java index 54e479f838aed..d2d6d7e485d8b 100644 --- a/src/java.base/share/classes/sun/nio/cs/UTF_8.java +++ b/src/java.base/share/classes/sun/nio/cs/UTF_8.java @@ -452,7 +452,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, int dl = dst.arrayOffset() + dst.limit(); // Handle ASCII-only prefix - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(sl - sp, dl - dp)); sp += n; dp += n; From 14275e5517c730284f97339a640873ec7d7e3352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 10 Jul 2025 09:53:03 +0200 Subject: [PATCH 05/32] Remove `StringCodingCountPositives`, `String{En,De}code` already cover our cases This reverts commit 196fc5d406851b8e7070c97ac53ca59c4615aad9. --- make/test/BuildMicrobenchmark.gmk | 1 - .../java/lang/StringCodingCountPositives.java | 65 ------------------- 2 files changed, 66 deletions(-) delete mode 100644 test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java diff --git a/make/test/BuildMicrobenchmark.gmk b/make/test/BuildMicrobenchmark.gmk index 573ea475d57e9..347ca44d25f39 100644 --- a/make/test/BuildMicrobenchmark.gmk +++ b/make/test/BuildMicrobenchmark.gmk @@ -88,7 +88,6 @@ $(eval $(call SetupJavaCompilation, BUILD_JDK_MICROBENCHMARK, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVAC_FLAGS := \ - --add-exports java.base/jdk.internal.access=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile.components=ALL-UNNAMED \ --add-exports java.base/jdk.internal.classfile.impl=ALL-UNNAMED \ --add-exports java.base/jdk.internal.event=ALL-UNNAMED \ diff --git a/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java b/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java deleted file mode 100644 index c839dbeb23616..0000000000000 --- a/test/micro/org/openjdk/bench/java/lang/StringCodingCountPositives.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package org.openjdk.bench.java.lang; - -import jdk.internal.access.JavaLangAccess; -import jdk.internal.access.SharedSecrets; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.CompilerControl; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Warmup; - -import java.nio.charset.StandardCharsets; -import java.util.concurrent.TimeUnit; - -@BenchmarkMode(Mode.AverageTime) -@OutputTimeUnit(TimeUnit.NANOSECONDS) -@Warmup(iterations = 5, time = 3) -@Measurement(iterations = 5, time = 3) -@Fork(value = 2, jvmArgs = {"--add-exports=java.base/jdk.internal.access=ALL-UNNAMED"}) -public class StringCodingCountPositives { - - public static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); - - private static final byte[] BUFFER = """ - Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam ac sem eu - urna egestas placerat. Etiam finibus ipsum nulla, non mattis dolor cursus a. - Nulla nec nisl consectetur, lacinia neque id, accumsan ante. Curabitur et - sapien in magna porta ultricies. Sed vel pellentesque nibh. Pellentesque dictum - dignissim diam eu ultricies. Class aptent taciti sociosqu ad litora torquent - per conubia nostra, per inceptos himenaeos. Suspendisse erat diam, fringilla - sed massa sed, posuere viverra orci. Suspendisse tempor libero non gravida - efficitur. Vivamus lacinia risus non orci viverra, at consectetur odio laoreet. - Suspendisse potenti.""".getBytes(StandardCharsets.UTF_8); - - @Benchmark - @CompilerControl(CompilerControl.Mode.DONT_INLINE) - public int countPositives() { - return JLA.countPositives(BUFFER, 0, BUFFER.length); - } - -} From b9a6adf16c08b4ee4c31e66a3e4ef66dcd65865a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 10 Jul 2025 10:25:18 +0200 Subject: [PATCH 06/32] Fix `EUC_JP.java.template` broken due to `encodeASCII` rename --- .../share/classes/sun/nio/cs/ext/EUC_JP.java.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jdk.charsets/share/classes/sun/nio/cs/ext/EUC_JP.java.template b/src/jdk.charsets/share/classes/sun/nio/cs/ext/EUC_JP.java.template index 4fc0b2796ee65..f3d03a9e9c70b 100644 --- a/src/jdk.charsets/share/classes/sun/nio/cs/ext/EUC_JP.java.template +++ b/src/jdk.charsets/share/classes/sun/nio/cs/ext/EUC_JP.java.template @@ -309,7 +309,7 @@ public class EUC_JP try { if (enc0201.isASCIICompatible()) { - int n = JLA.uncheckedEncodeASCII(sa, sp, da, dp, Math.min(dl - dp, sl - sp)); + int n = JLA.encodeASCII(sa, sp, da, dp, Math.min(dl - dp, sl - sp)); sp += n; dp += n; } From c331fbfad922b2c0cf18d5653e9e45f0e1f9c4ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 15 Jul 2025 21:10:09 +0200 Subject: [PATCH 07/32] Improve wording of the `VerifyIntrinsicChecks` flag --- src/hotspot/share/opto/c2_globals.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index b7a77155953f0..0a8f187a2d158 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -663,7 +663,7 @@ "prints attempted and successful inlining of intrinsics") \ \ develop(bool, VerifyIntrinsicChecks, false, \ - "Verify that Java level checks in intrinsics work as expected") \ + "Verify in intrinsic that Java level checks work as expected") \ \ develop(bool, StressReflectiveCode, false, \ "Use inexact types at allocations, etc., to test reflection") \ From b60ff457453fc410f32cca13fed0c3438f9da979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 15 Jul 2025 21:12:17 +0200 Subject: [PATCH 08/32] Remove Markdown-styling in comments --- src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp | 6 +++--- src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp | 6 +++--- src/hotspot/cpu/x86/macroAssembler_x86.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 8f796f8171471..b4546cc4e754f 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -6422,13 +6422,13 @@ void MacroAssembler::fill_words(Register base, Register cnt, Register value) // Intrinsic for // // - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) -// Encodes `char[]` to `byte[]` in ISO-8859-1 +// Encodes char[] to byte[] in ISO-8859-1 // // - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) -// Encodes `byte[]` (containing UTF-16) to `byte[]` in ISO-8859-1 +// Encodes byte[] (containing UTF-16) to byte[] in ISO-8859-1 // // - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) -// Encodes `char[]` to `byte[]` in ASCII +// Encodes char[] to byte[] in ASCII // // This version always returns the number of characters copied, and does not // clobber the 'len' register. A successful copy will complete with the post- diff --git a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp index 8725deec8ab67..2bbc157d87cb8 100644 --- a/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp +++ b/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp @@ -2826,13 +2826,13 @@ void C2_MacroAssembler::char_array_compress_v(Register src, Register dst, Regist // Intrinsic for // // - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) -// Encodes `char[]` to `byte[]` in ISO-8859-1 +// Encodes char[] to byte[] in ISO-8859-1 // // - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) -// Encodes `byte[]` (containing UTF-16) to `byte[]` in ISO-8859-1 +// Encodes byte[] (containing UTF-16) to byte[] in ISO-8859-1 // // - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) -// Encodes `char[]` to `byte[]` in ASCII +// Encodes char[] to byte[] in ASCII // // This version always returns the number of characters copied. A successful // copy will complete with the post-condition: 'res' == 'len', while an diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp index fd139b2f2adef..d2948ed5ee3dc 100644 --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp @@ -6011,7 +6011,7 @@ void MacroAssembler::evpbroadcast(BasicType type, XMMRegister dst, Register src, } } -// Encode given `char[]`/`byte[]` to `byte[]` in ISO_8859_1 or ASCII +// Encode given char[]/byte[] to byte[] in ISO_8859_1 or ASCII // // @IntrinsicCandidate // int sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0( From 7c042b35bfe02c7e1172d1d47a3e2814531485eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 15 Jul 2025 21:16:36 +0200 Subject: [PATCH 09/32] Minimize the number of touched lines in `vmIntrinsics.hpp` --- src/hotspot/share/classfile/vmIntrinsics.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/classfile/vmIntrinsics.hpp b/src/hotspot/share/classfile/vmIntrinsics.hpp index 5cf8930295eaa..cf917254fcf33 100644 --- a/src/hotspot/share/classfile/vmIntrinsics.hpp +++ b/src/hotspot/share/classfile/vmIntrinsics.hpp @@ -414,18 +414,18 @@ class methodHandle; do_signature(Preconditions_checkLongIndex_signature, "(JJLjava/util/function/BiFunction;)J") \ \ do_class(java_lang_StringCoding, "java/lang/StringCoding") \ - do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \ - do_name( countPositives_name, "countPositives0") \ - do_signature(countPositives_signature, "([BII)I") \ + do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \ + do_name( countPositives_name, "countPositives0") \ + do_signature(countPositives_signature, "([BII)I") \ \ do_class(sun_nio_cs_iso8859_1_Encoder, "sun/nio/cs/ISO_8859_1$Encoder") \ - do_intrinsic(_encodeISOArray, sun_nio_cs_iso8859_1_Encoder, encodeISOArray_name, encodeISOArray_signature, F_S) \ + do_intrinsic(_encodeISOArray, sun_nio_cs_iso8859_1_Encoder, encodeISOArray_name, encodeISOArray_signature, F_S) \ do_name( encodeISOArray_name, "encodeISOArray0") \ do_signature(encodeISOArray_signature, "([CI[BII)I") \ \ - do_intrinsic(_encodeByteISOArray, java_lang_StringCoding, encodeISOArray_name, indexOfI_signature, F_S) \ + do_intrinsic(_encodeByteISOArray, java_lang_StringCoding, encodeISOArray_name, indexOfI_signature, F_S) \ \ - do_intrinsic(_encodeAsciiArray, java_lang_StringCoding, encodeAsciiArray_name, encodeISOArray_signature, F_S) \ + do_intrinsic(_encodeAsciiArray, java_lang_StringCoding, encodeAsciiArray_name, encodeISOArray_signature, F_S) \ do_name( encodeAsciiArray_name, "encodeAsciiArray0") \ \ do_class(java_math_BigInteger, "java/math/BigInteger") \ From 2672f7c1adadd493540570e8439d372edafb1a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 15 Jul 2025 22:17:23 +0200 Subject: [PATCH 10/32] Apply review feedback (styling changes) --- .../share/classes/java/lang/StringCoding.java | 35 +++++++++++-------- .../share/classes/sun/nio/cs/ISO_8859_1.java | 16 ++++----- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 6a1496293cc1b..cc1ef2a80feeb 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -29,12 +29,9 @@ import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; +import java.util.Objects; import java.util.function.BiFunction; -import static java.util.Objects.requireNonNull; -import static jdk.internal.util.Preconditions.AIOOBE_FORMATTER; -import static jdk.internal.util.Preconditions.checkFromIndexSize; - /** * Utility class for string encoding and decoding. */ @@ -101,7 +98,7 @@ static boolean hasNegatives(byte[] ba, int off, int len) { * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ static int countPositives(byte[] ba, int off, int len) { - checkFromIndexSize(off, len, requireNonNull(ba, "ba").length, AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize(off, len, Objects.requireNonNull(ba, "ba").length, Preconditions.AIOOBE_FORMATTER); return countPositives0(ba, off, len); } @@ -146,20 +143,24 @@ private static int countPositives0(byte[] ba, int off, int len) { * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ - static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { - checkFromIndexSize(sp, len << 1, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); - checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + static int encodeISOArray(byte[] sa, int sp, + byte[] da, int dp, int len) { + Objects.requireNonNull(sa, "sa"); + Objects.requireNonNull(da, "da"); + Preconditions.checkFromIndexSize(sp, len << 1, sa.length, Preconditions.AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); return encodeISOArray0(sa, sp, da, dp, len); } @IntrinsicCandidate - private static int encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len) { + private static int encodeISOArray0(byte[] sa, int sp, + byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = StringUTF16.getChar(sa, sp++); if (c > '\u00FF') break; - da[dp++] = (byte) c; + da[dp++] = (byte)c; } return i; } @@ -179,20 +180,24 @@ private static int encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ - static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { - checkFromIndexSize(sp, len, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); - checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + static int encodeAsciiArray(char[] sa, int sp, + byte[] da, int dp, int len) { + Objects.requireNonNull(sa, "sa"); + Objects.requireNonNull(da, "da"); + Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); return encodeAsciiArray0(sa, sp, da, dp, len); } @IntrinsicCandidate - private static int encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) { + private static int encodeAsciiArray0(char[] sa, int sp, + byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = sa[sp++]; if (c >= '\u0080') break; - da[dp++] = (byte) c; + da[dp++] = (byte)c; } return i; } diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 88b828e6ed8a5..f0ae88b3f9cd9 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -31,6 +31,7 @@ import java.nio.charset.CharsetDecoder; import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; +import java.util.Objects; import java.util.function.BiFunction; import jdk.internal.access.JavaLangAccess; @@ -38,10 +39,6 @@ import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; -import static java.util.Objects.requireNonNull; -import static jdk.internal.util.Preconditions.AIOOBE_FORMATTER; -import static jdk.internal.util.Preconditions.checkFromIndexSize; - public class ISO_8859_1 extends Charset implements HistoricallyNamedCharset @@ -161,9 +158,12 @@ public boolean isLegalReplacement(byte[] repl) { * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ - private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { - checkFromIndexSize(sp, len, requireNonNull(sa, "sa").length, AIOOBE_FORMATTER); - checkFromIndexSize(dp, len, requireNonNull(da, "da").length, AIOOBE_FORMATTER); + private static int encodeISOArray(char[] sa, int sp, + byte[] da, int dp, int len) { + Objects.requireNonNull(sa, "sa"); + Objects.requireNonNull(da, "da"); + Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); return encodeISOArray0(sa, sp, da, dp, len); } @@ -174,7 +174,7 @@ private static int encodeISOArray0(char[] sa, int sp, byte[] da, int dp, int len char c = sa[sp++]; if (c > '\u00FF') break; - da[dp++] = (byte) c; + da[dp++] = (byte)c; } return i; } From 2b89e8808cbeacb34224cc5971a43902ab738cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 16 Jul 2025 09:47:52 +0200 Subject: [PATCH 11/32] Improve `generate_string_range_check` changes Co-authored-by: Tobias Hartmann --- src/hotspot/share/opto/library_call.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index bcc520cb41b57..7d7b6df679f1e 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -961,14 +961,14 @@ void LibraryCallKit::generate_string_range_check(Node* array, generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { + bailout = _gvn.transform(bailout); if (halt) { Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); - Node* bailoutN = _gvn.transform(bailout); - Node* halt = _gvn.transform(new HaltNode(bailoutN, frame, "unexpected guard failure in intrinsic")); + Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic")); C->root()->add_req(halt); } else { PreserveJVMState pjvms(this); - set_control(_gvn.transform(bailout)); + set_control(bailout); uncommon_trap(Deoptimization::Reason_intrinsic, Deoptimization::Action_maybe_recompile); } From bcb073cbb41dbdd469b7bfbaf6d2815e8348bcf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 16 Jul 2025 21:34:27 +0200 Subject: [PATCH 12/32] Add test verifying the effectiveness of `VerifyIntrinsicChecks` --- .../share/classes/java/lang/StringCoding.java | 4 +- .../intrinsics/TestVerifyIntrinsicChecks.java | 103 ++++++++++++++++++ .../patches/java.base/java/lang/Helper.java | 5 + 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index cc1ef2a80feeb..6bf5c417d049e 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -190,8 +190,8 @@ static int encodeAsciiArray(char[] sa, int sp, } @IntrinsicCandidate - private static int encodeAsciiArray0(char[] sa, int sp, - byte[] da, int dp, int len) { + static int encodeAsciiArray0(char[] sa, int sp, + byte[] da, int dp, int len) { int i = 0; for (; i < len; i++) { char c = sa[sp++]; diff --git a/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java new file mode 100644 index 0000000000000..ef58425dd6a9c --- /dev/null +++ b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @summary Verify the effectiveness of the `VerifyIntrinsicChecks` VM flag + * through (bypassing `StringCoding::encodeAsciiArray`, and) feeding + * invalid input to an intrinsified `StringCoding::encodeAsciiArray0` + * (note the `0` suffix!). + * @library /compiler/patches + * @library /test/lib + * @build java.base/java.lang.Helper + * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a + * development flag + * @requires vm.debug == true & vm.flavor == "server" + * @run main/othervm compiler.intrinsics.TestVerifyIntrinsicChecks verify + */ + +package compiler.intrinsics; + +import java.lang.Helper; +import java.time.Instant; +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.process.OutputAnalyzer; + +public final class TestVerifyIntrinsicChecks { + + public static void main(String[] args) throws Exception { + switch (args[0]) { + case "verify" -> { + log("Starting JVM in a separate process to verify the crash"); + OutputAnalyzer outputAnalyzer = ProcessTools.executeTestJava( + "-Xcomp", + "-XX:-TieredCompilation", + "-XX:CompileCommand=inline,java.lang.StringCoding::encodeAsciiArray0", + "-XX:+VerifyIntrinsicChecks", + "--patch-module", "java.base=%s/java.base".formatted(System.getProperty("test.patch.path")), + "compiler.intrinsics.TestVerifyIntrinsicChecks", + "crash"); + outputAnalyzer.shouldContain("unexpected null in intrinsic"); + outputAnalyzer.shouldNotHaveExitValue(0); + } + case "crash" -> { + log("Triggering the crash"); + warmUpIntrinsicMethod(); + violateIntrinsicMethodContract(); + } + default -> throw new IllegalArgumentException(); + } + } + + private static void warmUpIntrinsicMethod() { + log("Warming up the intrinsic method"); + char[] sa = createAsciiChars(8192); + byte[] sp = new byte[4096]; + for (int i = 0; i < 1_000; i++) { + Helper.StringCodingEncodeAsciiArray0(sa, i, sp, 0, sp.length - i); + } + } + + private static char[] createAsciiChars(int length) { + char[] buffer = new char[length]; + for (int i = 0; i < length; i++) { + buffer[i] = (char) (i % '\u0080'); + } + return buffer; + } + + private static void violateIntrinsicMethodContract() { + log("Violating the intrinsic method contract (sa=null)"); + Helper.StringCodingEncodeAsciiArray0(null, 1, null, 1, 1); + } + + private synchronized static void log(String format, Object... args) { + Object[] extendedArgs = new Object[2 + args.length]; + extendedArgs[0] = Instant.now(); + extendedArgs[1] = Thread.currentThread().getName(); + System.arraycopy(args, 0, extendedArgs, extendedArgs.length - args.length, args.length); + String extendedFormat = "%%s [%%s] %s%%n".formatted(format); + System.out.printf(extendedFormat, extendedArgs); + } + +} diff --git a/test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java b/test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java index e6c8b68fc6fc0..3985ad8ea90ec 100644 --- a/test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java +++ b/test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java @@ -39,6 +39,11 @@ public static int StringCodingCountPositives(byte[] ba, int off, int len) { return StringCoding.countPositives(ba, off, len); } + @jdk.internal.vm.annotation.ForceInline + public static int StringCodingEncodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len) { + return StringCoding.encodeAsciiArray0(sa, sp, da, dp, len); + } + @jdk.internal.vm.annotation.ForceInline public static byte[] compressByte(byte[] src, int srcOff, int dstSize, int dstOff, int len) { byte[] dst = new byte[dstSize]; From bfc301798d17318b97dd6dffa73548d25176be6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 17 Jul 2025 06:35:51 +0200 Subject: [PATCH 13/32] Fix compiler error in `generate_string_range_check` --- src/hotspot/share/opto/library_call.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 7d7b6df679f1e..c04400b7d4cd9 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -961,7 +961,7 @@ void LibraryCallKit::generate_string_range_check(Node* array, generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { - bailout = _gvn.transform(bailout); + bailout = (RegionNode*) _gvn.transform(bailout); if (halt) { Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic")); From abc0eeb3bfc6ac05b589bd32e6db98f7657e3438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 17 Jul 2025 08:06:49 +0200 Subject: [PATCH 14/32] Duplicate affected tests with `-XX:+VerifyIntrinsicChecks` variants --- .../intrinsics/string/TestCountPositives.java | 26 ++++++++++++++++--- .../string/TestEncodeIntrinsics.java | 18 ++++++++++++- .../intrinsics/string/TestHasNegatives.java | 21 +++++++++++++-- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java index 76ef476615927..3fed124a33d2b 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -32,6 +32,7 @@ * @build java.base/java.lang.Helper * @run main compiler.intrinsics.string.TestCountPositives */ + /* * @test * @bug 8281146 8318509 @@ -46,17 +47,34 @@ * @run main/othervm/timeout=1200 -XX:UseAVX=3 compiler.intrinsics.string.TestCountPositives * @run main/othervm/timeout=1200 -XX:UseAVX=3 -XX:+UnlockDiagnosticVMOptions -XX:AVX3Threshold=0 compiler.intrinsics.string.TestCountPositives */ -/** - * This test was derived from compiler.intrinsics.string.TestHasNegatives + +/* + * @test + * @summary Verify `StringCoding::countPositives` intrinsic Java wrapper checks + * by enabling the ones in the compiler intrinsic using + * `-XX:+VerifyIntrinsicChecks` + * @key randomness + * @library /compiler/patches + * @library /test/lib + * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a + * development flag + * @requires vm.debug == true + * @build java.base/java.lang.Helper + * @run main/othervm + * -XX:+VerifyIntrinsicChecks + * compiler.intrinsics.string.TestCountPositives */ + package compiler.intrinsics.string; import java.lang.Helper; import java.util.Random; -import java.util.stream.IntStream; import jdk.test.lib.Utils; +/** + * This test was derived from {@link TestHasNegatives}. + */ public class TestCountPositives { private static byte[] bytes = new byte[4096 + 32]; diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java index 38a516e7521a6..ffbddf78d7e9d 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,6 +31,22 @@ * @run main/othervm/timeout=1200 --add-opens=java.base/sun.nio.cs=ALL-UNNAMED -Xbatch -Xmx256m compiler.intrinsics.string.TestEncodeIntrinsics */ +/* + * @test + * @summary Verify `sun.nio.cs.ISO_8859_1.Encoder::encodeISOArray` intrinsic + * Java wrapper checks by enabling the ones in the compiler intrinsic + * using `-XX:+VerifyIntrinsicChecks` + * @key randomness + * @library /test/lib + * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a + * development flag + * @requires vm.debug == true + * @run main/othervm/timeout=1200 + * -XX:+VerifyIntrinsicChecks + * --add-opens=java.base/sun.nio.cs=ALL-UNNAMED -Xbatch -Xmx256m + * compiler.intrinsics.string.TestEncodeIntrinsics + */ + package compiler.intrinsics.string; import jdk.test.lib.Utils; diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java index 6edf2dc2e5676..59deffbe7fefb 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,6 +31,7 @@ * @build java.base/java.lang.Helper * @run main compiler.intrinsics.string.TestHasNegatives */ + /* * @test * @bug 8054307 8318509 @@ -46,11 +47,27 @@ * @run main/othervm/timeout=1200 -XX:UseAVX=3 -XX:+UnlockDiagnosticVMOptions -XX:AVX3Threshold=0 compiler.intrinsics.string.TestHasNegatives */ +/* + * @test + * @summary Verify `StringCoding::hasNegatives` intrinsic Java wrapper checks + * by enabling the ones in the compiler intrinsic using + * `-XX:+VerifyIntrinsicChecks` + * @key randomness + * @library /compiler/patches + * @library /test/lib + * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a + * development flag + * @requires vm.debug == true + * @build java.base/java.lang.Helper + * @run main/othervm + * -XX:+VerifyIntrinsicChecks + * compiler.intrinsics.string.TestHasNegatives + */ + package compiler.intrinsics.string; import java.lang.Helper; import java.util.Random; -import java.util.stream.IntStream; import jdk.test.lib.Utils; From db1ed388765344423e131e6157f0ab0ebceb9373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 17 Jul 2025 09:16:00 +0200 Subject: [PATCH 15/32] Replace casting with `as_Region()` in `generate_string_range_check` --- src/hotspot/share/opto/library_call.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index c04400b7d4cd9..d1b4f6d6b4829 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -961,7 +961,7 @@ void LibraryCallKit::generate_string_range_check(Node* array, generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { - bailout = (RegionNode*) _gvn.transform(bailout); + bailout = _gvn.transform(bailout)->as_Region(); if (halt) { Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic")); From 7a6cd39d4367bd2ee0774e247dbcafc59e9b86bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Thu, 17 Jul 2025 15:01:08 +0200 Subject: [PATCH 16/32] Fix out-of-bounds in `sun.nio.cs.SingleByte.Encoder::encodeArrayLoop` --- src/java.base/share/classes/sun/nio/cs/SingleByte.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/sun/nio/cs/SingleByte.java b/src/java.base/share/classes/sun/nio/cs/SingleByte.java index 8efa6b295ff0f..50d20991a2037 100644 --- a/src/java.base/share/classes/sun/nio/cs/SingleByte.java +++ b/src/java.base/share/classes/sun/nio/cs/SingleByte.java @@ -216,7 +216,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, ByteBuffer dst) { int dl = dst.arrayOffset() + dst.limit(); int len = Math.min(dl - dp, sl - sp); - if (isASCIICompatible) { + if (isASCIICompatible && dst.hasRemaining()) { int n = JLA.encodeASCII(sa, sp, da, dp, len); sp += n; dp += n; From 8c712ff29279af0fe23f731b4983489f856aa865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 18 Jul 2025 14:45:33 +0200 Subject: [PATCH 17/32] 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`. --- .../share/classes/java/lang/StringCoding.java | 12 ++++++++++-- .../share/classes/sun/nio/cs/ISO_8859_1.java | 6 +++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 6bf5c417d049e..69329b95029ca 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -148,7 +148,11 @@ static int encodeISOArray(byte[] sa, int sp, Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); Preconditions.checkFromIndexSize(sp, len << 1, sa.length, Preconditions.AIOOBE_FORMATTER); - Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); + // Not checking the `dp + len < da.length` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); return encodeISOArray0(sa, sp, da, dp, len); } @@ -185,7 +189,11 @@ static int encodeAsciiArray(char[] sa, int sp, Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); - Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); + // Not checking the `dp + len < da.length` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); return encodeAsciiArray0(sa, sp, da, dp, len); } diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index f0ae88b3f9cd9..53c3e0d12b0ef 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -163,7 +163,11 @@ private static int encodeISOArray(char[] sa, int sp, Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); - Preconditions.checkFromIndexSize(dp, len, da.length, Preconditions.AIOOBE_FORMATTER); + // Not checking the `dp + len < da.length` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); return encodeISOArray0(sa, sp, da, dp, len); } From 4016c7a1fe708bacf8d4be785f687b98d37ebc6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 18 Jul 2025 20:46:54 +0200 Subject: [PATCH 18/32] Disable `TestVerifyIntrinsicChecks` for GraalVM --- .../jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java index ef58425dd6a9c..7b27bbdafb3a0 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java +++ b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java @@ -32,7 +32,7 @@ * @build java.base/java.lang.Helper * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a * development flag - * @requires vm.debug == true & vm.flavor == "server" + * @requires vm.debug == true & vm.flavor == "server" & !vm.graal.enabled * @run main/othervm compiler.intrinsics.TestVerifyIntrinsicChecks verify */ From 943f84000a7d377afb191e374a571975b7cd29e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 18 Jul 2025 20:12:47 +0200 Subject: [PATCH 19/32] Fix `encodeISOArray` bounds checks and Javadoc --- .../share/classes/java/lang/StringCoding.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 69329b95029ca..8ce7a0cb3a5d2 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -129,12 +129,12 @@ private static int countPositives0(byte[] ba, int off, int len) { * This method assumes that {@code sa} is encoded in UTF-16, and hence, * each {@code char} maps to 2 bytes. *

- * {@code sp} is encoded in ISO-8859-1. There each {@code byte} corresponds + * {@code da} is encoded in ISO-8859-1. There each {@code byte} corresponds * to a {@code char}. *

* * @param sa the source byte array containing characters encoded in UTF-16 - * @param sp the index of the byte (not character!) from the source array to start reading from + * @param sp the index of the character (not byte!) from the source array to start reading from * @param da the target byte array * @param dp the index of the target array to start writing to * @param len the total number of characters (not bytes!) to be encoded @@ -147,7 +147,12 @@ static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - Preconditions.checkFromIndexSize(sp, len << 1, sa.length, Preconditions.AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize( + sp, + len, + // Halving the length of `sa` to obtain the number of characters: + sa.length >>> 1, + Preconditions.AIOOBE_FORMATTER); // Not checking the `dp + len < da.length` invariant, since "as many // codepoints as possible" contract still holds with a `da` of // insufficient capacity, and the compiler intrinsic matches this From fb8f6efea34b402f92037502272aab42cf66e373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 21 Jul 2025 09:49:41 +0200 Subject: [PATCH 20/32] Make `StringCoding` encoding intrinsics lenient --- .../share/classes/java/lang/StringCoding.java | 36 +++++++++---------- .../share/classes/sun/nio/cs/ISO_8859_1.java | 19 +++++----- .../share/classes/sun/nio/cs/SingleByte.java | 2 +- 3 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 8ce7a0cb3a5d2..709c766255724 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -140,24 +140,21 @@ private static int countPositives0(byte[] ba, int off, int len) { * @param len the total number of characters (not bytes!) to be encoded * @return the total number of characters (not bytes!) successfully encoded * @throws NullPointerException if any of the provided arrays is null - * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are - * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - Preconditions.checkFromIndexSize( - sp, - len, + if ((sp | dp | len) < 0 || // Halving the length of `sa` to obtain the number of characters: - sa.length >>> 1, - Preconditions.AIOOBE_FORMATTER); - // Not checking the `dp + len < da.length` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. - Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); + len > (sa.length >> 1) - sp || + dp >= da.length) { + // Not checking the `len < da.length - dp` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + return 0; + } return encodeISOArray0(sa, sp, da, dp, len); } @@ -186,19 +183,18 @@ private static int encodeISOArray0(byte[] sa, int sp, * @param len the total number of characters to be encoded * @return the total number of characters successfully encoded * @throws NullPointerException if any of the provided arrays is null - * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are - * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); - // Not checking the `dp + len < da.length` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. - Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); + if ((sp | dp | len) < 0 || len > sa.length - sp || dp >= da.length) { + // Not checking the `len < da.length - dp` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + return 0; + } return encodeAsciiArray0(sa, sp, da, dp, len); } diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 53c3e0d12b0ef..122913969cbdb 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -32,11 +32,9 @@ import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; import java.util.Objects; -import java.util.function.BiFunction; import jdk.internal.access.JavaLangAccess; import jdk.internal.access.SharedSecrets; -import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; public class ISO_8859_1 @@ -155,19 +153,18 @@ public boolean isLegalReplacement(byte[] repl) { * @param len the total number of characters to be encoded * @return the total number of characters successfully encoded * @throws NullPointerException if any of the provided arrays is null - * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are - * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - Preconditions.checkFromIndexSize(sp, len, sa.length, Preconditions.AIOOBE_FORMATTER); - // Not checking the `dp + len < da.length` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. - Preconditions.checkIndex(dp, da.length, Preconditions.AIOOBE_FORMATTER); + if ((sp | dp | len) < 0 || len > sa.length - sp || dp >= da.length) { + // Not checking the `len < da.length - dp` invariant, since "as many + // codepoints as possible" contract still holds with a `da` of + // insufficient capacity, and the compiler intrinsic matches this + // behavior too. + return 0; + } return encodeISOArray0(sa, sp, da, dp, len); } @@ -202,7 +199,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, int slen = sl - sp; int len = (dlen < slen) ? dlen : slen; try { - int ret = len <= 0 ? 0 : encodeISOArray(sa, sp, da, dp, len); + int ret = encodeISOArray(sa, sp, da, dp, len); sp = sp + ret; dp = dp + ret; if (ret != len) { diff --git a/src/java.base/share/classes/sun/nio/cs/SingleByte.java b/src/java.base/share/classes/sun/nio/cs/SingleByte.java index 50d20991a2037..8efa6b295ff0f 100644 --- a/src/java.base/share/classes/sun/nio/cs/SingleByte.java +++ b/src/java.base/share/classes/sun/nio/cs/SingleByte.java @@ -216,7 +216,7 @@ private CoderResult encodeArrayLoop(CharBuffer src, ByteBuffer dst) { int dl = dst.arrayOffset() + dst.limit(); int len = Math.min(dl - dp, sl - sp); - if (isASCIICompatible && dst.hasRemaining()) { + if (isASCIICompatible) { int n = JLA.encodeASCII(sa, sp, da, dp, len); sp += n; dp += n; From 86e3ed8db18c411b3c3567801a7209482f11a1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 21 Jul 2025 14:47:52 +0200 Subject: [PATCH 21/32] Remove superseded `@throws` Javadoc --- .../share/classes/jdk/internal/access/JavaLangAccess.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java index e4972c83890f0..058fe82b2cd08 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java @@ -429,8 +429,6 @@ public interface JavaLangAccess { * @param len the total number of characters to be encoded * @return the total number of characters successfully encoded * @throws NullPointerException if any of the provided arrays is null - * @throws ArrayIndexOutOfBoundsException if any of the provided sub-ranges are - * {@linkplain jdk.internal.util.Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ int encodeASCII(char[] sa, int sp, byte[] da, int dp, int len); From 025c7ef7805202ea67843bb59dd85c2cc2cfb0c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 21 Jul 2025 15:25:17 +0200 Subject: [PATCH 22/32] Fix bit shifting --- src/java.base/share/classes/java/lang/StringCoding.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 709c766255724..0c27cc40e1b11 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -147,7 +147,7 @@ static int encodeISOArray(byte[] sa, int sp, Objects.requireNonNull(da, "da"); if ((sp | dp | len) < 0 || // Halving the length of `sa` to obtain the number of characters: - len > (sa.length >> 1) - sp || + len > (sa.length >>> 1) - sp || dp >= da.length) { // Not checking the `len < da.length - dp` invariant, since "as many // codepoints as possible" contract still holds with a `da` of From 07cd41c540199d99a92dd094d24e7e9467ff04b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 22 Jul 2025 12:40:26 +0200 Subject: [PATCH 23/32] Cap destination array bounds --- src/java.base/share/classes/java/lang/StringCoding.java | 4 ++-- src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 0c27cc40e1b11..b16951b00ca03 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -155,7 +155,7 @@ static int encodeISOArray(byte[] sa, int sp, // behavior too. return 0; } - return encodeISOArray0(sa, sp, da, dp, len); + return encodeISOArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); } @IntrinsicCandidate @@ -195,7 +195,7 @@ static int encodeAsciiArray(char[] sa, int sp, // behavior too. return 0; } - return encodeAsciiArray0(sa, sp, da, dp, len); + return encodeAsciiArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); } @IntrinsicCandidate diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 122913969cbdb..e50128b8edaec 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -165,7 +165,7 @@ private static int encodeISOArray(char[] sa, int sp, // behavior too. return 0; } - return encodeISOArray0(sa, sp, da, dp, len); + return encodeISOArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); } @IntrinsicCandidate From cb4780d0889e2b763dc813199b0f537244cf92b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 22 Jul 2025 15:06:11 +0200 Subject: [PATCH 24/32] Make source array bound checks lenient too --- .../share/classes/java/lang/StringCoding.java | 19 +++++++------------ .../share/classes/sun/nio/cs/ISO_8859_1.java | 9 +++------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index b16951b00ca03..e7a429ecadd87 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -145,17 +145,15 @@ static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); + int sl; if ((sp | dp | len) < 0 || // Halving the length of `sa` to obtain the number of characters: - len > (sa.length >>> 1) - sp || + sp >= (sl = sa.length >>> 1) || dp >= da.length) { - // Not checking the `len < da.length - dp` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. return 0; } - return encodeISOArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); + int minLen = Math.min(len, Math.min(sl - sp, da.length - dp)); + return encodeISOArray0(sa, sp, da, dp, minLen); } @IntrinsicCandidate @@ -188,14 +186,11 @@ static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - if ((sp | dp | len) < 0 || len > sa.length - sp || dp >= da.length) { - // Not checking the `len < da.length - dp` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. + if ((sp | dp | len) < 0 || sp >= sa.length || dp >= da.length) { return 0; } - return encodeAsciiArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); + int minLen = Math.min(len, Math.min(sa.length - sp, da.length - dp)); + return encodeAsciiArray0(sa, sp, da, dp, minLen); } @IntrinsicCandidate diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index e50128b8edaec..66a90dbd9e35c 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -158,14 +158,11 @@ private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { Objects.requireNonNull(sa, "sa"); Objects.requireNonNull(da, "da"); - if ((sp | dp | len) < 0 || len > sa.length - sp || dp >= da.length) { - // Not checking the `len < da.length - dp` invariant, since "as many - // codepoints as possible" contract still holds with a `da` of - // insufficient capacity, and the compiler intrinsic matches this - // behavior too. + if ((sp | dp | len) < 0 || sp >= sa.length || dp >= da.length) { return 0; } - return encodeISOArray0(sa, sp, da, dp, Math.min(len, da.length - dp)); + int minLen = Math.min(len, Math.min(sa.length - sp, da.length - dp)); + return encodeISOArray0(sa, sp, da, dp, minLen); } @IntrinsicCandidate From dc5e673e516c86d0d4b141a04983413b3315b2c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 23 Jul 2025 12:06:14 +0200 Subject: [PATCH 25/32] Improve wording of `@param len` --- src/java.base/share/classes/java/lang/StringCoding.java | 4 ++-- src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index e7a429ecadd87..8a09d1b2a9c01 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -137,7 +137,7 @@ private static int countPositives0(byte[] ba, int off, int len) { * @param sp the index of the character (not byte!) from the source array to start reading from * @param da the target byte array * @param dp the index of the target array to start writing to - * @param len the total number of characters (not bytes!) to be encoded + * @param len the maximum number of characters (not bytes!) to be encoded * @return the total number of characters (not bytes!) successfully encoded * @throws NullPointerException if any of the provided arrays is null */ @@ -178,7 +178,7 @@ private static int encodeISOArray0(byte[] sa, int sp, * @param sp the index of the source array to start reading from * @param da the target byte array * @param dp the index of the target array to start writing to - * @param len the total number of characters to be encoded + * @param len the maximum number of characters to be encoded * @return the total number of characters successfully encoded * @throws NullPointerException if any of the provided arrays is null */ diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 66a90dbd9e35c..28dab390271ea 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -150,7 +150,7 @@ public boolean isLegalReplacement(byte[] repl) { * @param sp the index of the source array to start reading from * @param da the target byte array * @param dp the index of the target array to start writing to - * @param len the total number of characters to be encoded + * @param len the maximum number of characters to be encoded * @return the total number of characters successfully encoded * @throws NullPointerException if any of the provided arrays is null */ From 1d02189fba1d53230b556b1cc0741fc0be56cfd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Wed, 23 Jul 2025 12:10:33 +0200 Subject: [PATCH 26/32] Add `@bug` tags --- .../jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java | 1 + .../jtreg/compiler/intrinsics/string/TestCountPositives.java | 1 + .../jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java | 1 + .../jtreg/compiler/intrinsics/string/TestHasNegatives.java | 1 + 4 files changed, 4 insertions(+) diff --git a/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java index 7b27bbdafb3a0..392ca35e2fd5f 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java +++ b/test/hotspot/jtreg/compiler/intrinsics/TestVerifyIntrinsicChecks.java @@ -23,6 +23,7 @@ /* * @test + * @bug 8361842 * @summary Verify the effectiveness of the `VerifyIntrinsicChecks` VM flag * through (bypassing `StringCoding::encodeAsciiArray`, and) feeding * invalid input to an intrinsified `StringCoding::encodeAsciiArray0` diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java index 3fed124a33d2b..b684bc6776706 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java @@ -50,6 +50,7 @@ /* * @test + * @bug 8281146 * @summary Verify `StringCoding::countPositives` intrinsic Java wrapper checks * by enabling the ones in the compiler intrinsic using * `-XX:+VerifyIntrinsicChecks` diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java index ffbddf78d7e9d..1968405008501 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java @@ -33,6 +33,7 @@ /* * @test + * @bug 6896617 8274242 * @summary Verify `sun.nio.cs.ISO_8859_1.Encoder::encodeISOArray` intrinsic * Java wrapper checks by enabling the ones in the compiler intrinsic * using `-XX:+VerifyIntrinsicChecks` diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java index 59deffbe7fefb..8f1f06e1ec750 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java @@ -49,6 +49,7 @@ /* * @test + * @bug 8054307 * @summary Verify `StringCoding::hasNegatives` intrinsic Java wrapper checks * by enabling the ones in the compiler intrinsic using * `-XX:+VerifyIntrinsicChecks` From e70dfa3ce61583f60dfc58610b707fd3c9e167c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 25 Jul 2025 09:36:30 +0200 Subject: [PATCH 27/32] Replace `requireNonNull` with implicit null checks to reduce bytecode size --- .../share/classes/java/lang/StringCoding.java | 18 +++++++++--------- .../share/classes/sun/nio/cs/ISO_8859_1.java | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 8a09d1b2a9c01..616b8cd5b4e72 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -29,7 +29,6 @@ import jdk.internal.util.Preconditions; import jdk.internal.vm.annotation.IntrinsicCandidate; -import java.util.Objects; import java.util.function.BiFunction; /** @@ -98,7 +97,10 @@ static boolean hasNegatives(byte[] ba, int off, int len) { * {@linkplain Preconditions#checkFromIndexSize(int, int, int, BiFunction) out of bounds} */ static int countPositives(byte[] ba, int off, int len) { - Preconditions.checkFromIndexSize(off, len, Objects.requireNonNull(ba, "ba").length, Preconditions.AIOOBE_FORMATTER); + Preconditions.checkFromIndexSize( + off, len, + ba.length, // Implicit null check on `ba` + Preconditions.AIOOBE_FORMATTER); return countPositives0(ba, off, len); } @@ -143,13 +145,11 @@ private static int countPositives0(byte[] ba, int off, int len) { */ static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { - Objects.requireNonNull(sa, "sa"); - Objects.requireNonNull(da, "da"); int sl; if ((sp | dp | len) < 0 || // Halving the length of `sa` to obtain the number of characters: - sp >= (sl = sa.length >>> 1) || - dp >= da.length) { + sp >= (sl = sa.length >>> 1) || // Implicit null check on `sa` + dp >= da.length) { // Implicit null check on `da` return 0; } int minLen = Math.min(len, Math.min(sl - sp, da.length - dp)); @@ -184,9 +184,9 @@ private static int encodeISOArray0(byte[] sa, int sp, */ static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { - Objects.requireNonNull(sa, "sa"); - Objects.requireNonNull(da, "da"); - if ((sp | dp | len) < 0 || sp >= sa.length || dp >= da.length) { + if ((sp | dp | len) < 0 || + sp >= sa.length || // Implicit null check on `sa` + dp >= da.length) { // Implicit null check on `da` return 0; } int minLen = Math.min(len, Math.min(sa.length - sp, da.length - dp)); diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 28dab390271ea..62e09b1732955 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -156,9 +156,9 @@ public boolean isLegalReplacement(byte[] repl) { */ private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { - Objects.requireNonNull(sa, "sa"); - Objects.requireNonNull(da, "da"); - if ((sp | dp | len) < 0 || sp >= sa.length || dp >= da.length) { + if ((sp | dp | len) < 0 || + sp >= sa.length || // Implicit null check on `sa` + dp >= da.length) { // Implicit null check on `da` return 0; } int minLen = Math.min(len, Math.min(sa.length - sp, da.length - dp)); From 4d2a7a39c50ff850df31990dd4f7c14c8eeda0c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 11 Aug 2025 15:28:00 +0200 Subject: [PATCH 28/32] Move `->asRegion()` after `if (halt)` --- src/hotspot/share/opto/library_call.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 9b3920a0e3d3b..852a513df3f99 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -961,14 +961,14 @@ void LibraryCallKit::generate_string_range_check(Node* array, generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { - bailout = _gvn.transform(bailout)->as_Region(); if (halt) { + bailout = _gvn.transform(bailout)->as_Region(); Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic")); C->root()->add_req(halt); } else { PreserveJVMState pjvms(this); - set_control(bailout); + set_control(_gvn.transform(bailout)); uncommon_trap(Deoptimization::Reason_intrinsic, Deoptimization::Action_maybe_recompile); } From 9b721bb9feea9962a99483a0d66389e1a3014ca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Mon, 11 Aug 2025 15:29:51 +0200 Subject: [PATCH 29/32] Rename `generate_string_range_check::halt` to `halt_on_oob` --- src/hotspot/share/opto/library_call.cpp | 4 ++-- src/hotspot/share/opto/library_call.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 852a513df3f99..4e4b0478b8f12 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -943,7 +943,7 @@ void LibraryCallKit::generate_string_range_check(Node* array, Node* offset, Node* count, bool char_count, - bool halt) { + bool halt_on_oob) { if (stopped()) { return; // already stopped } @@ -961,7 +961,7 @@ void LibraryCallKit::generate_string_range_check(Node* array, generate_limit_guard(offset, count, load_array_length(array), bailout); if (bailout->req() > 1) { - if (halt) { + if (halt_on_oob) { bailout = _gvn.transform(bailout)->as_Region(); Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr)); Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic")); diff --git a/src/hotspot/share/opto/library_call.hpp b/src/hotspot/share/opto/library_call.hpp index 627c23ff5c4fc..fbac1363dae3d 100644 --- a/src/hotspot/share/opto/library_call.hpp +++ b/src/hotspot/share/opto/library_call.hpp @@ -164,7 +164,7 @@ class LibraryCallKit : public GraphKit { RegionNode* region); void generate_string_range_check(Node* array, Node* offset, Node* length, bool char_count, - bool halt = false); + bool halt_on_oob = false); Node* current_thread_helper(Node* &tls_output, ByteSize handle_offset, bool is_immutable); Node* generate_current_thread(Node* &tls_output); From d5aabf0c62a0a45ca65e4e5287ce9c5d0d0e2e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Tue, 12 Aug 2025 10:12:25 +0200 Subject: [PATCH 30/32] Remove `@apiNote` in `encodeISOArray()` Javadoc Those who are touching to these methods should well be aware of the details elaborated in the `@apiNote`, no need to put it on a display. --- .../share/classes/java/lang/StringCoding.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index 616b8cd5b4e72..ccf2ac9d4c10f 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -120,21 +120,6 @@ private static int countPositives0(byte[] ba, int off, int len) { * array containing characters encoded in UTF-16, into the destination byte * array, assuming that the encoding is ISO-8859-1 compatible. * - * @apiNote - * - * {@code sa} denotes the {@code byte[]} backing a {@link String}. When - * {@linkplain String#COMPACT_STRINGS compact strings} are disabled, a - * {@code char} is always represented by 2 bytes, i.e., - * encoded in UTF-16. When enabled, if the content is ISO-8859-1, a - * {@code char} is represented by 1 byte; otherwise again by 2 bytes. - *

- * This method assumes that {@code sa} is encoded in UTF-16, and hence, - * each {@code char} maps to 2 bytes. - *

- * {@code da} is encoded in ISO-8859-1. There each {@code byte} corresponds - * to a {@code char}. - *

- * * @param sa the source byte array containing characters encoded in UTF-16 * @param sp the index of the character (not byte!) from the source array to start reading from * @param da the target byte array From 6235034727c87f9b3b50b4f0cfd1e0fed4494fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 15 Aug 2025 09:36:37 +0200 Subject: [PATCH 31/32] Document why `Preconditions` is not used --- src/java.base/share/classes/java/lang/StringCoding.java | 4 ++++ src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index ccf2ac9d4c10f..545f216b75535 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -130,6 +130,8 @@ private static int countPositives0(byte[] ba, int off, int len) { */ static int encodeISOArray(byte[] sa, int sp, byte[] da, int dp, int len) { + // This method should tolerate invalid arguments, matching the lenient behavior of the VM intrinsic. + // Hence, using operator expressions instead of `Preconditions`, which throw on failure. int sl; if ((sp | dp | len) < 0 || // Halving the length of `sa` to obtain the number of characters: @@ -169,6 +171,8 @@ private static int encodeISOArray0(byte[] sa, int sp, */ static int encodeAsciiArray(char[] sa, int sp, byte[] da, int dp, int len) { + // This method should tolerate invalid arguments, matching the lenient behavior of the VM intrinsic. + // Hence, using operator expressions instead of `Preconditions`, which throw on failure. if ((sp | dp | len) < 0 || sp >= sa.length || // Implicit null check on `sa` dp >= da.length) { // Implicit null check on `da` diff --git a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java index 62e09b1732955..ff5970e92a896 100644 --- a/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java +++ b/src/java.base/share/classes/sun/nio/cs/ISO_8859_1.java @@ -156,6 +156,8 @@ public boolean isLegalReplacement(byte[] repl) { */ private static int encodeISOArray(char[] sa, int sp, byte[] da, int dp, int len) { + // This method should tolerate invalid arguments, matching the lenient behavior of the VM intrinsic. + // Hence, using operator expressions instead of `Preconditions`, which throw on failure. if ((sp | dp | len) < 0 || sp >= sa.length || // Implicit null check on `sa` dp >= da.length) { // Implicit null check on `da` From 2ba4ba6f45526fddfaed8ce276a47f3f378a9e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 15 Aug 2025 09:54:38 +0200 Subject: [PATCH 32/32] Document tests using `-XX:+VerifyIntrinsicChecks` don't check out-of-range conditions --- .../compiler/intrinsics/string/TestCountPositives.java | 5 ++++- .../compiler/intrinsics/string/TestEncodeIntrinsics.java | 7 +++++-- .../jtreg/compiler/intrinsics/string/TestHasNegatives.java | 5 ++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java index b684bc6776706..1c20a49d2817f 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestCountPositives.java @@ -52,8 +52,11 @@ * @test * @bug 8281146 * @summary Verify `StringCoding::countPositives` intrinsic Java wrapper checks - * by enabling the ones in the compiler intrinsic using + * by enabling the ones in the VM intrinsic using * `-XX:+VerifyIntrinsicChecks` + * @comment This does not check out-of-range conditions. The + * `-XX:+VerifyIntrinsicChecks` version of this test simply ensures + * that the VM intrinsic will produce no spurious errors. * @key randomness * @library /compiler/patches * @library /test/lib diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java index 1968405008501..bb343b246ffdb 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestEncodeIntrinsics.java @@ -35,8 +35,11 @@ * @test * @bug 6896617 8274242 * @summary Verify `sun.nio.cs.ISO_8859_1.Encoder::encodeISOArray` intrinsic - * Java wrapper checks by enabling the ones in the compiler intrinsic - * using `-XX:+VerifyIntrinsicChecks` + * Java wrapper checks by enabling the ones in the VM intrinsic using + * `-XX:+VerifyIntrinsicChecks` + * @comment This does not check out-of-range conditions. The + * `-XX:+VerifyIntrinsicChecks` version of this test simply ensures + * that the VM intrinsic will produce no spurious errors. * @key randomness * @library /test/lib * @comment `vm.debug == true` is required since `VerifyIntrinsicChecks` is a diff --git a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java index 8f1f06e1ec750..a15f6aade2e78 100644 --- a/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java +++ b/test/hotspot/jtreg/compiler/intrinsics/string/TestHasNegatives.java @@ -51,8 +51,11 @@ * @test * @bug 8054307 * @summary Verify `StringCoding::hasNegatives` intrinsic Java wrapper checks - * by enabling the ones in the compiler intrinsic using + * by enabling the ones in the VM intrinsic using * `-XX:+VerifyIntrinsicChecks` + * @comment This does not check out-of-range conditions. The + * `-XX:+VerifyIntrinsicChecks` version of this test simply ensures + * that the VM intrinsic will produce no spurious errors. * @key randomness * @library /compiler/patches * @library /test/lib