Tue, 03 May 2011 09:10:39 -0700
7041100: The load in String.equals intrinsic executed before null check
Summary: Remove control from loads in String.equals intrinsic and cast argument to not-null.
Reviewed-by: never
src/share/vm/opto/library_call.cpp | file | annotate | diff | comparison | revisions | |
test/compiler/7041100/Test7041100.java | file | annotate | diff | comparison | revisions |
1.1 --- a/src/share/vm/opto/library_call.cpp Mon May 02 18:53:37 2011 -0700 1.2 +++ b/src/share/vm/opto/library_call.cpp Tue May 03 09:10:39 2011 -0700 1.3 @@ -867,12 +867,10 @@ 1.4 Node* str1_offset = make_load(no_ctrl, str1_offseta, TypeInt::INT, T_INT, string_type->add_offset(offset_offset)); 1.5 Node* str1_start = array_element_address(str1_value, str1_offset, T_CHAR); 1.6 1.7 - // Pin loads from String::equals() argument since it could be NULL. 1.8 - Node* str2_ctrl = (opcode == Op_StrEquals) ? control() : no_ctrl; 1.9 Node* str2_valuea = basic_plus_adr(str2, str2, value_offset); 1.10 - Node* str2_value = make_load(str2_ctrl, str2_valuea, value_type, T_OBJECT, string_type->add_offset(value_offset)); 1.11 + Node* str2_value = make_load(no_ctrl, str2_valuea, value_type, T_OBJECT, string_type->add_offset(value_offset)); 1.12 Node* str2_offseta = basic_plus_adr(str2, str2, offset_offset); 1.13 - Node* str2_offset = make_load(str2_ctrl, str2_offseta, TypeInt::INT, T_INT, string_type->add_offset(offset_offset)); 1.14 + Node* str2_offset = make_load(no_ctrl, str2_offseta, TypeInt::INT, T_INT, string_type->add_offset(offset_offset)); 1.15 Node* str2_start = array_element_address(str2_value, str2_offset, T_CHAR); 1.16 1.17 Node* result = NULL; 1.18 @@ -1012,14 +1010,15 @@ 1.19 if (!stopped()) { 1.20 // Properly cast the argument to String 1.21 argument = _gvn.transform(new (C, 2) CheckCastPPNode(control(), argument, string_type)); 1.22 + // This path is taken only when argument's type is String:NotNull. 1.23 + argument = cast_not_null(argument, false); 1.24 1.25 // Get counts for string and argument 1.26 Node* receiver_cnta = basic_plus_adr(receiver, receiver, count_offset); 1.27 receiver_cnt = make_load(no_ctrl, receiver_cnta, TypeInt::INT, T_INT, string_type->add_offset(count_offset)); 1.28 1.29 - // Pin load from argument string since it could be NULL. 1.30 Node* argument_cnta = basic_plus_adr(argument, argument, count_offset); 1.31 - argument_cnt = make_load(control(), argument_cnta, TypeInt::INT, T_INT, string_type->add_offset(count_offset)); 1.32 + argument_cnt = make_load(no_ctrl, argument_cnta, TypeInt::INT, T_INT, string_type->add_offset(count_offset)); 1.33 1.34 // Check for receiver count != argument count 1.35 Node* cmp = _gvn.transform( new(C, 3) CmpINode(receiver_cnt, argument_cnt) );
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/compiler/7041100/Test7041100.java Tue May 03 09:10:39 2011 -0700 2.3 @@ -0,0 +1,53 @@ 2.4 +/* 2.5 + * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + * 2.26 + */ 2.27 + 2.28 +/** 2.29 + * @test 2.30 + * @bug 7041100 2.31 + * @summary The load in String.equals intrinsic executed before null check 2.32 + * 2.33 + * @run main/othervm -Xbatch Test7041100 abc def 2.34 + */ 2.35 + 2.36 +public class Test7041100 { 2.37 + 2.38 + static String n = null; 2.39 + public static void main(String[] args) throws Exception { 2.40 + for (int i = 0; i < 10000; i++) { 2.41 + stringEQ(args[0], args[1]); 2.42 + stringEQ(args[0], args[0]); 2.43 + stringEQ(args[0], n); 2.44 + stringEQ(n, args[0]); 2.45 + } 2.46 + } 2.47 + 2.48 + public static boolean stringEQ(String a, String b) { 2.49 + if (a == b) 2.50 + return true; 2.51 + if (a == null || b == null) 2.52 + return false; 2.53 + else 2.54 + return a.equals(b); 2.55 + } 2.56 +}