8186500: StringConcatFactory.makeConcatWithConstants throws AssertionError when recipe contains non-String constants
authorredestad
Thu, 24 Aug 2017 15:03:38 +0200
changeset 46902 c6cefe631b18
parent 46901 f63587417ae6
child 46903 eb4178c9feb1
8186500: StringConcatFactory.makeConcatWithConstants throws AssertionError when recipe contains non-String constants Reviewed-by: shade, psandoz
jdk/src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java
jdk/test/java/lang/String/concat/StringConcatFactoryInvariants.java
--- a/jdk/src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java	Wed Aug 23 21:27:02 2017 -0700
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java	Thu Aug 24 15:03:38 2017 +0200
@@ -25,19 +25,24 @@
 
 package java.lang.invoke;
 
+import jdk.internal.misc.Unsafe;
 import jdk.internal.org.objectweb.asm.ClassWriter;
 import jdk.internal.org.objectweb.asm.Label;
 import jdk.internal.org.objectweb.asm.MethodVisitor;
 import jdk.internal.org.objectweb.asm.Opcodes;
 import jdk.internal.vm.annotation.ForceInline;
-import jdk.internal.misc.Unsafe;
+import sun.invoke.util.Wrapper;
+import sun.security.action.GetPropertyAction;
 
 import java.lang.invoke.MethodHandles.Lookup;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Function;
-import sun.security.action.GetPropertyAction;
 
 import static jdk.internal.org.objectweb.asm.Opcodes.*;
 
@@ -319,12 +324,12 @@
     }
 
     private static final class RecipeElement {
-        private final Object value;
+        private final String value;
         private final int argPos;
         private final char tag;
 
         public RecipeElement(Object cnst) {
-            this.value = Objects.requireNonNull(cnst);
+            this.value = String.valueOf(Objects.requireNonNull(cnst));
             this.argPos = -1;
             this.tag = TAG_CONST;
         }
@@ -335,7 +340,7 @@
             this.tag = TAG_ARG;
         }
 
-        public Object getValue() {
+        public String getValue() {
             assert (tag == TAG_CONST);
             return value;
         }
@@ -923,8 +928,7 @@
                 for (RecipeElement el : recipe.getElements()) {
                     switch (el.getTag()) {
                         case TAG_CONST:
-                            Object cnst = el.getValue();
-                            len += cnst.toString().length();
+                            len += el.getValue().length();
                             break;
                         case TAG_ARG:
                             /*
@@ -983,9 +987,8 @@
                     String desc;
                     switch (el.getTag()) {
                         case TAG_CONST:
-                            Object cnst = el.getValue();
-                            mv.visitLdcInsn(cnst);
-                            desc = getSBAppendDesc(cnst.getClass());
+                            mv.visitLdcInsn(el.getValue());
+                            desc = getSBAppendDesc(String.class);
                             break;
                         case TAG_ARG:
                             Class<?> cl = arr[el.getArgPos()];
@@ -1273,8 +1276,7 @@
             for (RecipeElement el : recipe.getElements()) {
                 switch (el.getTag()) {
                     case TAG_CONST:
-                        Object cnst = el.getValue();
-                        initial += cnst.toString().length();
+                        initial += el.getValue().length();
                         break;
                     case TAG_ARG:
                         final int i = el.getArgPos();
@@ -1303,9 +1305,8 @@
                 MethodHandle appender;
                 switch (el.getTag()) {
                     case TAG_CONST:
-                        Object constant = el.getValue();
-                        MethodHandle mh = appender(adaptToStringBuilder(constant.getClass()));
-                        appender = MethodHandles.insertArguments(mh, 1, constant);
+                        MethodHandle mh = appender(adaptToStringBuilder(String.class));
+                        appender = MethodHandles.insertArguments(mh, 1, el.getValue());
                         break;
                     case TAG_ARG:
                         int ac = el.getArgPos();
@@ -1506,8 +1507,7 @@
                 mh = MethodHandles.dropArguments(mh, 2, int.class);
                 switch (el.getTag()) {
                     case TAG_CONST: {
-                        Object cnst = el.getValue();
-                        MethodHandle prepender = MethodHandles.insertArguments(prepender(cnst.getClass()), 3, cnst);
+                        MethodHandle prepender = MethodHandles.insertArguments(prepender(String.class), 3, el.getValue());
                         mh = MethodHandles.foldArguments(mh, 1, prepender,
                                 2, 0, 3 // index, storage, coder
                         );
@@ -1550,10 +1550,9 @@
             for (RecipeElement el : recipe.getElements()) {
                 switch (el.getTag()) {
                     case TAG_CONST:
-                        Object constant = el.getValue();
-                        String s = constant.toString();
-                        initialCoder = (byte) coderMixer(String.class).invoke(initialCoder, s);
-                        initialLen += s.length();
+                        String constant = el.getValue();
+                        initialCoder = (byte) coderMixer(String.class).invoke(initialCoder, constant);
+                        initialLen += constant.length();
                         break;
                     case TAG_ARG:
                         int ac = el.getArgPos();
@@ -1621,7 +1620,8 @@
         private static final Function<Class<?>, MethodHandle> PREPEND = new Function<Class<?>, MethodHandle>() {
             @Override
             public MethodHandle apply(Class<?> c) {
-                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "prepend", int.class, int.class, byte[].class, byte.class, c);
+                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "prepend", int.class, int.class, byte[].class, byte.class,
+                        Wrapper.asPrimitiveType(c));
             }
         };
 
@@ -1629,7 +1629,8 @@
         private static final Function<Class<?>, MethodHandle> CODER_MIX = new Function<Class<?>, MethodHandle>() {
             @Override
             public MethodHandle apply(Class<?> c) {
-                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "mixCoder", byte.class, byte.class, c);
+                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "mixCoder", byte.class, byte.class,
+                        Wrapper.asPrimitiveType(c));
             }
         };
 
@@ -1637,7 +1638,8 @@
         private static final Function<Class<?>, MethodHandle> LENGTH_MIX = new Function<Class<?>, MethodHandle>() {
             @Override
             public MethodHandle apply(Class<?> c) {
-                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "mixLen", int.class, int.class, c);
+                return lookupStatic(Lookup.IMPL_LOOKUP, STRING_HELPER, "mixLen", int.class, int.class,
+                        Wrapper.asPrimitiveType(c));
             }
         };
 
--- a/jdk/test/java/lang/String/concat/StringConcatFactoryInvariants.java	Wed Aug 23 21:27:02 2017 -0700
+++ b/jdk/test/java/lang/String/concat/StringConcatFactoryInvariants.java	Thu Aug 24 15:03:38 2017 +0200
@@ -70,7 +70,33 @@
         String methodName = "foo";
         MethodType mt = MethodType.methodType(String.class, String.class, int.class);
         String recipe = "" + TAG_ARG + TAG_ARG + TAG_CONST;
-        String[] constants = new String[]{"bar"};
+        Object[][] constants = new Object[][] {
+                new String[] { "bar" },
+                new Integer[] { 1 },
+                new Short[] { 2 },
+                new Long[] { 3L },
+                new Boolean[] { true },
+                new Character[] { 'a' },
+                new Byte[] { -128 },
+                new Class[] { String.class },
+                new MethodHandle[] { MethodHandles.constant(String.class, "constant") },
+                new MethodType[] { MethodType.methodType(String.class) }
+        };
+        // The string representation that should end up if the corresponding
+        // Object[] in constants is used as an argument to makeConcatWithConstants
+        String[] constantString = new String[] {
+                "bar",
+                "1",
+                "2",
+                "3",
+                "true",
+                "a",
+                "-128",
+                "class java.lang.String",
+                "MethodHandle()String",
+                "()String"
+        };
+
 
         final int LIMIT = 200;
 
@@ -109,8 +135,10 @@
         }
 
         {
-            CallSite cs = StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, recipe, constants);
-            test("foo42bar", (String) cs.getTarget().invokeExact("foo", 42));
+            for (int i = 0; i < constants.length; i++) {
+                CallSite cs = StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, recipe, constants[i]);
+                test("foo42".concat(constantString[i]), (String) cs.getTarget().invokeExact("foo", 42));
+            }
         }
 
         // Simple factory, check for nulls:
@@ -124,20 +152,27 @@
                 () -> StringConcatFactory.makeConcat(lookup, methodName, null));
 
         // Advanced factory, check for nulls:
-        failNPE("Lookup is null",
-                () -> StringConcatFactory.makeConcatWithConstants(null, methodName, mt, recipe, constants));
+        for (int i = 0; i < constants.length; i++) {
+            final Object[] consts = constants[i];
 
-        failNPE("Method name is null",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, null, mt, recipe, constants));
+            failNPE("Lookup is null",
+                    () -> StringConcatFactory.makeConcatWithConstants(null, methodName, mt, recipe, consts));
 
-        failNPE("MethodType is null",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, null, recipe, constants));
+            failNPE("Method name is null",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, null, mt, recipe, consts));
 
-        failNPE("Recipe is null",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, null, constants));
+            failNPE("MethodType is null",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, null, recipe, consts));
+
+            failNPE("Recipe is null",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, null, consts));
+        }
 
         failNPE("Constants vararg is null",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, recipe, null));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, recipe, (Object[]) null));
+
+        failNPE("Constant argument is null",
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mt, recipe, new Object[] { null }));
 
         // Simple factory, check for return type
         fail("Return type: void",
@@ -159,23 +194,26 @@
                 () -> StringConcatFactory.makeConcat(lookup, methodName, MethodType.methodType(Serializable.class, String.class, int.class)));
 
         // Advanced factory, check for return types
-        fail("Return type: void",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(void.class, String.class, int.class), recipe, constants));
+        for (int i = 0; i < constants.length; i++) {
+            final Object[] consts = constants[i];
+            fail("Return type: void",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(void.class, String.class, int.class), recipe, consts));
 
-        fail("Return type: int",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(int.class, String.class, int.class), recipe, constants));
-
-        fail("Return type: StringBuilder",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(StringBuilder.class, String.class, int.class), recipe, constants));
+            fail("Return type: int",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(int.class, String.class, int.class), recipe, consts));
 
-        ok("Return type: Object",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(Object.class, String.class, int.class), recipe, constants));
+            fail("Return type: StringBuilder",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(StringBuilder.class, String.class, int.class), recipe, consts));
+
+            ok("Return type: Object",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(Object.class, String.class, int.class), recipe, consts));
 
-        ok("Return type: CharSequence",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(CharSequence.class, String.class, int.class), recipe, constants));
+            ok("Return type: CharSequence",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(CharSequence.class, String.class, int.class), recipe, consts));
 
-        ok("Return type: Serializable",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(Serializable.class, String.class, int.class), recipe, constants));
+            ok("Return type: Serializable",
+                    () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(Serializable.class, String.class, int.class), recipe, consts));
+        }
 
         // Simple factory: check for dynamic arguments overflow
         ok("Dynamic arguments is under limit",
@@ -189,13 +227,13 @@
 
         // Advanced factory: check for dynamic arguments overflow
         ok("Dynamic arguments is under limit",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtUnderThreshold, recipeUnderThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtUnderThreshold, recipeUnderThreshold, constants[0]));
 
         ok("Dynamic arguments is at the limit",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeThreshold, constants[0]));
 
         fail("Dynamic arguments is over the limit",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtOverThreshold, recipeOverThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtOverThreshold, recipeOverThreshold, constants[0]));
 
         // Advanced factory: check for mismatched recipe and Constants
         ok("Static arguments and recipe match",
@@ -206,17 +244,17 @@
 
         // Advanced factory: check for mismatched recipe and dynamic arguments
         fail("Dynamic arguments and recipe mismatch",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeUnderThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeUnderThreshold, constants[0]));
 
         ok("Dynamic arguments and recipe match",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeThreshold, constants[0]));
 
         fail("Dynamic arguments and recipe mismatch",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeOverThreshold, constants));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, mtThreshold, recipeOverThreshold, constants[0]));
 
         // Test passing array as constant
         {
-            String[] arg = {"boo", "bar"};
+            Object[] arg = {"boo", "bar"};
 
             CallSite cs1 = StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(String.class, int.class), "" + TAG_ARG + TAG_CONST + TAG_CONST, arg);
             test("42boobar", (String) cs1.getTarget().invokeExact(42));
@@ -227,7 +265,7 @@
                 () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(String.class, int.class), "" + TAG_ARG + TAG_CONST, "foo"));
 
         failNPE("Cannot pass null constants",
-                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(String.class, int.class), "" + TAG_ARG + TAG_CONST, new String[]{null}));
+                () -> StringConcatFactory.makeConcatWithConstants(lookup, methodName, MethodType.methodType(String.class, int.class), "" + TAG_ARG + TAG_CONST, new Object[]{null}));
 
         // Simple factory: test empty arguments
         ok("Ok to pass empty arguments",