8204322: "+=" applied to String operands can provoke side effects
authorvromero
Wed, 06 Jun 2018 08:32:08 -0700
changeset 50430 6659a8f57d78
parent 50429 83aec1d357d4
child 50431 64e4b1686141
8204322: "+=" applied to String operands can provoke side effects Reviewed-by: mcimadamore, jlahoda, shade
src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/StringConcat.java
test/jdk/java/lang/String/concat/ImplicitStringConcatAssignLHS.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<JCTree> 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<JCTree> 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<JCTree> args, Type type, JCDiagnostic.DiagnosticPosition pos);
+        protected abstract void emit(JCDiagnostic.DiagnosticPosition pos, List<JCTree> args, boolean generateFirstArg, Type type);
 
         /** Peel the argument list into smaller chunks. */
         protected List<List<JCTree>> split(List<JCTree> args) {
@@ -318,9 +320,10 @@
         }
 
         /** Emit the indy concat for all these arguments, possibly peeling along the way */
-        protected void emit(List<JCTree> args, Type type, JCDiagnostic.DiagnosticPosition pos) {
+        protected void emit(JCDiagnostic.DiagnosticPosition pos, List<JCTree> args, boolean generateFirstArg, Type type) {
             List<List<JCTree>> split = split(args);
 
+            boolean first = true;
             for (List<JCTree> 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<JCTree> args, Type type, JCDiagnostic.DiagnosticPosition pos) {
+        protected void emit(JCDiagnostic.DiagnosticPosition pos, List<JCTree> args, boolean generateFirstArg, Type type) {
             List<List<JCTree>> split = split(args);
 
+            boolean first = true;
             for (List<JCTree> 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;
                     }
                 }
 
--- /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];
+    }
+
+}
+