8136544: Call site switching to megamorphic causes incorrect property read

Wed, 16 Sep 2015 16:26:30 +0530

author
sundar
date
Wed, 16 Sep 2015 16:26:30 +0530
changeset 1534
7b64298633b2
parent 1533
8edb98264b4f
child 1535
8f28ca037c57

8136544: Call site switching to megamorphic causes incorrect property read
Reviewed-by: attila, mhaupt

src/jdk/nashorn/internal/objects/NativeJavaImporter.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/NativeJavaPackage.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/ScriptObject.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/WithObject.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8136544.js file | annotate | diff | comparison | revisions
test/src/jdk/nashorn/api/scripting/test/ScopeTest.java file | annotate | diff | comparison | revisions
     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  }

mercurial