# HG changeset patch # User attila # Date 1373029847 -7200 # Node ID ce9cbe70f915107e47e5f1ff3bcdb5b587fada21 # Parent edca88d3a03ee8d0b0dc21ea9cae22ac168cb311 8019819: scope symbol didn't get a slot in certain cases Reviewed-by: hannesw, jlaskey, lagergren, sundar diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/codegen/Attr.java --- a/src/jdk/nashorn/internal/codegen/Attr.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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); } } } diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/codegen/CodeGenerator.java --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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()) { diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/codegen/CompilationPhase.java --- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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; } diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/codegen/FinalizeTypes.java --- a/src/jdk/nashorn/internal/codegen/FinalizeTypes.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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); } diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/ir/FunctionNode.java --- a/src/jdk/nashorn/internal/ir/FunctionNode.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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 diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/ir/LexicalContext.java --- a/src/jdk/nashorn/internal/ir/LexicalContext.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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 diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/ir/Symbol.java --- a/src/jdk/nashorn/internal/ir/Symbol.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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)); } } diff -r edca88d3a03e -r ce9cbe70f915 src/jdk/nashorn/internal/parser/Parser.java --- a/src/jdk/nashorn/internal/parser/Parser.java Fri Jul 05 14:36:54 2013 +0200 +++ b/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)); } } diff -r edca88d3a03e -r ce9cbe70f915 test/script/basic/JDK-8019819.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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; + } + } +}