8136544: Call site switching to megamorphic causes incorrect property read
Reviewed-by: attila, mhaupt
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeJavaImporter.java Tue Sep 15 19:31:24 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/NativeJavaImporter.java Wed Sep 16 16:26:30 2015 +0530
@@ -134,7 +134,7 @@
}
@Override
- protected Object invokeNoSuchProperty(final String name, final int programPoint) {
+ protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) {
final Object retval = createProperty(name);
if (isValid(programPoint)) {
throw new UnwarrantedOptimismException(retval, programPoint);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/NativeJavaPackage.java Tue Sep 15 19:31:24 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/NativeJavaPackage.java Wed Sep 16 16:26:30 2015 +0530
@@ -206,7 +206,7 @@
}
@Override
- protected Object invokeNoSuchProperty(final String key, final int programPoint) {
+ protected Object invokeNoSuchProperty(final String key, final boolean isScope, final int programPoint) {
final Object retval = createProperty(key);
if (isValid(programPoint)) {
throw new UnwarrantedOptimismException(retval, programPoint);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Tue Sep 15 19:31:24 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Wed Sep 16 16:26:30 2015 +0530
@@ -149,7 +149,7 @@
/** Method handle to retrieve prototype of this object */
public static final MethodHandle GETPROTO = findOwnMH_V("getProto", ScriptObject.class);
- static final MethodHandle MEGAMORPHIC_GET = findOwnMH_V("megamorphicGet", Object.class, String.class, boolean.class);
+ static final MethodHandle MEGAMORPHIC_GET = findOwnMH_V("megamorphicGet", Object.class, String.class, boolean.class, boolean.class);
static final MethodHandle GLOBALFILTER = findOwnMH_S("globalFilter", Object.class, Object.class);
static final MethodHandle DECLARE_AND_SET = findOwnMH_V("declareAndSet", void.class, String.class, Object.class);
@@ -2035,19 +2035,19 @@
private static GuardedInvocation findMegaMorphicGetMethod(final CallSiteDescriptor desc, final String name, final boolean isMethod) {
Context.getContextTrusted().getLogger(ObjectClassGenerator.class).warning("Megamorphic getter: " + desc + " " + name + " " +isMethod);
- final MethodHandle invoker = MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod);
+ final MethodHandle invoker = MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod, NashornCallSiteDescriptor.isScope(desc));
final MethodHandle guard = getScriptObjectGuard(desc.getMethodType(), true);
return new GuardedInvocation(invoker, guard);
}
@SuppressWarnings("unused")
- private Object megamorphicGet(final String key, final boolean isMethod) {
+ private Object megamorphicGet(final String key, final boolean isMethod, final boolean isScope) {
final FindProperty find = findProperty(key, true);
if (find != null) {
return find.getObjectValue();
}
- return isMethod ? getNoSuchMethod(key, INVALID_PROGRAM_POINT) : invokeNoSuchProperty(key, INVALID_PROGRAM_POINT);
+ return isMethod ? getNoSuchMethod(key, isScope, INVALID_PROGRAM_POINT) : invokeNoSuchProperty(key, isScope, INVALID_PROGRAM_POINT);
}
// Marks a property as declared and sets its value. Used as slow path for block-scoped LET and CONST
@@ -2382,10 +2382,11 @@
/**
* Invoke fall back if a property is not found.
* @param name Name of property.
+ * @param isScope is this a scope access?
* @param programPoint program point
* @return Result from call.
*/
- protected Object invokeNoSuchProperty(final String name, final int programPoint) {
+ protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) {
final FindProperty find = findProperty(NO_SUCH_PROPERTY_NAME, true);
Object ret = UNDEFINED;
@@ -2394,7 +2395,9 @@
final Object func = find.getObjectValue();
if (func instanceof ScriptFunction) {
- ret = ScriptRuntime.apply((ScriptFunction)func, this, name);
+ final ScriptFunction sfunc = (ScriptFunction)func;
+ final Object self = isScope && sfunc.isStrict()? UNDEFINED : this;
+ ret = ScriptRuntime.apply(sfunc, self, name);
}
}
@@ -2409,13 +2412,14 @@
/**
* Get __noSuchMethod__ as a function bound to this object and {@code name} if it is defined.
* @param name the method name
+ * @param isScope is this a scope access?
* @return the bound function, or undefined
*/
- private Object getNoSuchMethod(final String name, final int programPoint) {
+ private Object getNoSuchMethod(final String name, final boolean isScope, final int programPoint) {
final FindProperty find = findProperty(NO_SUCH_METHOD_NAME, true);
if (find == null) {
- return invokeNoSuchProperty(name, programPoint);
+ return invokeNoSuchProperty(name, isScope, programPoint);
}
final Object value = find.getObjectValue();
@@ -2423,7 +2427,9 @@
return UNDEFINED;
}
- return ((ScriptFunction)value).createBound(this, new Object[] {name});
+ final ScriptFunction func = (ScriptFunction)value;
+ final Object self = isScope && func.isStrict()? UNDEFINED : this;
+ return func.createBound(self, new Object[] {name});
}
private GuardedInvocation createEmptyGetter(final CallSiteDescriptor desc, final boolean explicitInstanceOfCheck, final String name) {
@@ -2738,7 +2744,7 @@
}
}
- return JSType.toInt32(invokeNoSuchProperty(key, programPoint));
+ return JSType.toInt32(invokeNoSuchProperty(key, false, programPoint));
}
@Override
@@ -2820,7 +2826,7 @@
}
}
- return JSType.toLong(invokeNoSuchProperty(key, programPoint));
+ return JSType.toLong(invokeNoSuchProperty(key, false, programPoint));
}
@Override
@@ -2902,7 +2908,7 @@
}
}
- return JSType.toNumber(invokeNoSuchProperty(key, INVALID_PROGRAM_POINT));
+ return JSType.toNumber(invokeNoSuchProperty(key, false, INVALID_PROGRAM_POINT));
}
@Override
@@ -2983,7 +2989,7 @@
}
}
- return invokeNoSuchProperty(key, INVALID_PROGRAM_POINT);
+ return invokeNoSuchProperty(key, false, INVALID_PROGRAM_POINT);
}
@Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java Tue Sep 15 19:31:24 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java Wed Sep 16 16:26:30 2015 +0530
@@ -26,6 +26,7 @@
package jdk.nashorn.internal.runtime;
import static jdk.nashorn.internal.lookup.Lookup.MH;
+import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
@@ -209,16 +210,18 @@
}
@Override
- protected Object invokeNoSuchProperty(final String name, final int programPoint) {
+ protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) {
FindProperty find = expression.findProperty(NO_SUCH_PROPERTY_NAME, true);
if (find != null) {
final Object func = find.getObjectValue();
if (func instanceof ScriptFunction) {
- return ScriptRuntime.apply((ScriptFunction)func, expression, name);
+ final ScriptFunction sfunc = (ScriptFunction)func;
+ final Object self = isScope && sfunc.isStrict()? UNDEFINED : expression;
+ return ScriptRuntime.apply(sfunc, self, name);
}
}
- return getProto().invokeNoSuchProperty(name, programPoint);
+ return getProto().invokeNoSuchProperty(name, isScope, programPoint);
}
@Override
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8136544.js Wed Sep 16 16:26:30 2015 +0530
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2015, 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-8136544: Call site switching to megamorphic causes incorrect property read
+ *
+ * @test
+ * @fork
+ * @option -Dnashorn.unstable.relink.threshold=8
+ * @run
+ */
+
+var ScriptContext = Java.type("javax.script.ScriptContext");
+var ScriptEngineManager = Java.type("javax.script.ScriptEngineManager");
+var m = new ScriptEngineManager();
+var e = m.getEngineByName("nashorn");
+
+var scope = e.getBindings(ScriptContext.ENGINE_SCOPE);
+var MYVAR = "myvar";
+
+function loopupVar() {
+ try {
+ e.eval(MYVAR);
+ return true;
+ } catch (e) {
+ return false;
+ }
+}
+
+// make sure we exercise callsite beyond megamorphic threshold we set
+// in this test via nashorn.unstable.relink.threshold property
+// In each iteration, callsite is exercised twice (two evals)
+// So, LIMIT should be more than 4 to exercise megamorphic callsites.
+
+var LIMIT = 5; // This LIMIT should be more than 4
+
+for (var i = 0; i < LIMIT; i++) {
+ // remove the variable and lookup
+ delete scope[MYVAR];
+ Assert.assertFalse(loopupVar(), "Expected true in iteration " + i);
+
+ // set that variable and check again
+ scope[MYVAR] = "foo";
+ Assert.assertTrue(loopupVar(), "Expected false in iteration " + i);
+}
--- a/nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Tue Sep 15 19:31:24 2015 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Wed Sep 16 16:26:30 2015 +0530
@@ -26,6 +26,7 @@
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
import javax.script.Bindings;
@@ -820,4 +821,38 @@
public void recursiveEvalCallScriptContextTest() throws ScriptException {
new RecursiveEval().program();
}
+
+ private static final String VAR_NAME = "myvar";
+
+ private static boolean lookupVar(final ScriptEngine engine, final String varName) {
+ try {
+ engine.eval(varName);
+ return true;
+ } catch (final ScriptException se) {
+ return false;
+ }
+ }
+
+ // @bug 8136544: Call site switching to megamorphic causes incorrect property read
+ @Test
+ public void megamorphicPropertyReadTest() throws ScriptException {
+ final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+ final ScriptEngine engine = factory.getScriptEngine();
+ final Bindings scope = engine.getBindings(ScriptContext.ENGINE_SCOPE);
+ boolean ret;
+
+ // Why 16 is the upper limit of this loop? The default nashorn dynalink megamorphic threshold is 16.
+ // See jdk.nashorn.internal.runtime.linker.Bootstrap.NASHORN_DEFAULT_UNSTABLE_RELINK_THRESHOLD
+ // We do, 'eval' of the same in this loop twice. So, 16*2 = 32 times that callsite in the script
+ // is exercised - much beyond the default megamorphic threshold.
+
+ for (int i = 0; i < 16; i++) {
+ scope.remove(VAR_NAME);
+ ret = lookupVar(engine, VAR_NAME);
+ assertFalse(ret, "Expected false in iteration " + i);
+ scope.put(VAR_NAME, "foo");
+ ret = lookupVar(engine, VAR_NAME);
+ assertTrue(ret, "Expected true in iteration " + i);
+ }
+ }
}