8029667: Prototype linking is incorrect

Reviewed-by: jlaskey, sundar

nashorn/src/jdk/nashorn/internal/runtime/FindProperty.java

@@ -112,7 +112,7 @@ public final class FindProperty {
         return property != null && property.hasGetterFunction(prototype) ? self : prototype;
     }
 
-   /**
+    /**
      * Return the appropriate receiver for a setter.
      * @return appropriate receiver
      */
@@ -172,5 +172,20 @@ public final class FindProperty {
         property.setObjectValue(getSetterReceiver(), getOwner(), value, strict);
     }
 
+    /**
+     * Get the number of objects in the prototype chain between the {@code self} and the
+     * {@code owner} objects.
+     * @return the prototype chain length
+     */
+    int getProtoChainLength() {
+        assert self != null;
+        int length = 0;
+        for (ScriptObject obj = self; obj != prototype; obj = obj.getProto()) {
+            assert !(obj instanceof WithObject);
+            ++length;
+        }
+        return length;
+    }
+
 }
 

nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java

@@ -143,6 +143,8 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
     private static final MethodHandle TRUNCATINGFILTER   = findOwnMH("truncatingFilter", Object[].class, int.class, Object[].class);
     private static final MethodHandle KNOWNFUNCPROPGUARD = findOwnMH("knownFunctionPropertyGuard", boolean.class, Object.class, PropertyMap.class, MethodHandle.class, Object.class, ScriptFunction.class);
 
+    private static final ArrayList<MethodHandle> protoFilters = new ArrayList<>();
+
     /** Method handle for getting a function argument at a given index. Used from MapCreator */
     public static final Call GET_ARGUMENT       = virtualCall(MethodHandles.lookup(), ScriptObject.class, "getArgument", Object.class, int.class);
 
@@ -1711,6 +1713,44 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
         return getter.replaceMethods(MH.foldArguments(invoker, argDroppingGetter), getter.getGuard());
     }
 
+    /**
+     * Test whether this object contains in its prototype chain or is itself a with-object.
+     * @return true if a with-object was found
+     */
+    final boolean hasWithScope() {
+        if (isScope()) {
+            for (ScriptObject obj = this; obj != null; obj = obj.getProto()) {
+                if (obj instanceof WithObject) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Add a filter to the first argument of {@code methodHandle} that calls its {@link #getProto()} method
+     * {@code depth} times.
+     * @param methodHandle a method handle
+     * @param depth        distance to target prototype
+     * @return the filtered method handle
+     */
+    static MethodHandle addProtoFilter(final MethodHandle methodHandle, final int depth) {
+        if (depth == 0) {
+            return methodHandle;
+        }
+        final int listIndex = depth - 1; // We don't need 0-deep walker
+        MethodHandle filter = listIndex < protoFilters.size() ? protoFilters.get(listIndex) : null;
+
+        if(filter == null) {
+            filter = addProtoFilter(GETPROTO, depth - 1);
+            protoFilters.add(null);
+            protoFilters.set(listIndex, filter);
+        }
+
+        return MH.filterArguments(methodHandle, 0, filter.asType(filter.type().changeReturnType(methodHandle.type().parameterType(0))));
+    }
+
     /**
      * Find the appropriate GET method for an invoke dynamic call.
      *
@@ -1722,7 +1762,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      */
     protected GuardedInvocation findGetMethod(final CallSiteDescriptor desc, final LinkRequest request, final String operator) {
         final String name = desc.getNameToken(CallSiteDescriptor.NAME_OPERAND);
-        if (request.isCallSiteUnstable()) {
+        if (request.isCallSiteUnstable() || hasWithScope()) {
             return findMegaMorphicGetMethod(desc, name, "getMethod".equals(operator));
         }
 
@@ -1748,22 +1788,24 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
         final Property property = find.getProperty();
         methodHandle = find.getGetter(returnType);
 
+        final boolean noGuard = ObjectClassGenerator.OBJECT_FIELDS_ONLY && NashornCallSiteDescriptor.isFastScope(desc) && !property.canChangeType();
         // getMap() is fine as we have the prototype switchpoint depending on where the property was found
-        final MethodHandle guard = NashornGuards.getMapGuard(getMap());
+        final MethodHandle guard = noGuard ? null : NashornGuards.getMapGuard(getMap());
 
         if (methodHandle != null) {
             assert methodHandle.type().returnType().equals(returnType);
             if (find.isSelf()) {
-                return new GuardedInvocation(methodHandle, ObjectClassGenerator.OBJECT_FIELDS_ONLY &&
-                        NashornCallSiteDescriptor.isFastScope(desc) && !property.canChangeType() ? null : guard);
+                return new GuardedInvocation(methodHandle, guard);
             }
 
-            final ScriptObject prototype = find.getOwner();
-
-            if (!property.hasGetterFunction(prototype)) {
-                methodHandle = bindTo(methodHandle, prototype);
+            if (!property.hasGetterFunction(find.getOwner())) {
+                // If not a scope bind to actual prototype as changing prototype will change the property map.
+                // For scopes we install a filter that replaces the self object with the prototype owning the property.
+                methodHandle = isScope() ?
+                        addProtoFilter(methodHandle, find.getProtoChainLength()) :
+                        bindTo(methodHandle, find.getOwner());
             }
-            return new GuardedInvocation(methodHandle, getMap().getProtoGetSwitchPoint(proto, name), guard);
+            return new GuardedInvocation(methodHandle, noGuard ? null : getMap().getProtoGetSwitchPoint(proto, name), guard);
         }
 
         assert !NashornCallSiteDescriptor.isFastScope(desc);
@@ -1833,7 +1875,7 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      */
     protected GuardedInvocation findSetMethod(final CallSiteDescriptor desc, final LinkRequest request) {
         final String name = desc.getNameToken(CallSiteDescriptor.NAME_OPERAND);
-        if (request.isCallSiteUnstable()) {
+        if (request.isCallSiteUnstable() || hasWithScope()) {
             return findMegaMorphicSetMethod(desc, name);
         }
 
@@ -2761,7 +2803,8 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
     public final void setObject(final FindProperty find, final boolean strict, final String key, final Object value) {
         FindProperty f = find;
 
-        if (f != null && f.isInherited() && !(f.getProperty() instanceof UserAccessorProperty)) {
+        if (f != null && f.isInherited() && !(f.getProperty() instanceof UserAccessorProperty) && !isScope()) {
+            // Setting a property should not modify the property in prototype unless this is a scope object.
             f = null;
         }
 

nashorn/src/jdk/nashorn/internal/runtime/SetMethodCreator.java

@@ -151,10 +151,12 @@ final class SetMethodCreator {
         assert methodHandle != null;
         assert property     != null;
 
-        final ScriptObject prototype = find.getOwner();
         final MethodHandle boundHandle;
-        if (!property.hasSetterFunction(prototype) && find.isInherited()) {
-            boundHandle = ScriptObject.bindTo(methodHandle, prototype);
+        if (!property.hasSetterFunction(find.getOwner()) && find.isInherited()) {
+            // Bind or add prototype filter depending on whether this is a scope object.
+            boundHandle = sobj.isScope() ?
+                    ScriptObject.addProtoFilter(methodHandle, find.getProtoChainLength()):
+                    ScriptObject.bindTo(methodHandle, find.getOwner());
         } else {
             boundHandle = methodHandle;
         }

nashorn/test/script/basic/JDK-8029667.js

@@ -0,0 +1,91 @@
+/*
+ * 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-8029667: Prototype linking is incorrect
+ *
+ * @test
+ * @run
+ */
+
+function f(x) {
+  return (function inner() { 
+      var y; (function dummy() { return y })() // force own scope for the inner function
+      with({}) { // 'with' block turns off fast scopes
+          return x
+      }
+  })();
+}
+print(f(1));
+print(f(2));
+
+function g(x) { 
+  (function inner() { 
+      var y; (function dummy() { return y })() // force own scope for the inner function
+      with({}) { // 'with' block turns off fast scopes
+          // Test setter as well as getter
+          x = x + 2;
+      }
+  })();
+  print(x);
+}
+
+g(1);
+g(2);
+
+var withScopes = [{ func: function() { print("called 1");} }, { func: function() { print("called 2");} }];
+
+for(var i in withScopes) {
+    with (withScopes[i]) {
+        var main = function() {
+            var frame; // <---- this local variable caused scope to be not set properly prior to fix
+
+            function callFunc() {
+                frame = func();
+            }
+
+            callFunc();
+        }
+    }
+    main();
+}
+
+for(var i in withScopes) {
+    with (withScopes[i]) {
+        var main = function() {
+            var frame; // <---- this local variable caused scope to be not set properly prior to fix
+
+            function callFunc() {
+                frame = func = i;
+            }
+
+            callFunc();
+        }
+    }
+    main();
+} 
+
+print(withScopes[0].func);
+print(withScopes[1].func);
+
+

nashorn/test/script/basic/JDK-8029667.js.EXPECTED

@@ -0,0 +1,8 @@
+1
+2
+3
+4
+called 1
+called 2
+0
+1