8046905: apply on apply is broken
authorattila
Mon, 23 Jun 2014 10:59:33 +0200
changeset 25237 bf5efe0a93ba
parent 25236 fac419f1e889
child 25238 28476bdc25ce
8046905: apply on apply is broken Reviewed-by: hannesw, lagergren
nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/test/script/basic/JDK-8046905.js
nashorn/test/script/basic/JDK-8046905.js.EXPECTED
nashorn/test/script/basic/JDK-8047057.js
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Fri Jun 20 12:25:00 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Mon Jun 23 10:59:33 2014 +0200
@@ -35,6 +35,7 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.lang.invoke.SwitchPoint;
+import java.util.Collections;
 import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.LinkRequest;
@@ -593,6 +594,12 @@
     private GuardedInvocation createApplyOrCallCall(final boolean isApply, final CallSiteDescriptor desc, final LinkRequest request, final Object[] args) {
         final MethodType descType = desc.getMethodType();
         final int paramCount = descType.parameterCount();
+        if(descType.parameterType(paramCount - 1).isArray()) {
+            // This is vararg invocation of apply or call. This can normally only happen when we do a recursive
+            // invocation of createApplyOrCallCall (because we're doing apply-of-apply). In this case, create delegate
+            // linkage by unpacking the vararg invocation and use pairArguments to introduce the necessary spreader.
+            return createVarArgApplyOrCallCall(isApply, desc, request, args);
+        }
 
         final boolean passesThis = paramCount > 2;
         final boolean passesArgs = paramCount > 3;
@@ -647,6 +654,7 @@
                     System.arraycopy(args, 3, tmp, 0, tmp.length);
                     appliedArgs[2] = NativeFunction.toApplyArgs(tmp);
                 } else {
+                    assert !isApply;
                     System.arraycopy(args, 3, appliedArgs, 2, args.length - 3);
                 }
             } else if (isFailedApplyToCall) {
@@ -705,8 +713,7 @@
         }
         final MethodType guardType = guard.type();
 
-        // Original function guard will expect the invoked function in parameter position 0, but we're passing it in
-        // position 1.
+        // We need to account for the dropped (apply|call) function argument.
         guard = MH.dropArguments(guard, 0, descType.parameterType(0));
         // Take the "isApplyFunction" guard, and bind it to this function.
         MethodHandle applyFnGuard = MH.insertArguments(IS_APPLY_FUNCTION, 2, this);
@@ -718,7 +725,72 @@
         return appliedInvocation.replaceMethods(inv, guard);
     }
 
-     private static MethodHandle bindImplicitThis(final Object fn, final MethodHandle mh) {
+    /*
+     * This method is used for linking nested apply. Specialized apply and call linking will create a variable arity
+     * call site for an apply call; when createApplyOrCallCall sees a linking request for apply or call with
+     * Nashorn-style variable arity call site (last argument type is Object[]) it'll delegate to this method.
+     * This method converts the link request from a vararg to a non-vararg one (unpacks the array), then delegates back
+     * to createApplyOrCallCall (with which it is thus mutually recursive), and adds appropriate argument spreaders to
+     * invocation and the guard of whatever createApplyOrCallCall returned to adapt it back into a variable arity
+     * invocation. It basically reduces the problem of vararg call site linking of apply and call back to the (already
+     * solved by createApplyOrCallCall) non-vararg call site linking.
+     */
+    private GuardedInvocation createVarArgApplyOrCallCall(final boolean isApply, final CallSiteDescriptor desc,
+            final LinkRequest request, final Object[] args) {
+        final MethodType descType = desc.getMethodType();
+        final int paramCount = descType.parameterCount();
+        final Object[] varArgs = (Object[])args[paramCount - 1];
+        // -1 'cause we're not passing the vararg array itself
+        final int copiedArgCount = args.length - 1;
+        int varArgCount = varArgs.length;
+
+        // Spread arguments for the delegate createApplyOrCallCall invocation.
+        final Object[] spreadArgs = new Object[copiedArgCount + varArgCount];
+        System.arraycopy(args, 0, spreadArgs, 0, copiedArgCount);
+        System.arraycopy(varArgs, 0, spreadArgs, copiedArgCount, varArgCount);
+
+        // Spread call site descriptor for the delegate createApplyOrCallCall invocation. We drop vararg array and
+        // replace it with a list of Object.class.
+        final MethodType spreadType = descType.dropParameterTypes(paramCount - 1, paramCount).appendParameterTypes(
+                Collections.<Class<?>>nCopies(varArgCount, Object.class));
+        final CallSiteDescriptor spreadDesc = desc.changeMethodType(spreadType);
+
+        // Delegate back to createApplyOrCallCall with the spread (that is, reverted to non-vararg) request/
+        final LinkRequest spreadRequest = request.replaceArguments(spreadDesc, spreadArgs);
+        final GuardedInvocation spreadInvocation = createApplyOrCallCall(isApply, spreadDesc, spreadRequest, spreadArgs);
+
+        // Add spreader combinators to returned invocation and guard.
+        return spreadInvocation.replaceMethods(
+                // Use standard ScriptObject.pairArguments on the invocation
+                pairArguments(spreadInvocation.getInvocation(), descType),
+                // Use our specialized spreadGuardArguments on the guard (see below).
+                spreadGuardArguments(spreadInvocation.getGuard(), descType));
+    }
+
+    private static MethodHandle spreadGuardArguments(final MethodHandle guard, final MethodType descType) {
+        final MethodType guardType = guard.type();
+        final int guardParamCount = guardType.parameterCount();
+        final int descParamCount = descType.parameterCount();
+        final int spreadCount = guardParamCount - descParamCount + 1;
+        if (spreadCount <= 0) {
+            // Guard doesn't dip into the varargs
+            return guard;
+        }
+
+        final MethodHandle arrayConvertingGuard;
+        // If the last parameter type of the guard is an array, then it is already itself a guard for a vararg apply
+        // invocation. We must filter the last argument with toApplyArgs otherwise deeper levels of nesting will fail
+        // with ClassCastException of NativeArray to Object[].
+        if(guardType.parameterType(guardParamCount - 1).isArray()) {
+            arrayConvertingGuard = MH.filterArguments(guard, guardParamCount - 1, NativeFunction.TO_APPLY_ARGS);
+        } else {
+            arrayConvertingGuard = guard;
+        }
+
+        return ScriptObject.adaptHandleToVarArgCallSite(arrayConvertingGuard, descParamCount);
+    }
+
+    private static MethodHandle bindImplicitThis(final Object fn, final MethodHandle mh) {
          final MethodHandle bound;
          if(fn instanceof ScriptFunction && ((ScriptFunction)fn).needsWrappedThis()) {
              bound = MH.filterArguments(mh, 1, SCRIPTFUNCTION_GLOBALFILTER);
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Fri Jun 20 12:25:00 2014 +0200
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Mon Jun 23 10:59:33 2014 +0200
@@ -2499,18 +2499,7 @@
         }
 
         if (isCallerVarArg) {
-            final int spreadArgs = parameterCount - callCount + 1;
-            return MH.filterArguments(
-                MH.asSpreader(
-                    methodHandle,
-                    Object[].class,
-                    spreadArgs),
-                callCount - 1,
-                MH.insertArguments(
-                    TRUNCATINGFILTER,
-                    0,
-                    spreadArgs)
-                );
+            return adaptHandleToVarArgCallSite(methodHandle, callCount);
         }
 
         if (callCount < parameterCount) {
@@ -2541,6 +2530,21 @@
         return methodHandle;
     }
 
+    static MethodHandle adaptHandleToVarArgCallSite(final MethodHandle mh, final int callSiteParamCount) {
+        final int spreadArgs = mh.type().parameterCount() - callSiteParamCount + 1;
+        return MH.filterArguments(
+            MH.asSpreader(
+            mh,
+            Object[].class,
+            spreadArgs),
+            callSiteParamCount - 1,
+            MH.insertArguments(
+                TRUNCATINGFILTER,
+                0,
+                spreadArgs)
+            );
+    }
+
     @SuppressWarnings("unused")
     private static Object[] truncatingFilter(final int n, final Object[] array) {
         final int length = array == null ? 0 : array.length;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8046905.js	Mon Jun 23 10:59:33 2014 +0200
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2010, 2014, 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.
+ */
+
+/**
+ * JDK-8046905: apply on apply is broken
+ *
+ * @test
+ * @run
+ */
+
+var apply = Function.prototype.apply;
+var call = Function.prototype.call;
+var sort = Array.prototype.sort;
+var join = Array.prototype.join;
+
+// Running three times so that we test an already linked call site too:
+// i==0: linking initially with assumed optimistic returned type int.
+// i==1: linking after deoptimization with returned type Object.
+// i==2: re-running code linked in previous iteration. This will 
+//       properly exercise the guards too.
+print("1 level of apply")
+for(i = 0; i < 3; ++i) {
+    print(sort.apply([4,3,2,1]))
+}
+print("2 levels of apply")
+for(i = 0; i < 3; ++i) {
+    print(apply.apply(sort,[[4,3,2,1]]))
+}
+print("3 levels of apply")
+for(i = 0; i < 3; ++i) {
+    print(apply.apply(apply,[sort,[[4,3,2,1]]]))
+}
+print("4 levels of apply")
+for(i = 0; i < 3; ++i) {
+    print(apply.apply(apply,[apply,[sort,[[4,3,2,1]]]]))
+}
+print("5 levels of apply")
+for(i = 0; i < 3; ++i) {
+    print(apply.apply(apply,[apply,[apply,[sort,[[4,3,2,1]]]]]))
+}
+print("Many levels of apply!")
+for(i = 0; i < 3; ++i) {
+    print(apply.apply(apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[apply,[sort,[[4,3,2,1]]]]]]]]]]]]]]]]]]]]]]))
+}
+
+print("different invocations that'll trigger relinking")
+var invocation = [sort,[[4,3,2,1]]];
+for(i = 0; i < 4; ++i) {
+    print(apply.apply(apply,[apply,invocation]))
+    // First change after i==1, so it relinks an otherwise stable linkage
+    if(i == 1) {
+	invocation = [sort,[[8,7,6,5]]];
+    } else if(i == 2) {
+        invocation = [join,[[8,7,6,5],["-"]]];
+    }
+}
+
+print("Many levels of call!")
+for(i = 0; i < 3; ++i) {
+    print(call.call(call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,call,sort,[4,3,2,1]))
+}
+
+print("call apply call apply call... a lot");
+for(i = 0; i < 3; ++i) {
+    print(apply.call(call, apply, [call, apply, [call, apply, [call, apply, [call, apply, [call, apply, [sort, [4,3,2,1]]]]]]]))
+}
+
+print("apply call apply call apply... a lot");
+for(i = 0; i < 3; ++i) {
+    print(call.apply(apply, [call, apply, [call, apply, [call, apply, [call, apply, [call, apply, [call, apply, [call, sort, [[4,3,2,1]]]]]]]]]))
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8046905.js.EXPECTED	Mon Jun 23 10:59:33 2014 +0200
@@ -0,0 +1,41 @@
+1 level of apply
+1,2,3,4
+1,2,3,4
+1,2,3,4
+2 levels of apply
+1,2,3,4
+1,2,3,4
+1,2,3,4
+3 levels of apply
+1,2,3,4
+1,2,3,4
+1,2,3,4
+4 levels of apply
+1,2,3,4
+1,2,3,4
+1,2,3,4
+5 levels of apply
+1,2,3,4
+1,2,3,4
+1,2,3,4
+Many levels of apply!
+1,2,3,4
+1,2,3,4
+1,2,3,4
+different invocations that'll trigger relinking
+1,2,3,4
+1,2,3,4
+5,6,7,8
+8-7-6-5
+Many levels of call!
+1,2,3,4
+1,2,3,4
+1,2,3,4
+call apply call apply call... a lot
+1,2,3,4
+1,2,3,4
+1,2,3,4
+apply call apply call apply... a lot
+1,2,3,4
+1,2,3,4
+1,2,3,4
--- a/nashorn/test/script/basic/JDK-8047057.js	Fri Jun 20 12:25:00 2014 +0200
+++ b/nashorn/test/script/basic/JDK-8047057.js	Mon Jun 23 10:59:33 2014 +0200
@@ -67,7 +67,7 @@
 makeFuncAndCall("with({5.0000000000000000000000: String()}){(false); }");
 makeFuncAndCall("try { var x = undefined, x = 5.0000000000000000000000; } catch(x) { x = undefined; }");
 makeFuncAndCall("(function (x){ x %= this}(false))");
-// makeFuncAndCall("eval.apply.apply(function(){ eval('') })");
+makeFuncAndCall("eval.apply.apply(function(){ eval('') })");
 makeFuncAndCall("(false % !this) && 0");
 makeFuncAndCall("with({8: 'fafafa'.replace()}){ }");
 makeFuncAndCall("(function (x) '' )(true)");