8006570: This-value for non-strict functions should be converted to object
authorhannesw
Tue, 22 Jan 2013 14:14:37 +0100
changeset 16186 91bd9d2aa2f5
parent 16185 893aabe8c800
child 16187 32ebb6dca838
8006570: This-value for non-strict functions should be converted to object Reviewed-by: jlaskey, lagergren, attila
nashorn/src/jdk/nashorn/internal/objects/Global.java
nashorn/src/jdk/nashorn/internal/objects/NativeBoolean.java
nashorn/src/jdk/nashorn/internal/objects/NativeFunction.java
nashorn/src/jdk/nashorn/internal/objects/NativeNumber.java
nashorn/src/jdk/nashorn/internal/objects/NativeString.java
nashorn/src/jdk/nashorn/internal/runtime/GlobalObject.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java
nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/src/jdk/nashorn/internal/runtime/SetMethodCreator.java
nashorn/src/jdk/nashorn/internal/runtime/linker/NashornGuardedInvocation.java
nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java
nashorn/src/jdk/nashorn/internal/runtime/linker/PrimitiveLookup.java
nashorn/test/script/basic/JDK-8006570.js
nashorn/test/script/basic/JDK-8006570.js.EXPECTED
--- a/nashorn/src/jdk/nashorn/internal/objects/Global.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/Global.java	Tue Jan 22 14:14:37 2013 +0100
@@ -419,6 +419,18 @@
     }
 
     @Override
+    public MethodHandle getWrapFilter(Object obj) {
+        if (obj instanceof String || obj instanceof ConsString) {
+            return NativeString.WRAPFILTER;
+        } else if (obj instanceof Number) {
+            return NativeNumber.WRAPFILTER;
+        } else if (obj instanceof Boolean) {
+            return NativeBoolean.WRAPFILTER;
+        }
+        throw new IllegalArgumentException("Unsupported primitive value: " + obj);
+    }
+
+    @Override
     public GuardedInvocation numberLookup(final NashornCallSiteDescriptor callSite, final Number self) {
         return NativeNumber.lookupPrimitive(callSite, self);
     }
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeBoolean.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeBoolean.java	Tue Jan 22 14:14:37 2013 +0100
@@ -50,7 +50,7 @@
 public final class NativeBoolean extends ScriptObject {
     private final boolean value;
 
-    private final static MethodHandle WRAPFILTER = findWrapFilter();
+    final static MethodHandle WRAPFILTER = findWrapFilter();
 
     NativeBoolean(final boolean value) {
         this(value, Global.instance().getBooleanPrototype());
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeFunction.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeFunction.java	Tue Jan 22 14:14:37 2013 +0100
@@ -67,7 +67,7 @@
     }
 
     private static Object convertThis(final ScriptFunction func, final Object thiz) {
-        if (!(thiz instanceof ScriptObject) && !func.isStrict() && !func.isBuiltin()) {
+        if (!(thiz instanceof ScriptObject) && func.isNonStrictFunction()) {
             if (thiz == UNDEFINED || thiz == null) {
                 return Global.instance();
             }
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeNumber.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeNumber.java	Tue Jan 22 14:14:37 2013 +0100
@@ -57,7 +57,7 @@
 @ScriptClass("Number")
 public final class NativeNumber extends ScriptObject {
 
-    private static final MethodHandle WRAPFILTER = findWrapFilter();
+    static final MethodHandle WRAPFILTER = findWrapFilter();
 
     /** ECMA 15.7.3.2 largest positive finite value */
     @Property(attributes = Attribute.NON_ENUMERABLE_CONSTANT, where = Where.CONSTRUCTOR)
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeString.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeString.java	Tue Jan 22 14:14:37 2013 +0100
@@ -68,7 +68,7 @@
 
     private final CharSequence value;
 
-    private static final MethodHandle WRAPFILTER = findWrapFilter();
+    static final MethodHandle WRAPFILTER = findWrapFilter();
 
     NativeString(final CharSequence value) {
         this(value, Global.instance().getStringPrototype());
--- a/nashorn/src/jdk/nashorn/internal/runtime/GlobalObject.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/GlobalObject.java	Tue Jan 22 14:14:37 2013 +0100
@@ -68,6 +68,16 @@
      */
    public Object wrapAsObject(Object obj);
 
+
+    /**
+     * Get a MethodHandle that converts the argument to its JavaScript object representation
+     *
+     * @param obj a JavaScript primitive object (String, Number, or Boolean)
+     *
+     * @return wrap filter methodhandle
+     */
+   public MethodHandle getWrapFilter(Object obj);
+
     /**
      * Wrapper for {@link jdk.nashorn.internal.objects.Global#numberLookup(NashornCallSiteDescriptor, Number)}
      *
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Tue Jan 22 14:14:37 2013 +0100
@@ -40,6 +40,7 @@
 import jdk.nashorn.internal.parser.Token;
 import jdk.nashorn.internal.runtime.linker.MethodHandleFactory;
 import jdk.nashorn.internal.runtime.linker.NashornCallSiteDescriptor;
+import jdk.nashorn.internal.runtime.linker.NashornGuardedInvocation;
 import jdk.nashorn.internal.runtime.linker.NashornGuards;
 import jdk.nashorn.internal.runtime.options.Options;
 import org.dynalang.dynalink.CallSiteDescriptor;
@@ -329,6 +330,14 @@
     public abstract boolean isBuiltin();
 
     /**
+     * Is this a non-strict (not built-in) script function?
+     * @return true if neither strict nor built-in
+     */
+    public boolean isNonStrictFunction() {
+        return !isStrict() && !isBuiltin();
+    }
+
+    /**
      * Execute this script function.
      * @param self  Target object.
      * @param arguments  Call arguments.
@@ -916,7 +925,7 @@
 
             if(NashornCallSiteDescriptor.isScope(desc)) {
                 // (this, callee, args...) => (callee, args...) => (callee, [this], args...)
-                boundHandle = MH.bindTo(callHandle, isStrict() || isBuiltin()? ScriptRuntime.UNDEFINED : Context.getGlobal());
+                boundHandle = MH.bindTo(callHandle, isNonStrictFunction() ? Context.getGlobal(): ScriptRuntime.UNDEFINED);
                 boundHandle = MH.dropArguments(boundHandle, 1, Object.class);
             } else {
                 // (this, callee, args...) permute => (callee, this, args...) which is what we get in
@@ -934,7 +943,7 @@
             final MethodHandle callHandle = getBestSpecializedInvokeHandle(type.dropParameterTypes(0, 1));
 
             if(NashornCallSiteDescriptor.isScope(desc)) {
-                boundHandle = MH.bindTo(callHandle, isStrict() || isBuiltin()? ScriptRuntime.UNDEFINED : Context.getGlobal());
+                boundHandle = MH.bindTo(callHandle, isNonStrictFunction()? Context.getGlobal() : ScriptRuntime.UNDEFINED);
                 boundHandle = MH.dropArguments(boundHandle, 0, Object.class, Object.class);
             } else {
                 boundHandle = MH.dropArguments(callHandle, 0, Object.class);
@@ -942,7 +951,7 @@
         }
 
         boundHandle = pairArguments(boundHandle, type);
-        return new GuardedInvocation(boundHandle, NashornGuards.getFunctionGuard(this));
+        return new NashornGuardedInvocation(boundHandle, null, NashornGuards.getFunctionGuard(this), isNonStrictFunction());
     }
 
     /**
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java	Tue Jan 22 14:14:37 2013 +0100
@@ -1643,16 +1643,14 @@
         // getMap() is fine as we have the prototype switchpoint depending on where the property was found
         final MethodHandle guard = NashornGuards.getMapGuard(getMap());
 
-        int invokeFlags = 0;
-
         if (methodHandle != null) {
             assert methodHandle.type().returnType().equals(returnType);
             final ScriptFunction getter = find.getGetterFunction();
-            invokeFlags = (getter != null && getter.isStrict()) ? NashornCallSiteDescriptor.CALLSITE_STRICT : 0;
+            final boolean nonStrict = getter != null && getter.isNonStrictFunction();
             if (find.isSelf()) {
                 return new NashornGuardedInvocation(methodHandle, null, ObjectClassGenerator.OBJECT_FIELDS_ONLY &&
                         NashornCallSiteDescriptor.isFastScope(desc) && !property.canChangeType() ? null : guard,
-                                invokeFlags);
+                            nonStrict);
             }
 
             final ScriptObject prototype = find.getOwner();
@@ -1660,11 +1658,11 @@
             if (!property.hasGetterFunction()) {
                 methodHandle = bindTo(methodHandle, prototype);
             }
-            return new NashornGuardedInvocation(methodHandle, getMap().getProtoGetSwitchPoint(name), guard, invokeFlags);
+            return new NashornGuardedInvocation(methodHandle, getMap().getProtoGetSwitchPoint(name), guard, nonStrict);
         }
 
         assert !NashornCallSiteDescriptor.isFastScope(desc);
-        return new NashornGuardedInvocation(Lookup.emptyGetter(returnType), getMap().getProtoGetSwitchPoint(name), guard, invokeFlags);
+        return new GuardedInvocation(Lookup.emptyGetter(returnType), getMap().getProtoGetSwitchPoint(name), guard);
     }
 
     private static GuardedInvocation findMegaMorphicGetMethod(final CallSiteDescriptor desc, final String name) {
@@ -1855,7 +1853,6 @@
         final Class<?>   valueClass = callType.parameterType(2);
 
         MethodHandle methodHandle = findOwnMH("set", void.class, keyClass, valueClass, boolean.class);
-        methodHandle = MH.asType(methodHandle, methodHandle.type().changeParameterType(0, Object.class));
         methodHandle = MH.insertArguments(methodHandle, 3, isStrict);
 
         return new GuardedInvocation(methodHandle, getScriptObjectGuard(callType));
--- a/nashorn/src/jdk/nashorn/internal/runtime/SetMethodCreator.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/SetMethodCreator.java	Tue Jan 22 14:14:37 2013 +0100
@@ -89,21 +89,19 @@
     private class SetMethod {
         private final MethodHandle methodHandle;
         private final Property property;
-        private final int invokeFlags;
+        private final boolean nonStrict;
 
         /**
          * Creates a new lookup result.
          * @param methodHandle the actual method handle
          * @param property the property object. Can be null in case we're creating a new property in the global object.
-         * @param invokeFlags flags for the invocation. Normally either 0, or
-         * {@link NashornCallSiteDescriptor#CALLSITE_STRICT} when an existing property with a strict function for a
-         * property setter is discovered.
+         * @param nonStrict True if an existing property with a non-strict function as property setter is discovered.
          */
-        SetMethod(final MethodHandle methodHandle, final Property property, final int invokeFlags) {
+        SetMethod(final MethodHandle methodHandle, final Property property, final boolean nonStrict) {
             assert methodHandle != null;
             this.methodHandle = methodHandle;
             this.property = property;
-            this.invokeFlags = invokeFlags;
+            this.nonStrict = nonStrict;
         }
 
         /**
@@ -111,7 +109,7 @@
          * @return the composed guarded invocation that represents the dynamic setter method for the property.
          */
         GuardedInvocation createGuardedInvocation() {
-            return new NashornGuardedInvocation(methodHandle, null, getGuard(), invokeFlags);
+            return new NashornGuardedInvocation(methodHandle, null, getGuard(), nonStrict);
         }
 
         private MethodHandle getGuard() {
@@ -164,20 +162,17 @@
         } else {
             boundHandle = methodHandle;
         }
-        return new SetMethod(boundHandle, property, getExistingPropertySetterInvokeFlags());
+        return new SetMethod(boundHandle, property, getExistingSetterNonStrictFlag());
     }
 
-    private int getExistingPropertySetterInvokeFlags() {
+    private boolean getExistingSetterNonStrictFlag() {
         final ScriptFunction setter = find.getSetterFunction();
-        if (setter != null && setter.isStrict()) {
-            return NashornCallSiteDescriptor.CALLSITE_STRICT;
-        }
-        return 0;
+        return setter != null && setter.isNonStrictFunction();
     }
 
     private SetMethod createGlobalPropertySetter() {
         final ScriptObject global = Context.getGlobal();
-        return new SetMethod(ScriptObject.bindTo(global.addSpill(getName()), global), null, 0);
+        return new SetMethod(ScriptObject.bindTo(global.addSpill(getName()), global), null, false);
     }
 
     private SetMethod createNewPropertySetter() {
@@ -197,7 +192,7 @@
         final int nextSpill = getMap().getSpillLength();
 
         final Property property = createSpillProperty(nextSpill);
-        return new SetMethod(createSpillMethodHandle(nextSpill, property), property, 0);
+        return new SetMethod(createSpillMethodHandle(nextSpill, property), property, false);
     }
 
     private Property createSpillProperty(final int nextSpill) {
@@ -227,7 +222,7 @@
         final Property property = new SpillProperty(getName(), 0, nextEmbed, ScriptObject.GET_EMBED[nextEmbed], ScriptObject.SET_EMBED[nextEmbed]);
         //TODO specfields
         final MethodHandle methodHandle = MH.insertArguments(ScriptObject.SETEMBED, 0, desc, getMap(), getNewMap(property), property.getSetter(Object.class, getMap()), nextEmbed);
-        return new SetMethod(methodHandle, property, 0);
+        return new SetMethod(methodHandle, property, false);
     }
 
     private PropertyMap getNewMap(Property property) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornGuardedInvocation.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornGuardedInvocation.java	Tue Jan 22 14:14:37 2013 +0100
@@ -33,7 +33,8 @@
  * Guarded invocation with Nashorn specific bits.
  */
 public class NashornGuardedInvocation extends GuardedInvocation {
-    private final int flags;
+
+    private final boolean nonStrict;
 
     /**
      * Constructor
@@ -41,28 +42,28 @@
      * @param invocation  invocation target
      * @param switchPoint SwitchPoint that will, when invalidated, require relinking callsite, null if no SwitchPoint
      * @param guard       guard that will, when failed, require relinking callsite, null if no guard
-     * @param flags       callsite flags
+     * @param nonStrict   non-strict invocation target flag
      */
-    public NashornGuardedInvocation(final MethodHandle invocation, final SwitchPoint switchPoint, final MethodHandle guard, final int flags) {
+    public NashornGuardedInvocation(final MethodHandle invocation, final SwitchPoint switchPoint, final MethodHandle guard, final boolean nonStrict) {
         super(invocation, switchPoint, guard);
-        this.flags = flags;
+        this.nonStrict = nonStrict;
 
     }
 
     /**
-     * Is this invocation created from a callsite with strict semantics
-     * @return true if strict
+     * Is the target of this invocation a non-strict function?
+     * @return true if invocation target is non-strict
      */
-    public boolean isStrict() {
-        return (flags & NashornCallSiteDescriptor.CALLSITE_STRICT) != 0;
+    public boolean isNonStrict() {
+        return nonStrict;
     }
 
     /**
-     * Check whether a GuardedInvocation is created from a callsite with strict semantics
+     * Check whether the target of this invocation is a non-strict script function.
      * @param inv guarded invocation
-     * @return true if strict
+     * @return true if invocation target is non-strict
      */
-    public static boolean isStrict(final GuardedInvocation inv) {
-        return (inv instanceof NashornGuardedInvocation)? ((NashornGuardedInvocation)inv).isStrict() : false;
+    public static boolean isNonStrict(final GuardedInvocation inv) {
+        return inv instanceof NashornGuardedInvocation && ((NashornGuardedInvocation)inv).isNonStrict();
     }
 }
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/NashornLinker.java	Tue Jan 22 14:14:37 2013 +0100
@@ -29,6 +29,10 @@
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+
+import jdk.nashorn.internal.runtime.ConsString;
+import jdk.nashorn.internal.runtime.Context;
+import jdk.nashorn.internal.runtime.GlobalObject;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.Undefined;
@@ -71,9 +75,23 @@
             return null;
         }
 
-        final GuardedInvocation inv;
+        GuardedInvocation inv;
         if (self instanceof ScriptObject) {
             inv = ((ScriptObject)self).lookup(desc, request.isCallSiteUnstable());
+
+            if (self instanceof ScriptFunction && "call".equals(desc.getNameToken(CallSiteDescriptor.OPERATOR))) {
+
+                // Add toObject wrapper for non-object self argument to non-strict functions
+                assert request.getArguments().length >= 2 : "No self argument in script function callsite";
+                Object thisObject = request.getArguments()[1];
+                // Add wrapper filter for primitive this-object on non-strict functions
+                if (NashornGuardedInvocation.isNonStrict(inv) && isPrimitive(thisObject)) {
+                    MethodHandle wrapFilter = ((GlobalObject) Context.getGlobal()).getWrapFilter(thisObject);
+                    MethodHandle invocation = MH.filterArguments(inv.getInvocation(), 1,
+                            MH.asType(wrapFilter, wrapFilter.type().changeReturnType(Object.class)));
+                    inv = new GuardedInvocation(invocation, inv.getSwitchPoint(), inv.getGuard());
+                }
+            }
         } else if (self instanceof Undefined) {
             inv = Undefined.lookup(desc);
         } else {
@@ -135,6 +153,13 @@
                 JavaAdapterFactory.isAutoConvertibleFromFunction(clazz);
     }
 
+    private static boolean isPrimitive(Object obj) {
+        return obj instanceof Boolean ||
+               obj instanceof Number ||
+               obj instanceof String ||
+               obj instanceof ConsString;
+    }
+
     @Override
     public Comparison compareConversion(final Class<?> sourceType, final Class<?> targetType1, final Class<?> targetType2) {
         if(ScriptObject.class.isAssignableFrom(sourceType)) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/linker/PrimitiveLookup.java	Mon Jan 21 21:17:38 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/linker/PrimitiveLookup.java	Tue Jan 22 14:14:37 2013 +0100
@@ -94,11 +94,11 @@
         final GuardedInvocation link = wrappedReceiver.lookup(desc);
         if (link != null) {
             MethodHandle method = link.getInvocation();
-            if (!NashornGuardedInvocation.isStrict(link)) {
+            final Class<?> receiverType = method.type().parameterType(0);
+            if (receiverType != Object.class || NashornGuardedInvocation.isNonStrict(link)) {
                 final MethodType wrapType = wrapFilter.type();
-                final Class<?> methodReceiverType = method.type().parameterType(0);
-                assert methodReceiverType.isAssignableFrom(wrapType.returnType());
-                method = MH.filterArguments(method, 0, MH.asType(wrapFilter, wrapType.changeReturnType(methodReceiverType)));
+                assert receiverType.isAssignableFrom(wrapType.returnType());
+                method = MH.filterArguments(method, 0, MH.asType(wrapFilter, wrapType.changeReturnType(receiverType)));
             }
             return new GuardedInvocation(method, guard, link.getSwitchPoint());
         }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8006570.js	Tue Jan 22 14:14:37 2013 +0100
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2010, 2013, 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-8006570 : this-value for non-strict functions should be converted to object
+ *
+ * @test
+ * @run
+ */
+
+var strict, nonstrict;
+
+nonstrict = Object.prototype.nonstrict = function nonstrict() {
+    print(typeof this, this instanceof Object);
+};
+
+(function() {
+    "use strict";
+    strict = Object.prototype.strict = function strict() {
+        print(typeof this, this instanceof Object);
+    };
+})();
+
+"foo".nonstrict();
+(1).nonstrict();
+true.nonstrict();
+nonstrict();
+nonstrict.call(null);
+nonstrict.call("foo");
+nonstrict.call(1);
+nonstrict.call(true);
+
+"foo".strict();
+(1).strict();
+true.strict();
+strict();
+strict.call(null);
+strict.call("foo");
+strict.call(1);
+strict.call(true);
\ No newline at end of file
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8006570.js.EXPECTED	Tue Jan 22 14:14:37 2013 +0100
@@ -0,0 +1,16 @@
+object true
+object true
+object true
+object true
+object true
+object true
+object true
+object true
+string false
+number false
+boolean false
+undefined false
+object false
+string false
+number false
+boolean false