# HG changeset patch # User kvn # Date 1293586442 28800 # Node ID 2ddb2fab82cbb8da5654be6224b9a9e9d8f8a59c # Parent 037c727f35fbab4d4f98768a1b163c20a212e4a4 7009359: HS with -XX:+AggressiveOpts optimize new StringBuffer(null) so it does not throw NPE as expected Summary: Bailout StringConcat optimization if null is passed to StringBuffer constructor. Reviewed-by: iveresov diff -r 037c727f35fb -r 2ddb2fab82cb src/share/vm/opto/stringopts.cpp --- a/src/share/vm/opto/stringopts.cpp Mon Dec 27 21:51:31 2010 -0800 +++ b/src/share/vm/opto/stringopts.cpp Tue Dec 28 17:34:02 2010 -0800 @@ -59,7 +59,8 @@ enum { StringMode, IntMode, - CharMode + CharMode, + StringNullCheckMode }; StringConcat(PhaseStringOpts* stringopts, CallStaticJavaNode* end): @@ -114,6 +115,9 @@ void push_string(Node* value) { push(value, StringMode); } + void push_string_null_check(Node* value) { + push(value, StringNullCheckMode); + } void push_int(Node* value) { push(value, IntMode); } @@ -416,7 +420,19 @@ if (sig == ciSymbol::string_void_signature()) { // StringBuilder(String) so pick this up as the first argument assert(use->in(TypeFunc::Parms + 1) != NULL, "what?"); - sc->push_string(use->in(TypeFunc::Parms + 1)); + const Type* type = _gvn->type(use->in(TypeFunc::Parms + 1)); + if (type == TypePtr::NULL_PTR) { + // StringBuilder(null) throws exception. +#ifndef PRODUCT + if (PrintOptimizeStringConcat) { + tty->print("giving up because StringBuilder(null) throws exception"); + alloc->jvms()->dump_spec(tty); tty->cr(); + } +#endif + return NULL; + } + // StringBuilder(str) argument needs null check. + sc->push_string_null_check(use->in(TypeFunc::Parms + 1)); } // The int variant takes an initial size for the backing // array so just treat it like the void version. @@ -436,7 +452,7 @@ #ifndef PRODUCT if (PrintOptimizeStringConcat) { tty->print("giving up because couldn't find constructor "); - alloc->jvms()->dump_spec(tty); + alloc->jvms()->dump_spec(tty); tty->cr(); } #endif break; @@ -1269,6 +1285,25 @@ string_sizes->init_req(argi, string_size); break; } + case StringConcat::StringNullCheckMode: { + const Type* type = kit.gvn().type(arg); + assert(type != TypePtr::NULL_PTR, "missing check"); + if (!type->higher_equal(TypeInstPtr::NOTNULL)) { + // Null check with uncommont trap since + // StringBuilder(null) throws exception. + // Use special uncommon trap instead of + // calling normal do_null_check(). + Node* p = __ Bool(__ CmpP(arg, kit.null()), BoolTest::ne); + IfNode* iff = kit.create_and_map_if(kit.control(), p, PROB_MIN, COUNT_UNKNOWN); + overflow->add_req(__ IfFalse(iff)); + Node* notnull = __ IfTrue(iff); + kit.set_control(notnull); // set control for the cast_not_null + arg = kit.cast_not_null(arg, false); + sc->set_argument(argi, arg); + } + assert(kit.gvn().type(arg)->higher_equal(TypeInstPtr::NOTNULL), "sanity"); + // Fallthrough to add string length. + } case StringConcat::StringMode: { const Type* type = kit.gvn().type(arg); if (type == TypePtr::NULL_PTR) { @@ -1328,6 +1363,7 @@ // Hook PreserveJVMState pjvms(&kit); kit.set_control(overflow); + C->record_for_igvn(overflow); kit.uncommon_trap(Deoptimization::Reason_intrinsic, Deoptimization::Action_make_not_entrant); } @@ -1363,6 +1399,7 @@ start = end; break; } + case StringConcat::StringNullCheckMode: case StringConcat::StringMode: { start = copy_string(kit, arg, char_array, start); break; diff -r 037c727f35fb -r 2ddb2fab82cb test/compiler/7009359/Test7009359.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/compiler/7009359/Test7009359.java Tue Dec 28 17:34:02 2010 -0800 @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2010, 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 + * @bug 7009359 + * @summary HS with -XX:+AggressiveOpts optimize new StringBuffer(null) so it does not throw NPE as expected + * + * @run main/othervm -Xbatch -XX:+IgnoreUnrecognizedVMOptions -XX:+OptimizeStringConcat -XX:CompileCommand=exclude,Test7009359,main Test7009359 + * + */ + +public class Test7009359 { + public static void main (String[] args) { + for(int i = 0; i < 1000000; i++) { + if(!stringmakerBUG(null).equals("NPE")) { + System.out.println("StringBuffer(null) does not throw NPE"); + System.exit(97); + } + } + } + + public static String stringmakerBUG(String str) { + try { + return new StringBuffer(str).toString(); + } catch (NullPointerException e) { + return "NPE"; + } + } +} +