8047067: all eval arguments need to be copied in Lower
Reviewed-by: lagergren, sundar
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Tue Jul 08 16:30:42 2014 +0530
+++ b/nashorn/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/nashorn/src/jdk/nashorn/internal/codegen/Lower.java Tue Jul 08 16:30:42 2014 +0530
+++ b/nashorn/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/nashorn/src/jdk/nashorn/internal/ir/CallNode.java Tue Jul 08 16:30:42 2014 +0530
+++ b/nashorn/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;
- }
-
- private EvalArgs setCode(final Expression code) {
- if (this.code == code) {
- return this;
- }
- return new EvalArgs(code, evalThis, location, strictMode);
+ public List<Expression> getArgs() {
+ return Collections.unmodifiableList(args);
}
- /**
- * 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) {
+ private EvalArgs setArgs(final List<Expression> args) {
+ if (this.args == args) {
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/nashorn/src/jdk/nashorn/internal/objects/Global.java Tue Jul 08 16:30:42 2014 +0530
+++ b/nashorn/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/nashorn/test/script/basic/JDK-8047057.js Tue Jul 08 16:30:42 2014 +0530
+++ b/nashorn/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/nashorn/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/])