Fri, 05 Jul 2013 15:10:47 +0200
8019819: scope symbol didn't get a slot in certain cases
Reviewed-by: hannesw, jlaskey, lagergren, sundar
1.1 --- a/src/jdk/nashorn/internal/codegen/Attr.java Fri Jul 05 14:36:54 2013 +0200 1.2 +++ b/src/jdk/nashorn/internal/codegen/Attr.java Fri Jul 05 15:10:47 2013 +0200 1.3 @@ -54,7 +54,6 @@ 1.4 import java.util.Iterator; 1.5 import java.util.List; 1.6 import java.util.Set; 1.7 - 1.8 import jdk.nashorn.internal.codegen.types.Type; 1.9 import jdk.nashorn.internal.ir.AccessNode; 1.10 import jdk.nashorn.internal.ir.BinaryNode; 1.11 @@ -343,10 +342,11 @@ 1.12 catchNestingLevel++; 1.13 1.14 // define block-local exception variable 1.15 - final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET | IS_ALWAYS_DEFINED); 1.16 + final String exname = exception.getName(); 1.17 + final Symbol def = defineSymbol(block, exname, IS_VAR | IS_LET | IS_ALWAYS_DEFINED); 1.18 newType(def, Type.OBJECT); //we can catch anything, not just ecma exceptions 1.19 1.20 - addLocalDef(exception.getName()); 1.21 + addLocalDef(exname); 1.22 1.23 return true; 1.24 } 1.25 @@ -678,7 +678,7 @@ 1.26 1.27 if (scopeBlock != null) { 1.28 assert lc.contains(scopeBlock); 1.29 - lc.setFlag(scopeBlock, Block.NEEDS_SCOPE); 1.30 + lc.setBlockNeedsScope(scopeBlock); 1.31 } 1.32 } 1.33 }
2.1 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Jul 05 14:36:54 2013 +0200 2.2 +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Jul 05 15:10:47 2013 +0200 2.3 @@ -881,9 +881,15 @@ 2.4 2.5 final FunctionNode function = lc.getCurrentFunction(); 2.6 if (isFunctionBody) { 2.7 - /* Fix the predefined slots so they have numbers >= 0, like varargs. */ 2.8 - if (function.needsParentScope()) { 2.9 - initParentScope(); 2.10 + if(method.hasScope()) { 2.11 + if (function.needsParentScope()) { 2.12 + method.loadCompilerConstant(CALLEE); 2.13 + method.invoke(ScriptFunction.GET_SCOPE); 2.14 + } else { 2.15 + assert function.hasScopeBlock(); 2.16 + method.loadNull(); 2.17 + } 2.18 + method.storeCompilerConstant(SCOPE); 2.19 } 2.20 if (function.needsArguments()) { 2.21 initArguments(function); 2.22 @@ -949,15 +955,6 @@ 2.23 protected void loadValue(final Symbol value) { 2.24 method.load(value); 2.25 } 2.26 - 2.27 - @Override 2.28 - protected void loadScope(MethodEmitter m) { 2.29 - if (function.needsParentScope()) { 2.30 - m.loadCompilerConstant(SCOPE); 2.31 - } else { 2.32 - m.loadNull(); 2.33 - } 2.34 - } 2.35 }.makeObject(method); 2.36 2.37 // runScript(): merge scope into global 2.38 @@ -998,12 +995,6 @@ 2.39 method.storeCompilerConstant(ARGUMENTS); 2.40 } 2.41 2.42 - private void initParentScope() { 2.43 - method.loadCompilerConstant(CALLEE); 2.44 - method.invoke(ScriptFunction.GET_SCOPE); 2.45 - method.storeCompilerConstant(SCOPE); 2.46 - } 2.47 - 2.48 @Override 2.49 public boolean enterFunctionNode(final FunctionNode functionNode) { 2.50 if (functionNode.isLazy()) {
3.1 --- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Jul 05 14:36:54 2013 +0200 3.2 +++ b/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Jul 05 15:10:47 2013 +0200 3.3 @@ -18,17 +18,15 @@ 3.4 import java.util.HashSet; 3.5 import java.util.List; 3.6 import java.util.Set; 3.7 - 3.8 import jdk.nashorn.internal.codegen.types.Range; 3.9 import jdk.nashorn.internal.codegen.types.Type; 3.10 -import jdk.nashorn.internal.ir.Block; 3.11 import jdk.nashorn.internal.ir.CallNode; 3.12 import jdk.nashorn.internal.ir.FunctionNode; 3.13 +import jdk.nashorn.internal.ir.FunctionNode.CompilationState; 3.14 import jdk.nashorn.internal.ir.LexicalContext; 3.15 +import jdk.nashorn.internal.ir.Node; 3.16 import jdk.nashorn.internal.ir.ReturnNode; 3.17 import jdk.nashorn.internal.ir.Symbol; 3.18 -import jdk.nashorn.internal.ir.FunctionNode.CompilationState; 3.19 -import jdk.nashorn.internal.ir.Node; 3.20 import jdk.nashorn.internal.ir.TemporarySymbols; 3.21 import jdk.nashorn.internal.ir.debug.ASTWriter; 3.22 import jdk.nashorn.internal.ir.debug.PrintVisitor; 3.23 @@ -117,7 +115,7 @@ 3.24 final FunctionNode parent = lc.getParentFunction(functionNode); 3.25 assert parent != null; 3.26 lc.setFlag(parent, FunctionNode.HAS_LAZY_CHILDREN); 3.27 - lc.setFlag(parent.getBody(), Block.NEEDS_SCOPE); 3.28 + lc.setBlockNeedsScope(parent.getBody()); 3.29 lc.setFlag(functionNode, FunctionNode.IS_LAZY); 3.30 return functionNode; 3.31 }
4.1 --- a/src/jdk/nashorn/internal/codegen/FinalizeTypes.java Fri Jul 05 14:36:54 2013 +0200 4.2 +++ b/src/jdk/nashorn/internal/codegen/FinalizeTypes.java Fri Jul 05 15:10:47 2013 +0200 4.3 @@ -31,7 +31,6 @@ 4.4 import java.util.ArrayList; 4.5 import java.util.HashSet; 4.6 import java.util.List; 4.7 - 4.8 import jdk.nashorn.internal.codegen.types.Type; 4.9 import jdk.nashorn.internal.ir.AccessNode; 4.10 import jdk.nashorn.internal.ir.Assignment; 4.11 @@ -415,10 +414,10 @@ 4.12 if (!functionNode.needsCallee()) { 4.13 functionNode.compilerConstant(CALLEE).setNeedsSlot(false); 4.14 } 4.15 - // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope or its 4.16 - // own scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope earlier than 4.17 - // this phase. 4.18 - if (!(functionNode.getBody().needsScope() || functionNode.needsParentScope())) { 4.19 + // Similar reasoning applies to __scope__ symbol: if the function doesn't need either parent scope and none of 4.20 + // its blocks create a scope, we ensure it doesn't get a slot, but we can't determine whether it needs a scope 4.21 + // earlier than this phase. 4.22 + if (!(functionNode.hasScopeBlock() || functionNode.needsParentScope())) { 4.23 functionNode.compilerConstant(SCOPE).setNeedsSlot(false); 4.24 } 4.25
5.1 --- a/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Jul 05 14:36:54 2013 +0200 5.2 +++ b/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Jul 05 15:10:47 2013 +0200 5.3 @@ -160,6 +160,10 @@ 5.4 /** Does a nested function contain eval? If it does, then all variables in this function might be get/set by it. */ 5.5 public static final int HAS_NESTED_EVAL = 1 << 6; 5.6 5.7 + /** Does this function have any blocks that create a scope? This is used to determine if the function needs to 5.8 + * have a local variable slot for the scope symbol. */ 5.9 + public static final int HAS_SCOPE_BLOCK = 1 << 7; 5.10 + 5.11 /** 5.12 * Flag this function as one that defines the identifier "arguments" as a function parameter or nested function 5.13 * name. This precludes it from needing to have an Arguments object defined as "arguments" local variable. Note that 5.14 @@ -577,7 +581,7 @@ 5.15 if(this.body == body) { 5.16 return this; 5.17 } 5.18 - return Node.replaceInLexicalContext(lc, this, new FunctionNode(this, lastToken, flags, name, returnType, compileUnit, compilationState, body, parameters, snapshot, hints)); 5.19 + 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)); 5.20 } 5.21 5.22 /** 5.23 @@ -638,6 +642,14 @@ 5.24 } 5.25 5.26 /** 5.27 + * Returns true if any of the blocks in this function create their own scope. 5.28 + * @return true if any of the blocks in this function create their own scope. 5.29 + */ 5.30 + public boolean hasScopeBlock() { 5.31 + return getFlag(HAS_SCOPE_BLOCK); 5.32 + } 5.33 + 5.34 + /** 5.35 * Return the kind of this function 5.36 * @see FunctionNode.Kind 5.37 * @return the kind
6.1 --- a/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Jul 05 14:36:54 2013 +0200 6.2 +++ b/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Jul 05 15:10:47 2013 +0200 6.3 @@ -54,13 +54,16 @@ 6.4 6.5 /** 6.6 * Set the flags for a lexical context node on the stack. Does not 6.7 - * replace the flags, but rather adds to them 6.8 + * replace the flags, but rather adds to them. 6.9 * 6.10 * @param node node 6.11 * @param flag new flag to set 6.12 */ 6.13 public void setFlag(final LexicalContextNode node, final int flag) { 6.14 if (flag != 0) { 6.15 + // Use setBlockNeedsScope() instead 6.16 + assert !(flag == Block.NEEDS_SCOPE && node instanceof Block); 6.17 + 6.18 for (int i = sp - 1; i >= 0; i--) { 6.19 if (stack[i] == node) { 6.20 flags[i] |= flag; 6.21 @@ -72,6 +75,29 @@ 6.22 } 6.23 6.24 /** 6.25 + * Marks the block as one that creates a scope. Note that this method must 6.26 + * be used instead of {@link #setFlag(LexicalContextNode, int)} with 6.27 + * {@link Block#NEEDS_SCOPE} because it atomically also sets the 6.28 + * {@link FunctionNode#HAS_SCOPE_BLOCK} flag on the block's containing 6.29 + * function. 6.30 + * @param block the block that needs to be marked as creating a scope. 6.31 + */ 6.32 + public void setBlockNeedsScope(final Block block) { 6.33 + for (int i = sp - 1; i >= 0; i--) { 6.34 + if (stack[i] == block) { 6.35 + flags[i] |= Block.NEEDS_SCOPE; 6.36 + for(int j = i - 1; j >=0; j --) { 6.37 + if(stack[j] instanceof FunctionNode) { 6.38 + flags[j] |= FunctionNode.HAS_SCOPE_BLOCK; 6.39 + return; 6.40 + } 6.41 + } 6.42 + } 6.43 + } 6.44 + assert false; 6.45 + } 6.46 + 6.47 + /** 6.48 * Get the flags for a lexical context node on the stack 6.49 * @param node node 6.50 * @return the flags for the node
7.1 --- a/src/jdk/nashorn/internal/ir/Symbol.java Fri Jul 05 14:36:54 2013 +0200 7.2 +++ b/src/jdk/nashorn/internal/ir/Symbol.java Fri Jul 05 15:10:47 2013 +0200 7.3 @@ -29,7 +29,6 @@ 7.4 import java.util.HashSet; 7.5 import java.util.Set; 7.6 import java.util.StringTokenizer; 7.7 - 7.8 import jdk.nashorn.internal.codegen.types.Range; 7.9 import jdk.nashorn.internal.codegen.types.Type; 7.10 import jdk.nashorn.internal.runtime.Context; 7.11 @@ -705,7 +704,7 @@ 7.12 public static void setSymbolIsScope(final LexicalContext lc, final Symbol symbol) { 7.13 symbol.setIsScope(); 7.14 if (!symbol.isGlobal()) { 7.15 - lc.setFlag(lc.getDefiningBlock(symbol), Block.NEEDS_SCOPE); 7.16 + lc.setBlockNeedsScope(lc.getDefiningBlock(symbol)); 7.17 } 7.18 } 7.19
8.1 --- a/src/jdk/nashorn/internal/parser/Parser.java Fri Jul 05 14:36:54 2013 +0200 8.2 +++ b/src/jdk/nashorn/internal/parser/Parser.java Fri Jul 05 15:10:47 2013 +0200 8.3 @@ -2957,7 +2957,7 @@ 8.4 } else { 8.5 lc.setFlag(fn, FunctionNode.HAS_NESTED_EVAL); 8.6 } 8.7 - lc.setFlag(lc.getFunctionBody(fn), Block.NEEDS_SCOPE); 8.8 + lc.setBlockNeedsScope(lc.getFunctionBody(fn)); 8.9 } 8.10 } 8.11
9.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 9.2 +++ b/test/script/basic/JDK-8019819.js Fri Jul 05 15:10:47 2013 +0200 9.3 @@ -0,0 +1,46 @@ 9.4 +/* 9.5 + * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved. 9.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 9.7 + * 9.8 + * This code is free software; you can redistribute it and/or modify it 9.9 + * under the terms of the GNU General Public License version 2 only, as 9.10 + * published by the Free Software Foundation. 9.11 + * 9.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 9.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 9.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 9.15 + * version 2 for more details (a copy is included in the LICENSE file that 9.16 + * accompanied this code). 9.17 + * 9.18 + * You should have received a copy of the GNU General Public License version 9.19 + * 2 along with this work; if not, write to the Free Software Foundation, 9.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 9.21 + * 9.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 9.23 + * or visit www.oracle.com if you need additional information or have any 9.24 + * questions. 9.25 + */ 9.26 + 9.27 +/** 9.28 + * JDK-8019819: scope symbol didn't get a slot in certain cases 9.29 + * 9.30 + * @test 9.31 + * @run 9.32 + */ 9.33 +function f() { 9.34 + try { 9.35 + } catch(e if [].g(e)) { 9.36 + with({}) { 9.37 + throw e; 9.38 + } 9.39 + } 9.40 +} 9.41 + 9.42 +function g() { 9.43 + try { 9.44 + } catch(e) { 9.45 + with({}) { 9.46 + throw e; 9.47 + } 9.48 + } 9.49 +}