8150635: j.l.i.MethodHandles.loop(...) throws IndexOutOfBoundsException
authormhaupt
Wed, 02 Mar 2016 20:16:11 +0100
changeset 36220 32754521e67e
parent 36219 437e72684a42
child 36221 0543ccb78f27
8150635: j.l.i.MethodHandles.loop(...) throws IndexOutOfBoundsException Reviewed-by: psandoz
jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
jdk/test/java/lang/invoke/T8139885.java
--- a/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Wed Mar 02 16:25:29 2016 +0000
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Wed Mar 02 20:16:11 2016 +0100
@@ -3268,12 +3268,17 @@
      * <li>This list of types is called the "common prefix".
      * </ol>
      * <p>
-     * <em>Step 1B: Determine loop parameters.</em><ol type="a">
-     * <li>Examine init function parameter lists.
-     * <li>Omitted init functions are deemed to have {@code null} parameter lists.
-     * <li>All init function parameter lists must be effectively identical.
-     * <li>The longest parameter list (which is necessarily unique) is called the "common suffix".
+     * <em>Step 1B: Determine loop parameters.</em><ul>
+     * <li><b>If at least one init function is given,</b><ol type="a">
+     *   <li>Examine init function parameter lists.
+     *   <li>Omitted init functions are deemed to have {@code null} parameter lists.
+     *   <li>All init function parameter lists must be effectively identical.
+     *   <li>The longest parameter list (which is necessarily unique) is called the "common suffix".
      * </ol>
+     * <li><b>If no init function is given,</b><ol type="a">
+     *   <li>Examine the suffixes of the step, pred, and fini parameter lists, after removing the "common prefix".
+     *   <li>The longest of these suffixes is taken as the "common suffix".
+     * </ol></ul>
      * <p>
      * <em>Step 1C: Determine loop return type.</em><ol type="a">
      * <li>Examine fini function return types, disregarding omitted fini functions.
@@ -3286,9 +3291,6 @@
      * <li>Every non-omitted pred function must have a {@code boolean} return type.
      * </ol>
      * <p>
-     * (Implementation Note: Steps 1A, 1B, 1C, 1D are logically independent of each other, and may be performed in any
-     * order.)
-     * <p>
      * <em>Step 2: Determine parameter lists.</em><ol type="a">
      * <li>The parameter list for the resulting loop handle will be the "common suffix".
      * <li>The parameter list for init functions will be adjusted to the "common suffix". (Note that their parameter
@@ -3375,10 +3377,10 @@
      * <blockquote><pre>{@code
      * // iterative implementation of the factorial function as a loop handle
      * static int one(int k) { return 1; }
-     * int inc(int i, int acc, int k) { return i + 1; }
-     * int mult(int i, int acc, int k) { return i * acc; }
-     * boolean pred(int i, int acc, int k) { return i < k; }
-     * int fin(int i, int acc, int k) { return acc; }
+     * static int inc(int i, int acc, int k) { return i + 1; }
+     * static int mult(int i, int acc, int k) { return i * acc; }
+     * static boolean pred(int i, int acc, int k) { return i < k; }
+     * static int fin(int i, int acc, int k) { return acc; }
      * // assume MH_one, MH_inc, MH_mult, MH_pred, and MH_fin are handles to the above methods
      * // null initializer for counter, should initialize to 0
      * MethodHandle[] counterClause = new MethodHandle[]{null, MH_inc};
@@ -3436,9 +3438,7 @@
                 collect(Collectors.toList());
 
         // Step 1B: determine loop parameters.
-        final List<Class<?>> empty = new ArrayList<>();
-        final List<Class<?>> commonSuffix = init.stream().filter(Objects::nonNull).map(MethodHandle::type).
-                map(MethodType::parameterList).reduce((p, q) -> p.size() >= q.size() ? p : q).orElse(empty);
+        final List<Class<?>> commonSuffix = buildCommonSuffix(init, step, pred, fini, commonPrefix.size());
         checkLoop1b(init, commonSuffix);
 
         // Step 1C: determine loop return type.
@@ -3520,9 +3520,9 @@
      * @apiNote Example:
      * <blockquote><pre>{@code
      * // implement the zip function for lists as a loop handle
-     * List<String> initZip(Iterator<String> a, Iterator<String> b) { return new ArrayList<>(); }
-     * boolean zipPred(List<String> zip, Iterator<String> a, Iterator<String> b) { return a.hasNext() && b.hasNext(); }
-     * List<String> zipStep(List<String> zip, Iterator<String> a, Iterator<String> b) {
+     * static List<String> initZip(Iterator<String> a, Iterator<String> b) { return new ArrayList<>(); }
+     * static boolean zipPred(List<String> zip, Iterator<String> a, Iterator<String> b) { return a.hasNext() && b.hasNext(); }
+     * static List<String> zipStep(List<String> zip, Iterator<String> a, Iterator<String> b) {
      *   zip.add(a.next());
      *   zip.add(b.next());
      *   return zip;
@@ -3594,9 +3594,9 @@
      * @apiNote Example:
      * <blockquote><pre>{@code
      * // int i = 0; while (i < limit) { ++i; } return i; => limit
-     * int zero(int limit) { return 0; }
-     * int step(int i, int limit) { return i + 1; }
-     * boolean pred(int i, int limit) { return i < limit; }
+     * static int zero(int limit) { return 0; }
+     * static int step(int i, int limit) { return i + 1; }
+     * static boolean pred(int i, int limit) { return i < limit; }
      * // assume MH_zero, MH_step, and MH_pred are handles to the above methods
      * MethodHandle loop = MethodHandles.doWhileLoop(MH_zero, MH_step, MH_pred);
      * assertEquals(23, loop.invoke(23));
@@ -3664,8 +3664,8 @@
      * <blockquote><pre>{@code
      * // String s = "Lambdaman!"; for (int i = 0; i < 13; ++i) { s = "na " + s; } return s;
      * // => a variation on a well known theme
-     * String start(String arg) { return arg; }
-     * String step(int counter, String v, String arg) { return "na " + v; }
+     * static String start(String arg) { return arg; }
+     * static String step(int counter, String v, String arg) { return "na " + v; }
      * // assume MH_start and MH_step are handles to the two methods above
      * MethodHandle fit13 = MethodHandles.constant(int.class, 13);
      * MethodHandle loop = MethodHandles.countedLoop(fit13, MH_start, MH_step);
@@ -3808,11 +3808,11 @@
      * @apiNote Example:
      * <blockquote><pre>{@code
      * // reverse a list
-     * List<String> reverseStep(String e, List<String> r, List<String> l) {
+     * static List<String> reverseStep(String e, List<String> r, List<String> l) {
      *   r.add(0, e);
      *   return r;
      * }
-     * List<String> newArrayList(List<String> l) { return new ArrayList<>(); }
+     * static List<String> newArrayList(List<String> l) { return new ArrayList<>(); }
      * // assume MH_reverseStep, MH_newArrayList are handles to the above methods
      * MethodHandle loop = MethodHandles.iteratedLoop(null, MH_newArrayList, MH_reverseStep);
      * List<String> list = Arrays.asList("a", "b", "c", "d", "e");
@@ -4084,6 +4084,21 @@
         }
     }
 
+    private static List<Class<?>> buildCommonSuffix(List<MethodHandle> init, List<MethodHandle> step, List<MethodHandle> pred, List<MethodHandle> fini, int cpSize) {
+        final List<Class<?>> empty = List.of();
+        final List<MethodHandle> nonNullInits = init.stream().filter(Objects::nonNull).collect(Collectors.toList());
+        if (nonNullInits.isEmpty()) {
+            final List<Class<?>> longest = Stream.of(step, pred, fini).flatMap(List::stream).filter(Objects::nonNull).
+                    // take only those that can contribute to a common suffix because they are longer than the prefix
+                    map(MethodHandle::type).filter(t -> t.parameterCount() > cpSize).map(MethodType::parameterList).
+                    reduce((p, q) -> p.size() >= q.size() ? p : q).orElse(empty);
+            return longest.size() == 0 ? empty : longest.subList(cpSize, longest.size());
+        } else {
+            return nonNullInits.stream().map(MethodHandle::type).map(MethodType::parameterList).
+                    reduce((p, q) -> p.size() >= q.size() ? p : q).get();
+        }
+    }
+
     private static void checkLoop1b(List<MethodHandle> init, List<Class<?>> commonSuffix) {
         if (init.stream().filter(Objects::nonNull).map(MethodHandle::type).map(MethodType::parameterList).
                 anyMatch(pl -> !pl.equals(commonSuffix.subList(0, pl.size())))) {
@@ -4109,8 +4124,10 @@
     }
 
     private static void checkLoop2(List<MethodHandle> step, List<MethodHandle> pred, List<MethodHandle> fini, List<Class<?>> commonParameterSequence) {
+        final int cpSize = commonParameterSequence.size();
         if (Stream.of(step, pred, fini).flatMap(List::stream).filter(Objects::nonNull).map(MethodHandle::type).
-                map(MethodType::parameterList).anyMatch(pl -> !pl.equals(commonParameterSequence.subList(0, pl.size())))) {
+                map(MethodType::parameterList).
+                anyMatch(pl -> pl.size() > cpSize || !pl.equals(commonParameterSequence.subList(0, pl.size())))) {
             throw newIllegalArgumentException("found non-effectively identical parameter type lists:\nstep: " + step +
                     "\npred: " + pred + "\nfini: " + fini + " (common parameter sequence: " + commonParameterSequence + ")");
         }
--- a/jdk/test/java/lang/invoke/T8139885.java	Wed Mar 02 16:25:29 2016 +0000
+++ b/jdk/test/java/lang/invoke/T8139885.java	Wed Mar 02 20:16:11 2016 +0100
@@ -27,6 +27,7 @@
  * @bug 8139885
  * @bug 8143798
  * @bug 8150825
+ * @bug 8150635
  * @run testng/othervm -ea -esa test.java.lang.invoke.T8139885
  */
 
@@ -77,6 +78,15 @@
     }
 
     @Test
+    public static void testLoopNullInit() throws Throwable {
+        // null initializer for counter, should initialize to 0, one-clause loop
+        MethodHandle[] counterClause = new MethodHandle[]{null, Loop.MH_inc, Loop.MH_pred, Loop.MH_fin};
+        MethodHandle loop = MethodHandles.loop(counterClause);
+        assertEquals(Loop.MT_loop, loop.type());
+        assertEquals(10, loop.invoke(10));
+    }
+
+    @Test
     public static void testLoopVoid1() throws Throwable {
         // construct a post-checked loop that only does one iteration and has a void body and void local state
         MethodHandle loop = MethodHandles.loop(new MethodHandle[]{Empty.MH_f, Empty.MH_f, Empty.MH_pred, null});
@@ -94,6 +104,15 @@
     }
 
     @Test
+    public static void testLoopVoid3() throws Throwable {
+        // construct a post-checked loop that only does one iteration and has a void body and void local state,
+        // and that has a void finalizer
+        MethodHandle loop = MethodHandles.loop(new MethodHandle[]{null, Empty.MH_f, Empty.MH_pred, Empty.MH_f});
+        assertEquals(MethodType.methodType(void.class), loop.type());
+        loop.invoke();
+    }
+
+    @Test
     public static void testLoopFacWithVoidState() throws Throwable {
         // like testLoopFac, but with additional void state that outputs a dot
         MethodHandle[] counterClause = new MethodHandle[]{Fac.MH_zero, Fac.MH_inc};
@@ -105,6 +124,31 @@
     }
 
     @Test
+    public static void testLoopVoidInt() throws Throwable {
+        // construct a post-checked loop that only does one iteration and has a void body and void local state,
+        // and that returns a constant
+        MethodHandle loop = MethodHandles.loop(new MethodHandle[]{null, Empty.MH_f, Empty.MH_pred, Empty.MH_c});
+        assertEquals(MethodType.methodType(int.class), loop.type());
+        assertEquals(23, loop.invoke());
+    }
+
+    @Test
+    public static void testLoopWithVirtuals() throws Throwable {
+        // construct a loop (to calculate factorial) that uses a mix of static and virtual methods
+        MethodHandle[] counterClause = new MethodHandle[]{null, LoopWithVirtuals.permute(LoopWithVirtuals.MH_inc)};
+        MethodHandle[] accumulatorClause = new MethodHandle[]{
+                // init function must indicate the loop arguments (there is no other means to determine them)
+                MethodHandles.dropArguments(LoopWithVirtuals.MH_one, 0, LoopWithVirtuals.class),
+                LoopWithVirtuals.permute(LoopWithVirtuals.MH_mult),
+                LoopWithVirtuals.permute(LoopWithVirtuals.MH_pred),
+                LoopWithVirtuals.permute(LoopWithVirtuals.MH_fin)
+        };
+        MethodHandle loop = MethodHandles.loop(counterClause, accumulatorClause);
+        assertEquals(LoopWithVirtuals.MT_loop, loop.type());
+        assertEquals(120, loop.invoke(new LoopWithVirtuals(), 5));
+    }
+
+    @Test
     public static void testLoopNegative() throws Throwable {
         MethodHandle mh_loop =
                 LOOKUP.findStatic(MethodHandles.class, "loop", methodType(MethodHandle.class, MethodHandle[][].class));
@@ -121,31 +165,38 @@
         List<MethodHandle> nesteps = Arrays.asList(Fac.MH_inc, eek, Fac.MH_dot);
         List<MethodHandle> nepreds = Arrays.asList(null, Fac.MH_pred, null);
         List<MethodHandle> nefinis = Arrays.asList(null, Fac.MH_fin, null);
+        List<MethodHandle> lvsteps = Arrays.asList(LoopWithVirtuals.MH_inc, LoopWithVirtuals.MH_mult);
+        List<MethodHandle> lvpreds = Arrays.asList(null, LoopWithVirtuals.MH_pred);
+        List<MethodHandle> lvfinis = Arrays.asList(null, LoopWithVirtuals.MH_fin);
         MethodHandle[][][] cases = {
-                null,
-                {},
-                {{null, Fac.MH_inc}, {Fac.MH_one, null, Fac.MH_mult, Fac.MH_pred, Fac.MH_fin}},
-                {{null, Fac.MH_inc}, null},
-                {{Fac.MH_zero, Fac.MH_dot}},
-                {{ii}, {id}, {i3}},
-                {{null, Fac.MH_inc, null, Fac.MH_fin}, {null, Fac.MH_inc, null, Fac.MH_inc},
-                        {null, Counted.MH_start, null, Counted.MH_step}},
-                {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, Fac.MH_mult, null, Fac.MH_fin}, {null, Fac.MH_dot}},
-                {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, Fac.MH_mult, Fac.MH_fin, Fac.MH_fin}, {null, Fac.MH_dot}},
-                {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, eek, Fac.MH_pred, Fac.MH_fin}, {null, Fac.MH_dot}}
+                /*  1 */ null,
+                /*  2 */ {},
+                /*  3 */ {{null, Fac.MH_inc}, {Fac.MH_one, null, Fac.MH_mult, Fac.MH_pred, Fac.MH_fin}},
+                /*  4 */ {{null, Fac.MH_inc}, null},
+                /*  5 */ {{Fac.MH_zero, Fac.MH_dot}},
+                /*  6 */ {{ii}, {id}, {i3}},
+                /*  7 */ {{null, Fac.MH_inc, null, Fac.MH_fin}, {null, Fac.MH_inc, null, Fac.MH_inc},
+                            {null, Counted.MH_start, null, Counted.MH_step}},
+                /*  8 */ {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, Fac.MH_mult, null, Fac.MH_fin}, {null, Fac.MH_dot}},
+                /*  9 */ {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, Fac.MH_mult, Fac.MH_fin, Fac.MH_fin}, {null, Fac.MH_dot}},
+                /* 10 */ {{Fac.MH_zero, Fac.MH_inc}, {Fac.MH_one, eek, Fac.MH_pred, Fac.MH_fin}, {null, Fac.MH_dot}},
+                /* 11 */ {{null, LoopWithVirtuals.MH_inc}, {LoopWithVirtuals.MH_one, LoopWithVirtuals.MH_mult, LoopWithVirtuals.MH_pred, LoopWithVirtuals.MH_fin}}
         };
         String[] messages = {
-                "null or no clauses passed",
-                "null or no clauses passed",
-                "All loop clauses must be represented as MethodHandle arrays with at most 4 elements.",
-                "null clauses are not allowed",
-                "clause 0: init and step return types must match: int != void",
-                "found non-effectively identical init parameter type lists: " + inits + " (common suffix: " + ints + ")",
-                "found non-identical finalizer return types: " + finis + " (return type: int)",
-                "no predicate found: " + preds1,
-                "predicates must have boolean return type: " + preds2,
-                "found non-effectively identical parameter type lists:\nstep: " + nesteps + "\npred: " + nepreds +
-                        "\nfini: " + nefinis + " (common parameter sequence: " + ints + ")"
+                /*  1 */ "null or no clauses passed",
+                /*  2 */ "null or no clauses passed",
+                /*  3 */ "All loop clauses must be represented as MethodHandle arrays with at most 4 elements.",
+                /*  4 */ "null clauses are not allowed",
+                /*  5 */ "clause 0: init and step return types must match: int != void",
+                /*  6 */ "found non-effectively identical init parameter type lists: " + inits +
+                            " (common suffix: " + ints + ")",
+                /*  7 */ "found non-identical finalizer return types: " + finis + " (return type: int)",
+                /*  8 */ "no predicate found: " + preds1,
+                /*  9 */ "predicates must have boolean return type: " + preds2,
+                /* 10 */ "found non-effectively identical parameter type lists:\nstep: " + nesteps +
+                            "\npred: " + nepreds + "\nfini: " + nefinis + " (common parameter sequence: " + ints + ")",
+                /* 11 */ "found non-effectively identical parameter type lists:\nstep: " + lvsteps +
+                            "\npred: " + lvpreds + "\nfini: " + lvfinis + " (common parameter sequence: " + ints + ")"
         };
         for (int i = 0; i < cases.length; ++i) {
             boolean caught = false;
@@ -570,18 +621,25 @@
             return false;
         }
 
+        static int c() {
+            return 23;
+        }
+
         static final Class<Empty> EMPTY = Empty.class;
 
         static final MethodType MT_f = methodType(void.class);
         static final MethodType MT_pred = methodType(boolean.class);
+        static final MethodType MT_c = methodType(int.class);
 
         static final MethodHandle MH_f;
         static final MethodHandle MH_pred;
+        static final MethodHandle MH_c;
 
         static {
             try {
                 MH_f = LOOKUP.findStatic(EMPTY, "f", MT_f);
                 MH_pred = LOOKUP.findStatic(EMPTY, "pred", MT_pred);
+                MH_c = LOOKUP.findStatic(EMPTY, "c", MT_c);
             } catch (Exception e) {
                 throw new ExceptionInInitializerError(e);
             }
@@ -651,6 +709,104 @@
 
     }
 
+    static class Loop {
+
+        static int inc(int i, int k) {
+            return i + 1;
+        }
+
+        static boolean pred(int i, int k) {
+            return i < k;
+        }
+
+        static int fin(int i, int k) {
+            return k;
+        }
+
+        static final Class<Loop> LOOP = Loop.class;
+
+        static final MethodType MT_inc = methodType(int.class, int.class, int.class);
+        static final MethodType MT_pred = methodType(boolean.class, int.class, int.class);
+        static final MethodType MT_fin = methodType(int.class, int.class, int.class);
+
+        static final MethodHandle MH_inc;
+        static final MethodHandle MH_pred;
+        static final MethodHandle MH_fin;
+
+        static final MethodType MT_loop = methodType(int.class, int.class);
+
+        static {
+            try {
+                MH_inc = LOOKUP.findStatic(LOOP, "inc", MT_inc);
+                MH_pred = LOOKUP.findStatic(LOOP, "pred", MT_pred);
+                MH_fin = LOOKUP.findStatic(LOOP, "fin", MT_fin);
+            } catch (Exception e) {
+                throw new ExceptionInInitializerError(e);
+            }
+        }
+
+    }
+
+    static class LoopWithVirtuals {
+
+        static int one(int k) {
+            return 1;
+        }
+
+        int inc(int i, int acc, int k) {
+            return i + 1;
+        }
+
+        int mult(int i, int acc, int k) {
+            return i * acc;
+        }
+
+        boolean pred(int i, int acc, int k) {
+            return i < k;
+        }
+
+        int fin(int i, int acc, int k) {
+            return acc;
+        }
+
+        static final Class<LoopWithVirtuals> LOOP_WITH_VIRTUALS = LoopWithVirtuals.class;
+
+        static final MethodType MT_one = methodType(int.class, int.class);
+        static final MethodType MT_inc = methodType(int.class, int.class, int.class, int.class);
+        static final MethodType MT_mult = methodType(int.class, int.class, int.class, int.class);
+        static final MethodType MT_pred = methodType(boolean.class, int.class, int.class, int.class);
+        static final MethodType MT_fin = methodType(int.class, int.class, int.class, int.class);
+
+        static final MethodHandle MH_one;
+        static final MethodHandle MH_inc;
+        static final MethodHandle MH_mult;
+        static final MethodHandle MH_pred;
+        static final MethodHandle MH_fin;
+
+        static final MethodType MT_loop = methodType(int.class, LOOP_WITH_VIRTUALS, int.class);
+
+        static {
+            try {
+                MH_one = LOOKUP.findStatic(LOOP_WITH_VIRTUALS, "one", MT_one);
+                MH_inc = LOOKUP.findVirtual(LOOP_WITH_VIRTUALS, "inc", MT_inc);
+                MH_mult = LOOKUP.findVirtual(LOOP_WITH_VIRTUALS, "mult", MT_mult);
+                MH_pred = LOOKUP.findVirtual(LOOP_WITH_VIRTUALS, "pred", MT_pred);
+                MH_fin = LOOKUP.findVirtual(LOOP_WITH_VIRTUALS, "fin", MT_fin);
+            } catch (Exception e) {
+                throw new ExceptionInInitializerError(e);
+            }
+        }
+
+        static MethodHandle permute(MethodHandle h) {
+            // The handles representing virtual methods need to be rearranged to match the required order of arguments
+            // (loop-local state comes first, then loop arguments). As the receiver comes first in the signature but is
+            // a loop argument, it must be moved to the appropriate position in the signature.
+            return MethodHandles.permuteArguments(h,
+                    methodType(h.type().returnType(), int.class, int.class, LOOP_WITH_VIRTUALS, int.class), 2, 0, 1, 3);
+        }
+
+    }
+
     static class While {
 
         static int zero(int limit) {