8160034: The `this` value in the `with` is broken by the repetition of a function call
Reviewed-by: attila, sundar
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java Wed Jul 27 15:53:41 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java Thu Jul 28 16:27:00 2016 +0200
@@ -3057,6 +3057,7 @@
LexicalScope(final Global global) {
super(global, PropertyMap.newMap());
+ setIsInternal();
}
@Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Wed Jul 27 15:53:41 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Thu Jul 28 16:27:00 2016 +0200
@@ -123,6 +123,9 @@
/** Is this a builtin object? */
public static final int IS_BUILTIN = 1 << 3;
+ /** Is this an internal object that should not be visible to scripts? */
+ public static final int IS_INTERNAL = 1 << 4;
+
/**
* Spill growth rate - by how many elements does {@link ScriptObject#primitiveSpill} and
* {@link ScriptObject#objectSpill} when full
@@ -1668,6 +1671,21 @@
}
/**
+ * Tag this script object as internal object that should not be visible to script code.
+ */
+ public final void setIsInternal() {
+ flags |= IS_INTERNAL;
+ }
+
+ /**
+ * Check if this script object is an internal object that should not be visible to script code.
+ * @return true if internal
+ */
+ public final boolean isInternal() {
+ return (flags & IS_INTERNAL) != 0;
+ }
+
+ /**
* Clears the properties from a ScriptObject
* (java.util.Map-like method to help ScriptObjectMirror implementation)
*
@@ -2045,7 +2063,13 @@
private Object megamorphicGet(final String key, final boolean isMethod, final boolean isScope) {
final FindProperty find = findProperty(key, true, isScope, this);
if (find != null) {
- return find.getObjectValue();
+ // If this is a method invocation, and found property has a different self object then this,
+ // then return a function bound to the self object. This is the case for functions in with expressions.
+ final Object value = find.getObjectValue();
+ if (isMethod && value instanceof ScriptFunction && find.getSelf() != this && !find.getSelf().isInternal()) {
+ return ((ScriptFunction) value).createBound(find.getSelf(), ScriptRuntime.EMPTY_ARRAY);
+ }
+ return value;
}
return isMethod ? getNoSuchMethod(key, isScope, INVALID_PROGRAM_POINT) : invokeNoSuchProperty(key, isScope, INVALID_PROGRAM_POINT);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java Wed Jul 27 15:53:41 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java Thu Jul 28 16:27:00 2016 +0200
@@ -66,6 +66,7 @@
WithObject(final ScriptObject scope, final ScriptObject expression) {
super(scope, null);
this.expression = expression;
+ setIsInternal();
}
/**
@@ -99,37 +100,23 @@
// With scopes can never be observed outside of Nashorn code, so all call sites that can address it will of
// necessity have a Nashorn descriptor - it is safe to cast.
final NashornCallSiteDescriptor ndesc = (NashornCallSiteDescriptor)desc;
- FindProperty find = null;
GuardedInvocation link = null;
- ScriptObject self;
-
- final boolean isNamedOperation;
- final String name;
final Operation op = desc.getOperation();
- if (op instanceof NamedOperation) {
- isNamedOperation = true;
- name = ((NamedOperation)op).getName().toString();
- } else {
- isNamedOperation = false;
- name = null;
- }
- self = expression;
- if (isNamedOperation) {
- find = self.findProperty(name, true);
- }
+ assert op instanceof NamedOperation; // WithObject is a scope object, access is always named
+ final String name = ((NamedOperation)op).getName().toString();
+
+ FindProperty find = expression.findProperty(name, true);
if (find != null) {
- link = self.lookup(desc, request);
+ link = expression.lookup(desc, request);
if (link != null) {
return fixExpressionCallSite(ndesc, link);
}
}
final ScriptObject scope = getProto();
- if (isNamedOperation) {
- find = scope.findProperty(name, true);
- }
+ find = scope.findProperty(name, true);
if (find != null) {
return fixScopeCallSite(scope.lookup(desc, request), name, find.getOwner());
@@ -137,43 +124,41 @@
// the property is not found - now check for
// __noSuchProperty__ and __noSuchMethod__ in expression
- if (self != null) {
- final String fallBack;
+ final String fallBack;
- final StandardOperation firstOp = ndesc.getFirstOperation();
- switch (firstOp) {
- case GET_METHOD:
- fallBack = NO_SUCH_METHOD_NAME;
- break;
- case GET_PROPERTY:
- case GET_ELEMENT:
- fallBack = NO_SUCH_PROPERTY_NAME;
- break;
- default:
- fallBack = null;
- break;
- }
+ final StandardOperation firstOp = ndesc.getFirstOperation();
+ switch (firstOp) {
+ case GET_METHOD:
+ fallBack = NO_SUCH_METHOD_NAME;
+ break;
+ case GET_PROPERTY:
+ case GET_ELEMENT:
+ fallBack = NO_SUCH_PROPERTY_NAME;
+ break;
+ default:
+ fallBack = null;
+ break;
+ }
- if (fallBack != null) {
- find = self.findProperty(fallBack, true);
- if (find != null) {
- switch (firstOp) {
- case GET_METHOD:
- link = self.noSuchMethod(desc, request);
- break;
- case GET_PROPERTY:
- case GET_ELEMENT:
- link = self.noSuchProperty(desc, request);
- break;
- default:
- break;
- }
+ if (fallBack != null) {
+ find = expression.findProperty(fallBack, true);
+ if (find != null) {
+ switch (firstOp) {
+ case GET_METHOD:
+ link = expression.noSuchMethod(desc, request);
+ break;
+ case GET_PROPERTY:
+ case GET_ELEMENT:
+ link = expression.noSuchProperty(desc, request);
+ break;
+ default:
+ break;
}
}
+ }
- if (link != null) {
- return fixExpressionCallSite(ndesc, link);
- }
+ if (link != null) {
+ return fixExpressionCallSite(ndesc, link);
}
// still not found, may be scope can handle with it's own
@@ -204,7 +189,7 @@
// (as opposed from another non-scope object in the proto chain such as Object.prototype).
final FindProperty exprProperty = expression.findProperty(key, true, false, expression);
if (exprProperty != null) {
- return exprProperty;
+ return exprProperty;
}
return super.findProperty(key, deep, isScope, start);
}
@@ -295,14 +280,14 @@
private GuardedInvocation fixScopeCallSite(final GuardedInvocation link, final String name, final ScriptObject owner) {
final GuardedInvocation newLink = fixReceiverType(link, WITHSCOPEFILTER);
final MethodHandle expressionGuard = expressionGuard(name, owner);
- final MethodHandle filterGuardReceiver = filterGuardReceiver(newLink, WITHSCOPEFILTER);
+ final MethodHandle filteredGuard = filterGuardReceiver(newLink, WITHSCOPEFILTER);
return link.replaceMethods(
filterReceiver(
newLink.getInvocation(),
WITHSCOPEFILTER),
NashornGuards.combineGuards(
expressionGuard,
- filterGuardReceiver));
+ filteredGuard));
}
private static MethodHandle filterGuardReceiver(final GuardedInvocation link, final MethodHandle receiverFilter) {
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornGuards.java Wed Jul 27 15:53:41 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornGuards.java Thu Jul 28 16:27:00 2016 +0200
@@ -140,7 +140,7 @@
if (!needsGuard(property, desc)) {
return null;
}
- if (NashornCallSiteDescriptor.isScope(desc)) {
+ if (NashornCallSiteDescriptor.isScope(desc) && sobj.isScope()) {
if (property != null && property.isBound() && !property.canChangeType()) {
// This is a declared top level variables in main script or eval, use identity guard.
return getIdentityGuard(sobj);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8160034.js Thu Jul 28 16:27:00 2016 +0200
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2016, 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-8160034.js: The `this` value in the `with` is broken by the repetition of a function call
+ *
+ * @test
+ * @option --unstable-relink-threshold=4
+ * @run
+ */
+
+
+var bar = "BAR";
+
+function Foo() {
+ this.bar = "bar";
+ this.baz = "baz";
+}
+
+function foo_proto_h() {
+ print(this.bar);
+ delete Foo.prototype._h;
+}
+
+function foo_proto_e() {
+ print(this.baz);
+}
+
+function _h() {
+ print(this.bar);
+ Foo.prototype._h = foo_proto_h;
+}
+
+Foo.prototype._e = foo_proto_e;
+Foo.prototype._h = foo_proto_h;
+
+
+var fn = new Function("with(this) { _h(); _e(); }");
+
+for (var i = 0; i < 20; i++) {
+ fn.call(new Foo());
+}
+
+for (var i = 0; i < 20; i++) {
+ foo = new Foo();
+ foo['e' + Math.random()] = 1; // force new map
+ fn.call(foo);
+}
+
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8160034.js.EXPECTED Thu Jul 28 16:27:00 2016 +0200
@@ -0,0 +1,80 @@
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz
+bar
+baz
+BAR
+baz