# HG changeset patch # User vromero # Date 1528299128 25200 # Node ID 6659a8f57d781a8be196873e361661cc0f14ee1e # Parent 83aec1d357d4d0d6aa1675e505463734a98a1eed 8204322: "+=" applied to String operands can provoke side effects Reviewed-by: mcimadamore, jlahoda, shade diff -r 83aec1d357d4 -r 6659a8f57d78 src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/StringConcat.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/StringConcat.java Wed Jun 06 10:45:40 2018 -0400 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/StringConcat.java Wed Jun 06 08:32:08 2018 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2018, 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 @@ -261,18 +261,20 @@ public Item makeConcat(JCTree.JCAssignOp tree) { List args = collectAll(tree.lhs, tree.rhs); Item l = gen.genExpr(tree.lhs, tree.lhs.type); - emit(args, tree.type, tree.pos()); + l.duplicate(); + l.load(); + emit(tree.pos(), args, false, tree.type); return l; } @Override public Item makeConcat(JCTree.JCBinary tree) { List args = collectAll(tree.lhs, tree.rhs); - emit(args, tree.type, tree.pos()); + emit(tree.pos(), args, true, tree.type); return gen.getItems().makeStackItem(syms.stringType); } - protected abstract void emit(List args, Type type, JCDiagnostic.DiagnosticPosition pos); + protected abstract void emit(JCDiagnostic.DiagnosticPosition pos, List args, boolean generateFirstArg, Type type); /** Peel the argument list into smaller chunks. */ protected List> split(List args) { @@ -318,9 +320,10 @@ } /** Emit the indy concat for all these arguments, possibly peeling along the way */ - protected void emit(List args, Type type, JCDiagnostic.DiagnosticPosition pos) { + protected void emit(JCDiagnostic.DiagnosticPosition pos, List args, boolean generateFirstArg, Type type) { List> split = split(args); + boolean first = true; for (List t : split) { Assert.check(!t.isEmpty(), "Arguments list is empty"); @@ -333,9 +336,11 @@ } else { dynamicArgs.add(sharpestAccessible(arg.type)); } - gen.genExpr(arg, arg.type).load(); + if (!first || generateFirstArg) { + gen.genExpr(arg, arg.type).load(); + } + first = false; } - doCall(type, pos, dynamicArgs.toList()); } @@ -402,9 +407,10 @@ } @Override - protected void emit(List args, Type type, JCDiagnostic.DiagnosticPosition pos) { + protected void emit(JCDiagnostic.DiagnosticPosition pos, List args, boolean generateFirstArg, Type type) { List> split = split(args); + boolean first = true; for (List t : split) { Assert.check(!t.isEmpty(), "Arguments list is empty"); @@ -433,7 +439,10 @@ // Ordinary arguments come through the dynamic arguments. recipe.append(TAG_ARG); dynamicArgs.add(sharpestAccessible(arg.type)); - gen.genExpr(arg, arg.type).load(); + if (!first || generateFirstArg) { + gen.genExpr(arg, arg.type).load(); + } + first = false; } } diff -r 83aec1d357d4 -r 6659a8f57d78 test/jdk/java/lang/String/concat/ImplicitStringConcatAssignLHS.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/lang/String/concat/ImplicitStringConcatAssignLHS.java Wed Jun 06 08:32:08 2018 -0700 @@ -0,0 +1,232 @@ +/* + * Copyright (c) 2018, 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 "+=" applied to String operands can provoke side effects + * @bug 8204322 + * + * @compile ImplicitStringConcatAssignLHS.java + * @run main/othervm -Xverify:all ImplicitStringConcatAssignLHS + * + * @compile -XDstringConcat=inline ImplicitStringConcatAssignLHS.java + * @run main/othervm -Xverify:all ImplicitStringConcatAssignLHS + * + * @compile -XDstringConcat=indy ImplicitStringConcatAssignLHS.java + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT ImplicitStringConcatAssignLHS + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * + * @compile -XDstringConcat=indyWithConstants ImplicitStringConcatAssignLHS.java + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT ImplicitStringConcatAssignLHS + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true ImplicitStringConcatAssignLHS + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=BC_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_SB_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS + * @run main/othervm -Xverify:all -Djava.lang.invoke.stringConcat=MH_INLINE_SIZED_EXACT -Djava.lang.invoke.stringConcat.debug=true -Djava.lang.invoke.stringConcat.cache=true ImplicitStringConcatAssignLHS +*/ +import java.lang.StringBuilder; + +public class ImplicitStringConcatAssignLHS { + + static final int ARR_SIZE = 10; // enough padding to capture ill offsets + + static int x; + + public static void main(String[] args) throws Exception { + { + x = 0; + Object[] arr = new Object[ARR_SIZE]; + arr[x++] += "foo"; + check(1, "plain-plain Object[]"); + } + + { + x = 0; + getObjArray()[x++] += "foo"; + check(2, "method-plain Object[]"); + } + + { + x = 0; + getObjArray()[getIndex()] += "foo"; + check(2, "method-method Object[]"); + } + + { + x = 0; + String[] arr = new String[ARR_SIZE]; + arr[x++] += "foo"; + check(1, "plain-plain String[]"); + } + + { + x = 0; + getStringArray()[x++] += "foo"; + check(2, "method-plain String[]"); + } + + { + x = 0; + getStringArray()[getIndex()] += "foo"; + check(2, "method-method String[]"); + } + + { + x = 0; + CharSequence[] arr = new CharSequence[ARR_SIZE]; + arr[x++] += "foo"; + check(1, "plain-plain CharSequence[]"); + } + + { + x = 0; + getCharSequenceArray()[x++] += "foo"; + check(2, "method-plain CharSequence[]"); + } + + { + x = 0; + getCharSequenceArray()[getIndex()] += "foo"; + check(2, "method-method CharSequence[]"); + } + + { + x = 0; + new MyClass().s += "foo"; + check(1, "MyClass::new (String)"); + } + + { + x = 0; + getMyClass().s += "foo"; + check(1, "method MyClass::new (String)"); + } + + { + x = 0; + new MyClass().o += "foo"; + check(1, "MyClass::new (object)"); + } + + { + x = 0; + getMyClass().o += "foo"; + check(1, "method MyClass::new (object)"); + } + } + + public static void check(int expected, String label) { + if (x != expected) { + StringBuilder sb = new StringBuilder(); + sb.append(label); + sb.append(": "); + sb.append("Expected = "); + sb.append(expected); + sb.append("actual = "); + sb.append(x); + throw new IllegalStateException(sb.toString()); + } + } + + public static int getIndex() { + return x++; + } + + public static class MyClass { + Object o; + String s; + + public MyClass() { + x++; + } + } + + public static MyClass getMyClass() { + return new MyClass(); +} + + public static Object[] getObjArray() { + x++; + return new Object[ARR_SIZE]; + } + + public static String[] getStringArray() { + x++; + return new String[ARR_SIZE]; + } + + public static CharSequence[] getCharSequenceArray() { + x++; + return new String[ARR_SIZE]; + } + +} +