Skip to content

Commit 655dc51

Browse files
vyvy
committed
8361842: Move input validation checks to Java for java.lang.StringCoding intrinsics
Reviewed-by: rriggs, liach, dfenacci, thartmann, redestad, jrose
1 parent f2f7a49 commit 655dc51

File tree

23 files changed

+414
-108
lines changed

23 files changed

+414
-108
lines changed

‎src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6421,10 +6421,14 @@ void MacroAssembler::fill_words(Register base, Register cnt, Register value)
64216421

64226422
// Intrinsic for
64236423
//
6424-
// - sun/nio/cs/ISO_8859_1$Encoder.implEncodeISOArray
6425-
// return the number of characters copied.
6426-
// - java/lang/StringUTF16.compress
6427-
// return index of non-latin1 character if copy fails, otherwise 'len'.
6424+
// - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
6425+
// Encodes char[] to byte[] in ISO-8859-1
6426+
//
6427+
// - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
6428+
// Encodes byte[] (containing UTF-16) to byte[] in ISO-8859-1
6429+
//
6430+
// - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len)
6431+
// Encodes char[] to byte[] in ASCII
64286432
//
64296433
// This version always returns the number of characters copied, and does not
64306434
// clobber the 'len' register. A successful copy will complete with the post-

‎src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,10 +2593,14 @@ void C2_MacroAssembler::char_array_compress_v(Register src, Register dst, Regist
25932593

25942594
// Intrinsic for
25952595
//
2596-
// - sun/nio/cs/ISO_8859_1$Encoder.implEncodeISOArray
2597-
// return the number of characters copied.
2598-
// - java/lang/StringUTF16.compress
2599-
// return index of non-latin1 character if copy fails, otherwise 'len'.
2596+
// - sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
2597+
// Encodes char[] to byte[] in ISO-8859-1
2598+
//
2599+
// - java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
2600+
// Encodes byte[] (containing UTF-16) to byte[] in ISO-8859-1
2601+
//
2602+
// - java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len)
2603+
// Encodes char[] to byte[] in ASCII
26002604
//
26012605
// This version always returns the number of characters copied. A successful
26022606
// copy will complete with the post-condition: 'res' == 'len', while an

‎src/hotspot/cpu/x86/macroAssembler_x86.cpp‎

Lines changed: 40 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6027,32 +6027,46 @@ void MacroAssembler::evpbroadcast(BasicType type, XMMRegister dst, Register src,
60276027
}
60286028
}
60296029

6030-
// encode char[] to byte[] in ISO_8859_1 or ASCII
6031-
//@IntrinsicCandidate
6032-
//private static int implEncodeISOArray(byte[] sa, int sp,
6033-
//byte[] da, int dp, int len) {
6034-
// int i = 0;
6035-
// for (; i < len; i++) {
6036-
// char c = StringUTF16.getChar(sa, sp++);
6037-
// if (c > '\u00FF')
6038-
// break;
6039-
// da[dp++] = (byte)c;
6040-
// }
6041-
// return i;
6042-
//}
6043-
//
6044-
//@IntrinsicCandidate
6045-
//private static int implEncodeAsciiArray(char[] sa, int sp,
6046-
// byte[] da, int dp, int len) {
6047-
// int i = 0;
6048-
// for (; i < len; i++) {
6049-
// char c = sa[sp++];
6050-
// if (c >= '\u0080')
6051-
// break;
6052-
// da[dp++] = (byte)c;
6053-
// }
6054-
// return i;
6055-
//}
6030+
// Encode given char[]/byte[] to byte[] in ISO_8859_1 or ASCII
6031+
//
6032+
// @IntrinsicCandidate
6033+
// int sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(
6034+
// char[] sa, int sp, byte[] da, int dp, int len) {
6035+
// int i = 0;
6036+
// for (; i < len; i++) {
6037+
// char c = sa[sp++];
6038+
// if (c > '\u00FF')
6039+
// break;
6040+
// da[dp++] = (byte) c;
6041+
// }
6042+
// return i;
6043+
// }
6044+
//
6045+
// @IntrinsicCandidate
6046+
// int java.lang.StringCoding.encodeISOArray0(
6047+
// byte[] sa, int sp, byte[] da, int dp, int len) {
6048+
// int i = 0;
6049+
// for (; i < len; i++) {
6050+
// char c = StringUTF16.getChar(sa, sp++);
6051+
// if (c > '\u00FF')
6052+
// break;
6053+
// da[dp++] = (byte) c;
6054+
// }
6055+
// return i;
6056+
// }
6057+
//
6058+
// @IntrinsicCandidate
6059+
// int java.lang.StringCoding.encodeAsciiArray0(
6060+
// char[] sa, int sp, byte[] da, int dp, int len) {
6061+
// int i = 0;
6062+
// for (; i < len; i++) {
6063+
// char c = sa[sp++];
6064+
// if (c >= '\u0080')
6065+
// break;
6066+
// da[dp++] = (byte) c;
6067+
// }
6068+
// return i;
6069+
// }
60566070
void MacroAssembler::encode_iso_array(Register src, Register dst, Register len,
60576071
XMMRegister tmp1Reg, XMMRegister tmp2Reg,
60586072
XMMRegister tmp3Reg, XMMRegister tmp4Reg,

‎src/hotspot/share/classfile/vmIntrinsics.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -415,18 +415,18 @@ class methodHandle;
415415
\
416416
do_class(java_lang_StringCoding, "java/lang/StringCoding") \
417417
do_intrinsic(_countPositives, java_lang_StringCoding, countPositives_name, countPositives_signature, F_S) \
418-
do_name( countPositives_name, "countPositives") \
418+
do_name( countPositives_name, "countPositives0") \
419419
do_signature(countPositives_signature, "([BII)I") \
420420
\
421421
do_class(sun_nio_cs_iso8859_1_Encoder, "sun/nio/cs/ISO_8859_1$Encoder") \
422422
do_intrinsic(_encodeISOArray, sun_nio_cs_iso8859_1_Encoder, encodeISOArray_name, encodeISOArray_signature, F_S) \
423-
do_name( encodeISOArray_name, "implEncodeISOArray") \
423+
do_name( encodeISOArray_name, "encodeISOArray0") \
424424
do_signature(encodeISOArray_signature, "([CI[BII)I") \
425425
\
426426
do_intrinsic(_encodeByteISOArray, java_lang_StringCoding, encodeISOArray_name, indexOfI_signature, F_S) \
427427
\
428428
do_intrinsic(_encodeAsciiArray, java_lang_StringCoding, encodeAsciiArray_name, encodeISOArray_signature, F_S) \
429-
do_name( encodeAsciiArray_name, "implEncodeAsciiArray") \
429+
do_name( encodeAsciiArray_name, "encodeAsciiArray0") \
430430
\
431431
do_class(java_math_BigInteger, "java/math/BigInteger") \
432432
do_intrinsic(_multiplyToLen, java_math_BigInteger, multiplyToLen_name, multiplyToLen_signature, F_S) \

‎src/hotspot/share/opto/c2_globals.hpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,9 @@
666666
product(bool, PrintIntrinsics, false, DIAGNOSTIC, \
667667
"prints attempted and successful inlining of intrinsics") \
668668
\
669+
develop(bool, VerifyIntrinsicChecks, false, \
670+
"Verify in intrinsic that Java level checks work as expected") \
671+
\
669672
develop(bool, StressReflectiveCode, false, \
670673
"Use inexact types at allocations, etc., to test reflection") \
671674
\

‎src/hotspot/share/opto/library_call.cpp‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,11 @@ inline Node* LibraryCallKit::generate_limit_guard(Node* offset,
939939
}
940940

941941
// Emit range checks for the given String.value byte array
942-
void LibraryCallKit::generate_string_range_check(Node* array, Node* offset, Node* count, bool char_count) {
942+
void LibraryCallKit::generate_string_range_check(Node* array,
943+
Node* offset,
944+
Node* count,
945+
bool char_count,
946+
bool halt_on_oob) {
943947
if (stopped()) {
944948
return; // already stopped
945949
}
@@ -957,10 +961,17 @@ void LibraryCallKit::generate_string_range_check(Node* array, Node* offset, Node
957961
generate_limit_guard(offset, count, load_array_length(array), bailout);
958962

959963
if (bailout->req() > 1) {
960-
PreserveJVMState pjvms(this);
961-
set_control(_gvn.transform(bailout));
962-
uncommon_trap(Deoptimization::Reason_intrinsic,
963-
Deoptimization::Action_maybe_recompile);
964+
if (halt_on_oob) {
965+
bailout = _gvn.transform(bailout)->as_Region();
966+
Node* frame = _gvn.transform(new ParmNode(C->start(), TypeFunc::FramePtr));
967+
Node* halt = _gvn.transform(new HaltNode(bailout, frame, "unexpected guard failure in intrinsic"));
968+
C->root()->add_req(halt);
969+
} else {
970+
PreserveJVMState pjvms(this);
971+
set_control(_gvn.transform(bailout));
972+
uncommon_trap(Deoptimization::Reason_intrinsic,
973+
Deoptimization::Action_maybe_recompile);
974+
}
964975
}
965976
}
966977

@@ -1118,6 +1129,7 @@ bool LibraryCallKit::inline_array_equals(StrIntrinsicNode::ArgEnc ae) {
11181129

11191130

11201131
//------------------------------inline_countPositives------------------------------
1132+
// int java.lang.StringCoding#countPositives0(byte[] ba, int off, int len)
11211133
bool LibraryCallKit::inline_countPositives() {
11221134
if (too_many_traps(Deoptimization::Reason_intrinsic)) {
11231135
return false;
@@ -1129,13 +1141,14 @@ bool LibraryCallKit::inline_countPositives() {
11291141
Node* offset = argument(1);
11301142
Node* len = argument(2);
11311143

1132-
ba = must_be_not_null(ba, true);
1133-
1134-
// Range checks
1135-
generate_string_range_check(ba, offset, len, false);
1136-
if (stopped()) {
1137-
return true;
1144+
if (VerifyIntrinsicChecks) {
1145+
ba = must_be_not_null(ba, true);
1146+
generate_string_range_check(ba, offset, len, false, true);
1147+
if (stopped()) {
1148+
return true;
1149+
}
11381150
}
1151+
11391152
Node* ba_start = array_element_address(ba, offset, T_BYTE);
11401153
Node* result = new CountPositivesNode(control(), memory(TypeAryPtr::BYTES), ba_start, len);
11411154
set_result(_gvn.transform(result));
@@ -6128,6 +6141,9 @@ CallStaticJavaNode* LibraryCallKit::get_uncommon_trap_from_success_proj(Node* no
61286141
}
61296142

61306143
//-------------inline_encodeISOArray-----------------------------------
6144+
// int sun.nio.cs.ISO_8859_1.Encoder#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
6145+
// int java.lang.StringCoding#encodeISOArray0(byte[] sa, int sp, byte[] da, int dp, int len)
6146+
// int java.lang.StringCoding#encodeAsciiArray0(char[] sa, int sp, byte[] da, int dp, int len)
61316147
// encode char[] to byte[] in ISO_8859_1 or ASCII
61326148
bool LibraryCallKit::inline_encodeISOArray(bool ascii) {
61336149
assert(callee()->signature()->size() == 5, "encodeISOArray has 5 parameters");
@@ -6138,8 +6154,14 @@ bool LibraryCallKit::inline_encodeISOArray(bool ascii) {
61386154
Node *dst_offset = argument(3);
61396155
Node *length = argument(4);
61406156

6141-
src = must_be_not_null(src, true);
6142-
dst = must_be_not_null(dst, true);
6157+
// Cast source & target arrays to not-null
6158+
if (VerifyIntrinsicChecks) {
6159+
src = must_be_not_null(src, true);
6160+
dst = must_be_not_null(dst, true);
6161+
if (stopped()) {
6162+
return true;
6163+
}
6164+
}
61436165

61446166
const TypeAryPtr* src_type = src->Value(&_gvn)->isa_aryptr();
61456167
const TypeAryPtr* dst_type = dst->Value(&_gvn)->isa_aryptr();
@@ -6156,6 +6178,15 @@ bool LibraryCallKit::inline_encodeISOArray(bool ascii) {
61566178
return false;
61576179
}
61586180

6181+
// Check source & target bounds
6182+
if (VerifyIntrinsicChecks) {
6183+
generate_string_range_check(src, src_offset, length, src_elem == T_BYTE, true);
6184+
generate_string_range_check(dst, dst_offset, length, false, true);
6185+
if (stopped()) {
6186+
return true;
6187+
}
6188+
}
6189+
61596190
Node* src_start = array_element_address(src, src_offset, T_CHAR);
61606191
Node* dst_start = array_element_address(dst, dst_offset, dst_elem);
61616192
// 'src_start' points to src array + scaled offset

‎src/hotspot/share/opto/library_call.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ class LibraryCallKit : public GraphKit {
163163
Node* array_length,
164164
RegionNode* region);
165165
void generate_string_range_check(Node* array, Node* offset,
166-
Node* length, bool char_count);
166+
Node* length, bool char_count,
167+
bool halt_on_oob = false);
167168
Node* current_thread_helper(Node* &tls_output, ByteSize handle_offset,
168169
bool is_immutable);
169170
Node* generate_current_thread(Node* &tls_output);

‎src/java.base/share/classes/java/lang/String.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ private static byte[] encode8859_1(byte coder, byte[] val, boolean doReplace) {
10191019
int sp = 0;
10201020
int sl = len;
10211021
while (sp < sl) {
1022-
int ret = StringCoding.implEncodeISOArray(val, sp, dst, dp, len);
1022+
int ret = StringCoding.encodeISOArray(val, sp, dst, dp, len);
10231023
sp = sp + ret;
10241024
dp = dp + ret;
10251025
if (ret != len) {

0 commit comments

Comments
 (0)