8024846: keep separate internal arguments variable

Reviewed-by: lagergren, sundar

4 files changed, 115 insertions, 40 deletions

diff --git a/src/jdk/nashorn/internal/codegen/Attr.java b/src/jdk/nashorn/internal/codegen/Attr.java
index 88e904dc..813d3fc9 100644
--- a/src/jdk/nashorn/internal/codegen/Attr.java
+++ b/src/jdk/nashorn/internal/codegen/Attr.java
@@ -26,6 +26,7 @@
 package jdk.nashorn.internal.codegen;
 
 import static jdk.nashorn.internal.codegen.CompilerConstants.ARGUMENTS;
+import static jdk.nashorn.internal.codegen.CompilerConstants.ARGUMENTS_VAR;
 import static jdk.nashorn.internal.codegen.CompilerConstants.CALLEE;
 import static jdk.nashorn.internal.codegen.CompilerConstants.EXCEPTION_PREFIX;
 import static jdk.nashorn.internal.codegen.CompilerConstants.ITERATOR_PREFIX;
@@ -172,7 +173,9 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
             initCompileConstant(VARARGS, body, IS_PARAM | IS_INTERNAL);
             if (functionNode.needsArguments()) {
                 initCompileConstant(ARGUMENTS, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED);
-                addLocalDef(ARGUMENTS.symbolName());
+                final String argumentsName = ARGUMENTS_VAR.symbolName();
+                newType(defineSymbol(body, argumentsName, IS_VAR | IS_ALWAYS_DEFINED), Type.typeFor(ARGUMENTS_VAR.type()));
+                addLocalDef(argumentsName);
             }
         }
 
@@ -491,30 +494,29 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
             objectifySymbols(body);
         }
 
-        if (body.getFlag(Block.NEEDS_SELF_SYMBOL)) {
-            final IdentNode callee = compilerConstant(CALLEE);
-            VarNode selfInit =
-                new VarNode(
-                    newFunctionNode.getLineNumber(),
-                    newFunctionNode.getToken(),
-                    newFunctionNode.getFinish(),
-                    newFunctionNode.getIdent(),
-                    callee);
-
-            LOG.info("Accepting self symbol init ", selfInit, " for ", newFunctionNode.getName());
-
-            final List<Statement> newStatements = new ArrayList<>();
-            assert callee.getSymbol() != null && callee.getSymbol().hasSlot();
+        List<VarNode> syntheticInitializers = null;
 
-            final IdentNode name       = selfInit.getName();
-            final Symbol    nameSymbol = body.getExistingSymbol(name.getName());
-
-            assert nameSymbol != null;
+        if (body.getFlag(Block.NEEDS_SELF_SYMBOL)) {
+            syntheticInitializers = new ArrayList<>(2);
+            LOG.info("Accepting self symbol init for ", newFunctionNode.getName());
+            // "var fn = :callee"
+            syntheticInitializers.add(createSyntheticInitializer(newFunctionNode.getIdent(), CALLEE, newFunctionNode));
+        }
 
-            selfInit = selfInit.setName((IdentNode)name.setSymbol(lc, nameSymbol));
+        if(newFunctionNode.needsArguments()) {
+            if(syntheticInitializers == null) {
+                syntheticInitializers = new ArrayList<>(1);
+            }
+            // "var arguments = :arguments"
+            syntheticInitializers.add(createSyntheticInitializer(createImplicitIdentifier(ARGUMENTS_VAR.symbolName()),
+                    ARGUMENTS, newFunctionNode));
+        }
 
-            newStatements.add(selfInit);
-            newStatements.addAll(body.getStatements());
+        if(syntheticInitializers != null) {
+            final List<Statement> stmts = body.getStatements();
+            final List<Statement> newStatements = new ArrayList<>(stmts.size() + syntheticInitializers.size());
+            newStatements.addAll(syntheticInitializers);
+            newStatements.addAll(stmts);
             newFunctionNode = newFunctionNode.setBody(lc, body.setStatements(lc, newStatements));
         }
 
@@ -533,6 +535,28 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
         return newFunctionNode;
     }
 
+    /**
+     * Creates a synthetic initializer for a variable (a var statement that doesn't occur in the source code). Typically
+     * used to create assignmnent of {@code :callee} to the function name symbol in self-referential function
+     * expressions as well as for assignment of {@code :arguments} to {@code arguments}.
+     *
+     * @param name the ident node identifying the variable to initialize
+     * @param initConstant the compiler constant it is initialized to
+     * @param fn the function node the assignment is for
+     * @return a var node with the appropriate assignment
+     */
+    private VarNode createSyntheticInitializer(final IdentNode name, final CompilerConstants initConstant, final FunctionNode fn) {
+        final IdentNode init = compilerConstant(initConstant);
+        assert init.getSymbol() != null && init.getSymbol().hasSlot();
+
+        VarNode synthVar = new VarNode(fn.getLineNumber(), fn.getToken(), fn.getFinish(), name, init);
+
+        final Symbol nameSymbol = fn.getBody().getExistingSymbol(name.getName());
+        assert nameSymbol != null;
+
+        return synthVar.setName((IdentNode)name.setSymbol(lc, nameSymbol));
+    }
+
     @Override
     public Node leaveCONVERT(final UnaryNode unaryNode) {
         assert false : "There should be no convert operators in IR during Attribution";
@@ -974,15 +998,18 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
     }
 
     private IdentNode compilerConstant(CompilerConstants cc) {
-        final FunctionNode functionNode = lc.getCurrentFunction();
-        return (IdentNode)
-            new IdentNode(
-                functionNode.getToken(),
-                functionNode.getFinish(),
-                cc.symbolName()).
-                setSymbol(
-                    lc,
-                    functionNode.compilerConstant(cc));
+        return (IdentNode)createImplicitIdentifier(cc.symbolName()).setSymbol(lc, lc.getCurrentFunction().compilerConstant(cc));
+    }
+
+    /**
+     * Creates an ident node for an implicit identifier within the function (one not declared in the script source
+     * code). These identifiers are defined with function's token and finish.
+     * @param name the name of the identifier
+     * @return an ident node representing the implicit identifier.
+     */
+    private IdentNode createImplicitIdentifier(final String name) {
+        final FunctionNode fn = lc.getCurrentFunction();
+        return new IdentNode(fn.getToken(), fn.getFinish(), name);
     }
 
     @Override
diff --git a/src/jdk/nashorn/internal/codegen/CompilerConstants.java b/src/jdk/nashorn/internal/codegen/CompilerConstants.java
index 5207fbce..fb347c75 100644
--- a/src/jdk/nashorn/internal/codegen/CompilerConstants.java
+++ b/src/jdk/nashorn/internal/codegen/CompilerConstants.java
@@ -102,8 +102,12 @@ public enum CompilerConstants {
     /** the varargs variable when necessary */
     VARARGS(":varargs", Object[].class),
 
-    /** the arguments vector when necessary and the slot */
-    ARGUMENTS("arguments", ScriptObject.class, 2),
+    /** the arguments variable (visible to function body). Initially set to ARGUMENTS, but can be reassigned by code in
+     * the function body.*/
+    ARGUMENTS_VAR("arguments", Object.class),
+
+    /** the internal arguments object, when necessary (not visible to scripts, can't be reassigned). */
+    ARGUMENTS(":arguments", ScriptObject.class),
 
     /** prefix for iterators for for (x in ...) */
     ITERATOR_PREFIX(":i", Iterator.class),
diff --git a/src/jdk/nashorn/internal/parser/Parser.java b/src/jdk/nashorn/internal/parser/Parser.java
index ea01f598..d0519171 100644
--- a/src/jdk/nashorn/internal/parser/Parser.java
+++ b/src/jdk/nashorn/internal/parser/Parser.java
@@ -25,7 +25,6 @@
 
 package jdk.nashorn.internal.parser;
 
-import static jdk.nashorn.internal.codegen.CompilerConstants.ARGUMENTS;
 import static jdk.nashorn.internal.codegen.CompilerConstants.EVAL;
 import static jdk.nashorn.internal.codegen.CompilerConstants.FUNCTION_PREFIX;
 import static jdk.nashorn.internal.codegen.CompilerConstants.RUN_SCRIPT;
@@ -115,6 +114,8 @@ import jdk.nashorn.internal.runtime.Timing;
  * Builds the IR.
  */
 public class Parser extends AbstractParser {
+    private static final String ARGUMENTS_NAME = CompilerConstants.ARGUMENTS_VAR.symbolName();
+
     /** Current script environment. */
     private final ScriptEnvironment env;
 
@@ -511,13 +512,19 @@ loop:
      * @param ident Referenced property.
      */
     private void detectSpecialProperty(final IdentNode ident) {
-        final String name = ident.getName();
-
-        if (ARGUMENTS.symbolName().equals(name)) {
+        if (isArguments(ident)) {
             lc.setFlag(lc.getCurrentFunction(), FunctionNode.USES_ARGUMENTS);
         }
     }
 
+    private static boolean isArguments(final String name) {
+        return ARGUMENTS_NAME.equals(name);
+    }
+
+    private static boolean isArguments(final IdentNode ident) {
+        return isArguments(ident.getName());
+    }
+
     /**
      * Tells whether a IdentNode can be used as L-value of an assignment
      *
@@ -2449,7 +2456,7 @@ loop:
             } else if (env._function_statement == ScriptEnvironment.FunctionStatementBehavior.WARNING) {
                 warning(JSErrorType.SYNTAX_ERROR, AbstractParser.message("no.func.decl.here.warn"), functionToken);
             }
-            if (ARGUMENTS.symbolName().equals(name.getName())) {
+            if (isArguments(name)) {
                 lc.setFlag(lc.getCurrentFunction(), FunctionNode.DEFINES_ARGUMENTS);
             }
         }
@@ -2468,7 +2475,7 @@ loop:
                 final IdentNode parameter = parameters.get(i);
                 String parameterName = parameter.getName();
 
-                if (ARGUMENTS.symbolName().equals(parameterName)) {
+                if (isArguments(parameterName)) {
                     functionNode = functionNode.setFlag(lc, FunctionNode.DEFINES_ARGUMENTS);
                 }
 
@@ -2486,7 +2493,7 @@ loop:
                 parametersSet.add(parameterName);
             }
         } else if (arity == 1) {
-            if (ARGUMENTS.symbolName().equals(parameters.get(0).getName())) {
+            if (isArguments(parameters.get(0))) {
                 functionNode = functionNode.setFlag(lc, FunctionNode.DEFINES_ARGUMENTS);
             }
         }
diff --git a/test/script/basic/JDK-8024846.js b/test/script/basic/JDK-8024846.js
new file mode 100644
index 00000000..ccdf8fcc
--- /dev/null
+++ b/test/script/basic/JDK-8024846.js
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2010, 2013, 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-8024846: keep separate internal arguments variable
+ *
+ * @test
+ */
+
+function f() { print(arguments); }
+
+function func(obj) {
+    var arguments = obj;
+    for (var i in arguments) {
+    }
+    return obj;
+}