8019819: scope symbol didn't get a slot in certain cases
Reviewed-by: hannesw, jlaskey, lagergren, sundar
--- a/nashorn/src/jdk/nashorn/internal/codegen/Attr.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/Attr.java Fri Jul 05 15:10:47 2013 +0200
@@ -54,7 +54,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
-
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.ir.AccessNode;
import jdk.nashorn.internal.ir.BinaryNode;
@@ -343,10 +342,11 @@
catchNestingLevel++;
// define block-local exception variable
- final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
+ final String exname = exception.getName();
+ final Symbol def = defineSymbol(block, exname, IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
newType(def, Type.OBJECT); //we can catch anything, not just ecma exceptions
- addLocalDef(exception.getName());
+ addLocalDef(exname);
return true;
}
@@ -678,7 +678,7 @@
if (scopeBlock != null) {
assert lc.contains(scopeBlock);
- lc.setFlag(scopeBlock, Block.NEEDS_SCOPE);
+ lc.setBlockNeedsScope(scopeBlock);
}
}
}
--- a/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Jul 05 15:10:47 2013 +0200
@@ -881,9 +881,15 @@
final FunctionNode function = lc.getCurrentFunction();
if (isFunctionBody) {
- /* Fix the predefined slots so they have numbers >= 0, like varargs. */
- if (function.needsParentScope()) {
- initParentScope();
+ if(method.hasScope()) {
+ if (function.needsParentScope()) {
+ method.loadCompilerConstant(CALLEE);
+ method.invoke(ScriptFunction.GET_SCOPE);
+ } else {
+ assert function.hasScopeBlock();
+ method.loadNull();
+ }
+ method.storeCompilerConstant(SCOPE);
}
if (function.needsArguments()) {
initArguments(function);
@@ -949,15 +955,6 @@
protected void loadValue(final Symbol value) {
method.load(value);
}
-
- @Override
- protected void loadScope(MethodEmitter m) {
- if (function.needsParentScope()) {
- m.loadCompilerConstant(SCOPE);
- } else {
- m.loadNull();
- }
- }
}.makeObject(method);
// runScript(): merge scope into global
@@ -998,12 +995,6 @@
method.storeCompilerConstant(ARGUMENTS);
}
- private void initParentScope() {
- method.loadCompilerConstant(CALLEE);
- method.invoke(ScriptFunction.GET_SCOPE);
- method.storeCompilerConstant(SCOPE);
- }
-
@Override
public boolean enterFunctionNode(final FunctionNode functionNode) {
if (functionNode.isLazy()) {
--- a/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Jul 05 15:10:47 2013 +0200
@@ -18,17 +18,15 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
-
import jdk.nashorn.internal.codegen.types.Range;
import jdk.nashorn.internal.codegen.types.Type;
-import jdk.nashorn.internal.ir.Block;
import jdk.nashorn.internal.ir.CallNode;
import jdk.nashorn.internal.ir.FunctionNode;
+import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
import jdk.nashorn.internal.ir.LexicalContext;
+import jdk.nashorn.internal.ir.Node;
import jdk.nashorn.internal.ir.ReturnNode;
import jdk.nashorn.internal.ir.Symbol;
-import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
-import jdk.nashorn.internal.ir.Node;
import jdk.nashorn.internal.ir.TemporarySymbols;
import jdk.nashorn.internal.ir.debug.ASTWriter;
import jdk.nashorn.internal.ir.debug.PrintVisitor;
@@ -117,7 +115,7 @@
final FunctionNode parent = lc.getParentFunction(functionNode);
assert parent != null;
lc.setFlag(parent, FunctionNode.HAS_LAZY_CHILDREN);
- lc.setFlag(parent.getBody(), Block.NEEDS_SCOPE);
+ lc.setBlockNeedsScope(parent.getBody());
lc.setFlag(functionNode, FunctionNode.IS_LAZY);
return functionNode;
}
--- a/nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/codegen/FinalizeTypes.java Fri Jul 05 15:10:47 2013 +0200
@@ -31,7 +31,6 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
-
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.ir.AccessNode;
import jdk.nashorn.internal.ir.Assignment;
@@ -415,10 +414,10 @@
if (!functionNode.needsCallee()) {
functionNode.compilerConstant(CALLEE).setNeedsSlot(false);
}
- // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope or its
- // own scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope earlier than
- // this phase.
- if (!(functionNode.getBody().needsScope() || functionNode.needsParentScope())) {
+ // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope and none of
+ // its blocks create a scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope
+ // earlier than this phase.
+ if (!(functionNode.hasScopeBlock() || functionNode.needsParentScope())) {
functionNode.compilerConstant(SCOPE).setNeedsSlot(false);
}
--- a/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Jul 05 15:10:47 2013 +0200
@@ -160,6 +160,10 @@
/** Does a nested function contain eval? If it does, then all variables in this function might be get/set by it. */
public static final int HAS_NESTED_EVAL = 1 << 6;
+ /** Does this function have any blocks that create a scope? This is used to determine if the function needs to
+ * have a local variable slot for the scope symbol. */
+ public static final int HAS_SCOPE_BLOCK = 1 << 7;
+
/**
* Flag this function as one that defines the identifier "arguments" as a function parameter or nested function
* name. This precludes it from needing to have an Arguments object defined as "arguments" local variable. Note that
@@ -577,7 +581,7 @@
if(this.body == body) {
return this;
}
- return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
+ return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags | (body.needsScope() ? FunctionNode.HAS_SCOPE_BLOCK : 0), name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints));
}
/**
@@ -638,6 +642,14 @@
}
/**
+ * Returns true if any of the blocks in this function create their own scope.
+ * @return true if any of the blocks in this function create their own scope.
+ */
+ public boolean hasScopeBlock() {
+ return getFlag(HAS_SCOPE_BLOCK);
+ }
+
+ /**
* Return the kind of this function
* @see FunctionNode.Kind
* @return the kind
--- a/nashorn/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Jul 05 15:10:47 2013 +0200
@@ -54,13 +54,16 @@
/**
* Set the flags for a lexical context node on the stack. Does not
- * replace the flags, but rather adds to them
+ * replace the flags, but rather adds to them.
*
* @param node node
* @param flag new flag to set
*/
public void setFlag(final LexicalContextNode node, final int flag) {
if (flag != 0) {
+ // Use setBlockNeedsScope() instead
+ assert !(flag == Block.NEEDS_SCOPE && node instanceof Block);
+
for (int i = sp - 1; i >= 0; i--) {
if (stack[i] == node) {
flags[i] |= flag;
@@ -72,6 +75,29 @@
}
/**
+ * Marks the block as one that creates a scope. Note that this method must
+ * be used instead of {@link #setFlag(LexicalContextNode, int)} with
+ * {@link Block#NEEDS_SCOPE} because it atomically also sets the
+ * {@link FunctionNode#HAS_SCOPE_BLOCK} flag on the block's containing
+ * function.
+ * @param block the block that needs to be marked as creating a scope.
+ */
+ public void setBlockNeedsScope(final Block block) {
+ for (int i = sp - 1; i >= 0; i--) {
+ if (stack[i] == block) {
+ flags[i] |= Block.NEEDS_SCOPE;
+ for(int j = i - 1; j >=0; j --) {
+ if(stack[j] instanceof FunctionNode) {
+ flags[j] |= FunctionNode.HAS_SCOPE_BLOCK;
+ return;
+ }
+ }
+ }
+ }
+ assert false;
+ }
+
+ /**
* Get the flags for a lexical context node on the stack
* @param node node
* @return the flags for the node
--- a/nashorn/src/jdk/nashorn/internal/ir/Symbol.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/ir/Symbol.java Fri Jul 05 15:10:47 2013 +0200
@@ -29,7 +29,6 @@
import java.util.HashSet;
import java.util.Set;
import java.util.StringTokenizer;
-
import jdk.nashorn.internal.codegen.types.Range;
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.runtime.Context;
@@ -705,7 +704,7 @@
public static void setSymbolIsScope(final LexicalContext lc, final Symbol symbol) {
symbol.setIsScope();
if (!symbol.isGlobal()) {
- lc.setFlag(lc.getDefiningBlock(symbol), Block.NEEDS_SCOPE);
+ lc.setBlockNeedsScope(lc.getDefiningBlock(symbol));
}
}
--- a/nashorn/src/jdk/nashorn/internal/parser/Parser.java Fri Jul 05 14:36:54 2013 +0200
+++ b/nashorn/src/jdk/nashorn/internal/parser/Parser.java Fri Jul 05 15:10:47 2013 +0200
@@ -2957,7 +2957,7 @@
} else {
lc.setFlag(fn, FunctionNode.HAS_NESTED_EVAL);
}
- lc.setFlag(lc.getFunctionBody(fn), Block.NEEDS_SCOPE);
+ lc.setBlockNeedsScope(lc.getFunctionBody(fn));
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8019819.js Fri Jul 05 15:10:47 2013 +0200
@@ -0,0 +1,46 @@
+/*
+ * 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-8019819: scope symbol didn't get a slot in certain cases
+ *
+ * @test
+ * @run
+ */
+function f() {
+ try {
+ } catch(e if [].g(e)) {
+ with({}) {
+ throw e;
+ }
+ }
+}
+
+function g() {
+ try {
+ } catch(e) {
+ with({}) {
+ throw e;
+ }
+ }
+}