changeset 931:d714b9555e5d

8047067: all eval arguments need to be copied in Lower Reviewed-by: lagergren, sundar
author attila
date Tue, 08 Jul 2014 13:13:31 +0200
parents 40995fbd176b
children aa3fda2d2967
files src/jdk/nashorn/internal/codegen/CodeGenerator.java src/jdk/nashorn/internal/codegen/Lower.java src/jdk/nashorn/internal/ir/CallNode.java src/jdk/nashorn/internal/objects/Global.java test/script/basic/JDK-8047057.js test/script/basic/JDK-8047067.js
diffstat 6 files changed, 72 insertions(+), 75 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Tue Jul 08 16:30:42 2014 +0530
+++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java	Tue Jul 08 13:13:31 2014 +0200
@@ -1317,20 +1317,14 @@
 
                         // Load up self (scope).
                         method.loadCompilerConstant(SCOPE);
-                        final CallNode.EvalArgs evalArgs = callNode.getEvalArgs();
+                        final List<Expression> evalArgs = callNode.getEvalArgs().getArgs();
                         // load evaluated code
-                        loadExpressionAsObject(evalArgs.getCode());
+                        loadExpressionAsObject(evalArgs.get(0));
                         // load second and subsequent args for side-effect
-                        final List<Expression> callArgs = callNode.getArgs();
-                        final int numArgs = callArgs.size();
+                        final int numArgs = evalArgs.size();
                         for (int i = 1; i < numArgs; i++) {
-                            loadExpressionUnbounded(callArgs.get(i)).pop();
+                            loadAndDiscard(evalArgs.get(i));
                         }
-                        // special/extra 'eval' arguments
-                        loadExpressionUnbounded(evalArgs.getThis());
-                        method.load(evalArgs.getLocation());
-                        method.load(evalArgs.getStrictMode());
-                        method.convert(Type.OBJECT);
                         method._goto(invoke_direct_eval);
 
                         method.label(is_not_eval);
@@ -1339,7 +1333,7 @@
                         // This is some scope 'eval' or global eval replaced by user
                         // but not the built-in ECMAScript 'eval' function call
                         method.loadNull();
-                        argsCount = loadArgs(callArgs);
+                        argsCount = loadArgs(callNode.getArgs());
                     }
 
                     @Override
@@ -1349,6 +1343,11 @@
                         method._goto(eval_done);
 
                         method.label(invoke_direct_eval);
+                        // Special/extra 'eval' arguments. These can be loaded late (in consumeStack) as we know none of
+                        // them can ever be optimistic.
+                        method.loadCompilerConstant(THIS);
+                        method.load(callNode.getEvalArgs().getLocation());
+                        method.load(CodeGenerator.this.lc.getCurrentFunction().isStrict());
                         // direct call to Global.directEval
                         globalDirectEval();
                         convertOptimisticReturnValue();
@@ -4438,7 +4437,7 @@
 
     private MethodEmitter globalDirectEval() {
         return method.invokestatic(GLOBAL_OBJECT, "directEval",
-                methodDescriptor(Object.class, Object.class, Object.class, Object.class, Object.class, Object.class));
+                methodDescriptor(Object.class, Object.class, Object.class, Object.class, Object.class, boolean.class));
     }
 
     private abstract class OptimisticOperation {
--- a/src/jdk/nashorn/internal/codegen/Lower.java	Tue Jul 08 16:30:42 2014 +0530
+++ b/src/jdk/nashorn/internal/codegen/Lower.java	Tue Jul 08 13:13:31 2014 +0200
@@ -27,7 +27,6 @@
 
 import static jdk.nashorn.internal.codegen.CompilerConstants.EVAL;
 import static jdk.nashorn.internal.codegen.CompilerConstants.RETURN;
-import static jdk.nashorn.internal.codegen.CompilerConstants.THIS;
 import static jdk.nashorn.internal.ir.Expression.isAlwaysTrue;
 
 import java.util.ArrayList;
@@ -603,13 +602,11 @@
 
             // 'eval' call with at least one argument
             if (args.size() >= 1 && EVAL.symbolName().equals(callee.getName())) {
-                final FunctionNode currentFunction = lc.getCurrentFunction();
-                return callNode.setEvalArgs(
-                    new CallNode.EvalArgs(
-                        (Expression)ensureUniqueNamesIn(args.get(0)).accept(this),
-                        compilerConstant(THIS),
-                        evalLocation(callee),
-                        currentFunction.isStrict()));
+                final List<Expression> evalArgs = new ArrayList<>(args.size());
+                for(final Expression arg: args) {
+                    evalArgs.add((Expression)ensureUniqueNamesIn(arg).accept(this));
+                }
+                return callNode.setEvalArgs(new CallNode.EvalArgs(evalArgs, evalLocation(callee)));
             }
         }
 
--- a/src/jdk/nashorn/internal/ir/CallNode.java	Tue Jul 08 16:30:42 2014 +0530
+++ b/src/jdk/nashorn/internal/ir/CallNode.java	Tue Jul 08 13:13:31 2014 +0200
@@ -65,61 +65,35 @@
      * Arguments to be passed to builtin {@code eval} function
      */
     public static class EvalArgs {
-        /** evaluated code */
-        private final Expression code;
-
-        /** 'this' passed to evaluated code */
-        private final IdentNode evalThis;
+        private final List<Expression> args;
 
         /** location string for the eval call */
         private final String location;
 
-        /** is this call from a strict context? */
-        private final boolean strictMode;
-
         /**
          * Constructor
          *
-         * @param code       code to evaluate
-         * @param evalThis   this node
-         * @param location   location for the eval call
-         * @param strictMode is this a call from a strict context?
+         * @param args     arguments to eval
+         * @param location location for the eval call
          */
-        public EvalArgs(final Expression code, final IdentNode evalThis, final String location, final boolean strictMode) {
-            this.code = code;
-            this.evalThis = evalThis;
+        public EvalArgs(final List<Expression> args, final String location) {
+            this.args = args;
             this.location = location;
-            this.strictMode = strictMode;
         }
 
         /**
          * Return the code that is to be eval:ed by this eval function
          * @return code as an AST node
          */
-        public Expression getCode() {
-            return code;
+        public List<Expression> getArgs() {
+            return Collections.unmodifiableList(args);
         }
 
-        private EvalArgs setCode(final Expression code) {
-            if (this.code == code) {
+        private EvalArgs setArgs(final List<Expression> args) {
+            if (this.args == args) {
                 return this;
             }
-            return new EvalArgs(code, evalThis, location, strictMode);
-        }
-
-        /**
-         * Get the {@code this} symbol used to invoke this eval call
-         * @return the {@code this} symbol
-         */
-        public IdentNode getThis() {
-            return this.evalThis;
-        }
-
-        private EvalArgs setThis(final IdentNode evalThis) {
-            if (this.evalThis == evalThis) {
-                return this;
-            }
-            return new EvalArgs(code, evalThis, location, strictMode);
+            return new EvalArgs(args, location);
         }
 
         /**
@@ -129,14 +103,6 @@
         public String getLocation() {
             return this.location;
         }
-
-        /**
-         * Check whether this eval call is executed in strict mode
-         * @return true if executed in strict mode, false otherwise
-         */
-        public boolean getStrictMode() {
-            return this.strictMode;
-        }
     }
 
     /** arguments for 'eval' call. Non-null only if this call node is 'eval' */
@@ -212,8 +178,7 @@
                     setArgs(Node.accept(visitor, Expression.class, args)).
                     setEvalArgs(evalArgs == null ?
                             null :
-                            evalArgs.setCode((Expression)evalArgs.getCode().accept(visitor)).
-                                setThis((IdentNode)evalArgs.getThis().accept(visitor))));
+                            evalArgs.setArgs(Node.accept(visitor, Expression.class, evalArgs.getArgs()))));
             // Theoretically, we'd need to instead pass lc to every setter and do a replacement on each. In practice,
             // setType from TypeOverride can't accept a lc, and we don't necessarily want to go there now.
             if (this != newCallNode) {
--- a/src/jdk/nashorn/internal/objects/Global.java	Tue Jul 08 16:30:42 2014 +0530
+++ b/src/jdk/nashorn/internal/objects/Global.java	Tue Jul 08 13:13:31 2014 +0200
@@ -908,7 +908,7 @@
      * @return the result of eval
      */
     public static Object eval(final Object self, final Object str) {
-        return directEval(self, str, UNDEFINED, UNDEFINED, UNDEFINED);
+        return directEval(self, str, UNDEFINED, UNDEFINED, false);
     }
 
     /**
@@ -924,14 +924,14 @@
      *
      * This is directly invoked from generated when eval(code) is called in user code
      */
-    public static Object directEval(final Object self, final Object str, final Object callThis, final Object location, final Object strict) {
+    public static Object directEval(final Object self, final Object str, final Object callThis, final Object location, final boolean strict) {
         if (!(str instanceof String || str instanceof ConsString)) {
             return str;
         }
         final Global global = Global.instanceFrom(self);
         final ScriptObject scope = self instanceof ScriptObject ? (ScriptObject)self : global;
 
-        return global.getContext().eval(scope, str.toString(), callThis, location, Boolean.TRUE.equals(strict), true);
+        return global.getContext().eval(scope, str.toString(), callThis, location, strict, true);
     }
 
     /**
--- a/test/script/basic/JDK-8047057.js	Tue Jul 08 16:30:42 2014 +0530
+++ b/test/script/basic/JDK-8047057.js	Tue Jul 08 13:13:31 2014 +0200
@@ -49,7 +49,7 @@
 makeFuncAndCall("L: { { break L; } return; }");
 makeFuncAndCall("L: { while(0) break L; return; }");
 makeFuncExpectError("L: {while(0) break L; return [](); }", TypeError);
-// makeFuncAndCall("do with({}) break ; while(0);");
+makeFuncAndCall("do with({}) break ; while(0);");
 makeFuncAndCall("while(0) with({}) continue ;");
 makeFuncAndCall("eval([]);");
 makeFuncAndCall("try{} finally{[]}");
@@ -59,10 +59,10 @@
 makeFuncAndCall("try { var x = 1, x = null; } finally { }");
 makeFuncAndCall("try { var x = {}, x = []; } catch(x3) { }");
 makeFuncAndCall("[delete this]");
-// makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
-// makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
-// makeFuncAndCall("eval(\"[,,];\", [11,12,13,14].some)");
-// makeFuncAndCall("eval(\"1.2e3\", ({})[ /x/ ])");
+makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
+makeFuncAndCall("if(eval('', eval('', function() {}))) { }");
+makeFuncAndCall("eval(\"[,,];\", [11,12,13,14].some)");
+makeFuncAndCall("eval(\"1.2e3\", ({})[ /x/ ])");
 makeFuncExpectError("eval(\"x4\", x3);", ReferenceError);
 makeFuncAndCall("with({5.0000000000000000000000: String()}){(false); }");
 makeFuncAndCall("try { var x = undefined, x = 5.0000000000000000000000; } catch(x) { x = undefined; }");
@@ -72,4 +72,4 @@
 makeFuncAndCall("with({8: 'fafafa'.replace()}){ }");
 makeFuncAndCall("(function (x) '' )(true)");
 makeFuncExpectError("new eval(function(){})", TypeError);
-//** makeFuncAndCall('eval("23", ({})[/x/])');
+makeFuncAndCall('eval("23", ({})[/x/])');
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/script/basic/JDK-8047067.js	Tue Jul 08 13:13:31 2014 +0200
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2010, 2014, 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-8047067: all eval arguments need to be copied in Lower
+ *
+ * @test
+ * @run
+ */
+
+// The second expression triggers optimistic deoptimization, and if not
+// all eval arguments were copied in Lower, we'd end up with duplicate
+// program points that'd cause incorrect continuation program point in
+// the rest-of, and therefore a bad stack, and therefore an AIOOBE in
+// the continuation setup code.
+eval("23", ({})[/x/])