7041100: The load in String.equals intrinsic executed before null check

Tue, 03 May 2011 09:10:39 -0700

author
kvn
date
Tue, 03 May 2011 09:10:39 -0700
changeset 2869
e6d7eed3330c
parent 2868
2e038ad0c1d0
child 2871
8a9941687aae

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 +}

mercurial