8066222: too strong assertion on function expression names
authorattila
Wed, 03 Dec 2014 16:31:15 +0100
changeset 27822 a3866b9fb44d
parent 27821 87ac5547bbf3
child 27823 a591041cf33c
8066222: too strong assertion on function expression names Reviewed-by: hannesw, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/AssignSymbols.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/FunctionNode.java
nashorn/test/script/basic/JDK-8066222.js
nashorn/test/script/basic/JDK-8066222.js.EXPECTED
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/AssignSymbols.java	Wed Dec 03 14:49:36 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/AssignSymbols.java	Wed Dec 03 16:31:15 2014 +0100
@@ -135,15 +135,11 @@
             functionNode.compilerConstant(SCOPE).setNeedsSlot(false);
         }
         // Named function expressions that end up not referencing themselves won't need a local slot for the self symbol.
-        if(!functionNode.isDeclared() && !functionNode.usesSelfSymbol() && !functionNode.isAnonymous()) {
+        if(functionNode.isNamedFunctionExpression() && !functionNode.usesSelfSymbol()) {
             final Symbol selfSymbol = functionNode.getBody().getExistingSymbol(functionNode.getIdent().getName());
-            if(selfSymbol != null) {
-                if(selfSymbol.isFunctionSelf()) {
-                    selfSymbol.setNeedsSlot(false);
-                    selfSymbol.clearFlag(Symbol.IS_VAR);
-                }
-            } else {
-                assert functionNode.isProgram();
+            if(selfSymbol != null && selfSymbol.isFunctionSelf()) {
+                selfSymbol.setNeedsSlot(false);
+                selfSymbol.clearFlag(Symbol.IS_VAR);
             }
         }
         return functionNode;
@@ -490,20 +486,31 @@
         final Block body = lc.getCurrentBlock();
 
         initFunctionWideVariables(functionNode, body);
+        acceptDeclarations(functionNode, body);
+        defineFunctionSelfSymbol(functionNode, body);
+    }
 
-        if (!functionNode.isProgram() && !functionNode.isDeclared() && !functionNode.isAnonymous()) {
-            // It's neither declared nor program - it's a function expression then; assign it a self-symbol unless it's
-            // anonymous.
-            final String name = functionNode.getIdent().getName();
-            assert name != null;
-            assert body.getExistingSymbol(name) == null;
-            defineSymbol(body, name, functionNode, IS_VAR | IS_FUNCTION_SELF | HAS_OBJECT_VALUE);
-            if(functionNode.allVarsInScope()) { // basically, has deep eval
-                lc.setFlag(functionNode, FunctionNode.USES_SELF_SYMBOL);
-            }
+    private void defineFunctionSelfSymbol(final FunctionNode functionNode, final Block body) {
+        // Function self-symbol is only declared as a local variable for named function expressions. Declared functions
+        // don't need it as they are local variables in their declaring scope.
+        if (!functionNode.isNamedFunctionExpression()) {
+            return;
         }
 
-        acceptDeclarations(functionNode, body);
+        final String name = functionNode.getIdent().getName();
+        assert name != null; // As it's a named function expression.
+
+        if (body.getExistingSymbol(name) != null) {
+            // Body already has a declaration for the name. It's either a parameter "function x(x)" or a
+            // top-level variable "function x() { ... var x; ... }".
+            return;
+        }
+
+        defineSymbol(body, name, functionNode, IS_VAR | IS_FUNCTION_SELF | HAS_OBJECT_VALUE);
+        if(functionNode.allVarsInScope()) { // basically, has deep eval
+            // We must conservatively presume that eval'd code can dynamically use the function symbol.
+            lc.setFlag(functionNode, FunctionNode.USES_SELF_SYMBOL);
+        }
     }
 
     @Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/FunctionNode.java	Wed Dec 03 14:49:36 2014 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/FunctionNode.java	Wed Dec 03 16:31:15 2014 +0100
@@ -1091,6 +1091,15 @@
         return getFlag(USES_SELF_SYMBOL);
     }
 
+    /**
+     * Returns true if this is a named function expression (that is, it isn't a declared function, it isn't an
+     * anonymous function expression, and it isn't a program).
+     * @return true if this is a named function expression
+     */
+    public boolean isNamedFunctionExpression() {
+        return !getFlag(IS_PROGRAM | IS_ANONYMOUS | IS_DECLARED);
+    }
+
     @Override
     public Type getType(final Function<Symbol, Type> localVariableTypes) {
         return FUNCTION_TYPE;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066222.js	Wed Dec 03 16:31:15 2014 +0100
@@ -0,0 +1,35 @@
+/*
+ * 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-8066222: too strong assertion on function expression names
+ *
+ * @test
+ * @run
+ */
+
+// Has to print "SUCCESS"
+(function x (x){print(x)})("SUCCESS");
+
+// Has to print "undefined" and "1", not the function source in any case.
+(function x(){print(x); var x=1; print(x)})();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8066222.js.EXPECTED	Wed Dec 03 16:31:15 2014 +0100
@@ -0,0 +1,3 @@
+SUCCESS
+undefined
+1