8066236: RuntimeNode forces copy creation on visitation

Reviewed-by: hannesw, lagergren

4 files changed, 57 insertions, 6 deletions

diff --git a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
index af1a8786..ac3c2934 100644
--- a/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
+++ b/src/jdk/nashorn/internal/codegen/LocalVariableTypesCalculator.java
@@ -93,6 +93,13 @@ import jdk.nashorn.internal.parser.TokenType;
  * variable to its widest used type after the join point. That would eliminate some widenings of undefined variables to
  * object, most notably those used only in loops. We need a full liveness analysis for that. Currently, we can establish
  * per-type liveness, which eliminates most of unwanted dead widenings.
+ * NOTE: the way this class is implemented, it actually processes the AST in two passes. The first pass is top-down and
+ * implemented in {@code enterXxx} methods. This pass does not mutate the AST (except for one occurrence, noted below),
+ * as being able to find relevant labels for control flow joins is sensitive to their reference identity, and mutated
+ * label-carrying nodes will create copies of their labels. A second bottom-up pass applying the changes is implemented
+ * in the separate visitor sitting in {@link #leaveFunctionNode(FunctionNode)}. This visitor will also instantiate new
+ * instances of the calculator to be run on nested functions (when not lazy compiling).
+ *
  */
 final class LocalVariableTypesCalculator extends NodeVisitor<LexicalContext>{
 
diff --git a/src/jdk/nashorn/internal/ir/RuntimeNode.java b/src/jdk/nashorn/internal/ir/RuntimeNode.java
index fd34c3ec..4eca8ff0 100644
--- a/src/jdk/nashorn/internal/ir/RuntimeNode.java
+++ b/src/jdk/nashorn/internal/ir/RuntimeNode.java
@@ -27,7 +27,6 @@ package jdk.nashorn.internal.ir;
 
 import static jdk.nashorn.internal.runtime.UnwarrantedOptimismException.INVALID_PROGRAM_POINT;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -468,11 +467,7 @@ public class RuntimeNode extends Expression implements Optimistic {
     @Override
     public Node accept(final NodeVisitor<? extends LexicalContext> visitor) {
         if (visitor.enterRuntimeNode(this)) {
-            final List<Expression> newArgs = new ArrayList<>();
-            for (final Node arg : args) {
-                newArgs.add((Expression)arg.accept(visitor));
-            }
-            return visitor.leaveRuntimeNode(setArgs(newArgs));
+            return visitor.leaveRuntimeNode(setArgs(Node.accept(visitor, args)));
         }
 
         return this;
diff --git a/test/script/basic/JDK-8066236.js b/test/script/basic/JDK-8066236.js
new file mode 100644
index 00000000..760e0f46
--- /dev/null
+++ b/test/script/basic/JDK-8066236.js
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 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-8066236: RuntimeNode forces copy creation on visitation
+ *
+ * @test
+ * @run
+ */
+
+// Note: we're using Function("code") instead of (function(){ code }) so that
+// we don't trigger parser API validation in JDK-8008448 tests. The test code
+// encapsulated in functions below can't be correctly handled by the parser API
+// currently, as it contains parser-generated REFERENCE_ERROR runtime nodes.
+try {
+    Function("L: {this = x;break L}")();
+} catch (e) {
+   print("threw ReferenceError: " + (e instanceof ReferenceError));
+}
+try {
+    Function("L:with(this--)break L;")();
+} catch (e) {
+   print("threw ReferenceError: " + (e instanceof ReferenceError));
+}
+Function("L:with(Object in Object)break L;")();
+print("SUCCESS");
diff --git a/test/script/basic/JDK-8066236.js.EXPECTED b/test/script/basic/JDK-8066236.js.EXPECTED
new file mode 100644
index 00000000..b64a3056
--- /dev/null
+++ b/test/script/basic/JDK-8066236.js.EXPECTED
@@ -0,0 +1,3 @@
+threw ReferenceError: true
+threw ReferenceError: true
+SUCCESS