Wed, 16 Sep 2015 16:26:30 +0530
8136544: Call site switching to megamorphic causes incorrect property read
Reviewed-by: attila, mhaupt
1.1 --- a/src/jdk/nashorn/internal/objects/NativeJavaImporter.java Tue Sep 15 19:31:24 2015 +0530 1.2 +++ b/src/jdk/nashorn/internal/objects/NativeJavaImporter.java Wed Sep 16 16:26:30 2015 +0530 1.3 @@ -134,7 +134,7 @@ 1.4 } 1.5 1.6 @Override 1.7 - protected Object invokeNoSuchProperty(final String name, final int programPoint) { 1.8 + protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) { 1.9 final Object retval = createProperty(name); 1.10 if (isValid(programPoint)) { 1.11 throw new UnwarrantedOptimismException(retval, programPoint);
2.1 --- a/src/jdk/nashorn/internal/runtime/NativeJavaPackage.java Tue Sep 15 19:31:24 2015 +0530 2.2 +++ b/src/jdk/nashorn/internal/runtime/NativeJavaPackage.java Wed Sep 16 16:26:30 2015 +0530 2.3 @@ -206,7 +206,7 @@ 2.4 } 2.5 2.6 @Override 2.7 - protected Object invokeNoSuchProperty(final String key, final int programPoint) { 2.8 + protected Object invokeNoSuchProperty(final String key, final boolean isScope, final int programPoint) { 2.9 final Object retval = createProperty(key); 2.10 if (isValid(programPoint)) { 2.11 throw new UnwarrantedOptimismException(retval, programPoint);
3.1 --- a/src/jdk/nashorn/internal/runtime/ScriptObject.java Tue Sep 15 19:31:24 2015 +0530 3.2 +++ b/src/jdk/nashorn/internal/runtime/ScriptObject.java Wed Sep 16 16:26:30 2015 +0530 3.3 @@ -149,7 +149,7 @@ 3.4 /** Method handle to retrieve prototype of this object */ 3.5 public static final MethodHandle GETPROTO = findOwnMH_V("getProto", ScriptObject.class); 3.6 3.7 - static final MethodHandle MEGAMORPHIC_GET = findOwnMH_V("megamorphicGet", Object.class, String.class, boolean.class); 3.8 + static final MethodHandle MEGAMORPHIC_GET = findOwnMH_V("megamorphicGet", Object.class, String.class, boolean.class, boolean.class); 3.9 static final MethodHandle GLOBALFILTER = findOwnMH_S("globalFilter", Object.class, Object.class); 3.10 static final MethodHandle DECLARE_AND_SET = findOwnMH_V("declareAndSet", void.class, String.class, Object.class); 3.11 3.12 @@ -2020,19 +2020,19 @@ 3.13 3.14 private static GuardedInvocation findMegaMorphicGetMethod(final CallSiteDescriptor desc, final String name, final boolean isMethod) { 3.15 Context.getContextTrusted().getLogger(ObjectClassGenerator.class).warning("Megamorphic getter: " + desc + " " + name + " " +isMethod); 3.16 - final MethodHandle invoker = MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod); 3.17 + final MethodHandle invoker = MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod, NashornCallSiteDescriptor.isScope(desc)); 3.18 final MethodHandle guard = getScriptObjectGuard(desc.getMethodType(), true); 3.19 return new GuardedInvocation(invoker, guard); 3.20 } 3.21 3.22 @SuppressWarnings("unused") 3.23 - private Object megamorphicGet(final String key, final boolean isMethod) { 3.24 + private Object megamorphicGet(final String key, final boolean isMethod, final boolean isScope) { 3.25 final FindProperty find = findProperty(key, true); 3.26 if (find != null) { 3.27 return find.getObjectValue(); 3.28 } 3.29 3.30 - return isMethod ? getNoSuchMethod(key, INVALID_PROGRAM_POINT) : invokeNoSuchProperty(key, INVALID_PROGRAM_POINT); 3.31 + return isMethod ? getNoSuchMethod(key, isScope, INVALID_PROGRAM_POINT) : invokeNoSuchProperty(key, isScope, INVALID_PROGRAM_POINT); 3.32 } 3.33 3.34 // Marks a property as declared and sets its value. Used as slow path for block-scoped LET and CONST 3.35 @@ -2367,10 +2367,11 @@ 3.36 /** 3.37 * Invoke fall back if a property is not found. 3.38 * @param name Name of property. 3.39 + * @param isScope is this a scope access? 3.40 * @param programPoint program point 3.41 * @return Result from call. 3.42 */ 3.43 - protected Object invokeNoSuchProperty(final String name, final int programPoint) { 3.44 + protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) { 3.45 final FindProperty find = findProperty(NO_SUCH_PROPERTY_NAME, true); 3.46 3.47 Object ret = UNDEFINED; 3.48 @@ -2379,7 +2380,9 @@ 3.49 final Object func = find.getObjectValue(); 3.50 3.51 if (func instanceof ScriptFunction) { 3.52 - ret = ScriptRuntime.apply((ScriptFunction)func, this, name); 3.53 + final ScriptFunction sfunc = (ScriptFunction)func; 3.54 + final Object self = isScope && sfunc.isStrict()? UNDEFINED : this; 3.55 + ret = ScriptRuntime.apply(sfunc, self, name); 3.56 } 3.57 } 3.58 3.59 @@ -2394,13 +2397,14 @@ 3.60 /** 3.61 * Get __noSuchMethod__ as a function bound to this object and {@code name} if it is defined. 3.62 * @param name the method name 3.63 + * @param isScope is this a scope access? 3.64 * @return the bound function, or undefined 3.65 */ 3.66 - private Object getNoSuchMethod(final String name, final int programPoint) { 3.67 + private Object getNoSuchMethod(final String name, final boolean isScope, final int programPoint) { 3.68 final FindProperty find = findProperty(NO_SUCH_METHOD_NAME, true); 3.69 3.70 if (find == null) { 3.71 - return invokeNoSuchProperty(name, programPoint); 3.72 + return invokeNoSuchProperty(name, isScope, programPoint); 3.73 } 3.74 3.75 final Object value = find.getObjectValue(); 3.76 @@ -2408,7 +2412,9 @@ 3.77 return UNDEFINED; 3.78 } 3.79 3.80 - return ((ScriptFunction)value).createBound(this, new Object[] {name}); 3.81 + final ScriptFunction func = (ScriptFunction)value; 3.82 + final Object self = isScope && func.isStrict()? UNDEFINED : this; 3.83 + return func.createBound(self, new Object[] {name}); 3.84 } 3.85 3.86 private GuardedInvocation createEmptyGetter(final CallSiteDescriptor desc, final boolean explicitInstanceOfCheck, final String name) { 3.87 @@ -2723,7 +2729,7 @@ 3.88 } 3.89 } 3.90 3.91 - return JSType.toInt32(invokeNoSuchProperty(key, programPoint)); 3.92 + return JSType.toInt32(invokeNoSuchProperty(key, false, programPoint)); 3.93 } 3.94 3.95 @Override 3.96 @@ -2805,7 +2811,7 @@ 3.97 } 3.98 } 3.99 3.100 - return JSType.toLong(invokeNoSuchProperty(key, programPoint)); 3.101 + return JSType.toLong(invokeNoSuchProperty(key, false, programPoint)); 3.102 } 3.103 3.104 @Override 3.105 @@ -2887,7 +2893,7 @@ 3.106 } 3.107 } 3.108 3.109 - return JSType.toNumber(invokeNoSuchProperty(key, INVALID_PROGRAM_POINT)); 3.110 + return JSType.toNumber(invokeNoSuchProperty(key, false, INVALID_PROGRAM_POINT)); 3.111 } 3.112 3.113 @Override 3.114 @@ -2968,7 +2974,7 @@ 3.115 } 3.116 } 3.117 3.118 - return invokeNoSuchProperty(key, INVALID_PROGRAM_POINT); 3.119 + return invokeNoSuchProperty(key, false, INVALID_PROGRAM_POINT); 3.120 } 3.121 3.122 @Override
4.1 --- a/src/jdk/nashorn/internal/runtime/WithObject.java Tue Sep 15 19:31:24 2015 +0530 4.2 +++ b/src/jdk/nashorn/internal/runtime/WithObject.java Wed Sep 16 16:26:30 2015 +0530 4.3 @@ -26,6 +26,7 @@ 4.4 package jdk.nashorn.internal.runtime; 4.5 4.6 import static jdk.nashorn.internal.lookup.Lookup.MH; 4.7 +import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED; 4.8 4.9 import java.lang.invoke.MethodHandle; 4.10 import java.lang.invoke.MethodHandles; 4.11 @@ -209,16 +210,18 @@ 4.12 } 4.13 4.14 @Override 4.15 - protected Object invokeNoSuchProperty(final String name, final int programPoint) { 4.16 + protected Object invokeNoSuchProperty(final String name, final boolean isScope, final int programPoint) { 4.17 FindProperty find = expression.findProperty(NO_SUCH_PROPERTY_NAME, true); 4.18 if (find != null) { 4.19 final Object func = find.getObjectValue(); 4.20 if (func instanceof ScriptFunction) { 4.21 - return ScriptRuntime.apply((ScriptFunction)func, expression, name); 4.22 + final ScriptFunction sfunc = (ScriptFunction)func; 4.23 + final Object self = isScope && sfunc.isStrict()? UNDEFINED : expression; 4.24 + return ScriptRuntime.apply(sfunc, self, name); 4.25 } 4.26 } 4.27 4.28 - return getProto().invokeNoSuchProperty(name, programPoint); 4.29 + return getProto().invokeNoSuchProperty(name, isScope, programPoint); 4.30 } 4.31 4.32 @Override
5.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 5.2 +++ b/test/script/basic/JDK-8136544.js Wed Sep 16 16:26:30 2015 +0530 5.3 @@ -0,0 +1,65 @@ 5.4 +/* 5.5 + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. 5.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 5.7 + * 5.8 + * This code is free software; you can redistribute it and/or modify it 5.9 + * under the terms of the GNU General Public License version 2 only, as 5.10 + * published by the Free Software Foundation. 5.11 + * 5.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 5.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 5.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 5.15 + * version 2 for more details (a copy is included in the LICENSE file that 5.16 + * accompanied this code). 5.17 + * 5.18 + * You should have received a copy of the GNU General Public License version 5.19 + * 2 along with this work; if not, write to the Free Software Foundation, 5.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 5.21 + * 5.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 5.23 + * or visit www.oracle.com if you need additional information or have any 5.24 + * questions. 5.25 + */ 5.26 + 5.27 +/** 5.28 + * JDK-8136544: Call site switching to megamorphic causes incorrect property read 5.29 + * 5.30 + * @test 5.31 + * @fork 5.32 + * @option -Dnashorn.unstable.relink.threshold=8 5.33 + * @run 5.34 + */ 5.35 + 5.36 +var ScriptContext = Java.type("javax.script.ScriptContext"); 5.37 +var ScriptEngineManager = Java.type("javax.script.ScriptEngineManager"); 5.38 +var m = new ScriptEngineManager(); 5.39 +var e = m.getEngineByName("nashorn"); 5.40 + 5.41 +var scope = e.getBindings(ScriptContext.ENGINE_SCOPE); 5.42 +var MYVAR = "myvar"; 5.43 + 5.44 +function loopupVar() { 5.45 + try { 5.46 + e.eval(MYVAR); 5.47 + return true; 5.48 + } catch (e) { 5.49 + return false; 5.50 + } 5.51 +} 5.52 + 5.53 +// make sure we exercise callsite beyond megamorphic threshold we set 5.54 +// in this test via nashorn.unstable.relink.threshold property 5.55 +// In each iteration, callsite is exercised twice (two evals) 5.56 +// So, LIMIT should be more than 4 to exercise megamorphic callsites. 5.57 + 5.58 +var LIMIT = 5; // This LIMIT should be more than 4 5.59 + 5.60 +for (var i = 0; i < LIMIT; i++) { 5.61 + // remove the variable and lookup 5.62 + delete scope[MYVAR]; 5.63 + Assert.assertFalse(loopupVar(), "Expected true in iteration " + i); 5.64 + 5.65 + // set that variable and check again 5.66 + scope[MYVAR] = "foo"; 5.67 + Assert.assertTrue(loopupVar(), "Expected false in iteration " + i); 5.68 +}
6.1 --- a/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Tue Sep 15 19:31:24 2015 +0530 6.2 +++ b/test/src/jdk/nashorn/api/scripting/test/ScopeTest.java Wed Sep 16 16:26:30 2015 +0530 6.3 @@ -26,6 +26,7 @@ 6.4 6.5 import static org.testng.Assert.assertEquals; 6.6 import static org.testng.Assert.assertNotNull; 6.7 +import static org.testng.Assert.assertFalse; 6.8 import static org.testng.Assert.assertTrue; 6.9 import static org.testng.Assert.fail; 6.10 import javax.script.Bindings; 6.11 @@ -820,4 +821,38 @@ 6.12 public void recursiveEvalCallScriptContextTest() throws ScriptException { 6.13 new RecursiveEval().program(); 6.14 } 6.15 + 6.16 + private static final String VAR_NAME = "myvar"; 6.17 + 6.18 + private static boolean lookupVar(final ScriptEngine engine, final String varName) { 6.19 + try { 6.20 + engine.eval(varName); 6.21 + return true; 6.22 + } catch (final ScriptException se) { 6.23 + return false; 6.24 + } 6.25 + } 6.26 + 6.27 + // @bug 8136544: Call site switching to megamorphic causes incorrect property read 6.28 + @Test 6.29 + public void megamorphicPropertyReadTest() throws ScriptException { 6.30 + final NashornScriptEngineFactory factory = new NashornScriptEngineFactory(); 6.31 + final ScriptEngine engine = factory.getScriptEngine(); 6.32 + final Bindings scope = engine.getBindings(ScriptContext.ENGINE_SCOPE); 6.33 + boolean ret; 6.34 + 6.35 + // Why 16 is the upper limit of this loop? The default nashorn dynalink megamorphic threshold is 16. 6.36 + // See jdk.nashorn.internal.runtime.linker.Bootstrap.NASHORN_DEFAULT_UNSTABLE_RELINK_THRESHOLD 6.37 + // We do, 'eval' of the same in this loop twice. So, 16*2 = 32 times that callsite in the script 6.38 + // is exercised - much beyond the default megamorphic threshold. 6.39 + 6.40 + for (int i = 0; i < 16; i++) { 6.41 + scope.remove(VAR_NAME); 6.42 + ret = lookupVar(engine, VAR_NAME); 6.43 + assertFalse(ret, "Expected false in iteration " + i); 6.44 + scope.put(VAR_NAME, "foo"); 6.45 + ret = lookupVar(engine, VAR_NAME); 6.46 + assertTrue(ret, "Expected true in iteration " + i); 6.47 + } 6.48 + } 6.49 }