8019819: scope symbol didn't get a slot in certain cases

Fri, 05 Jul 2013 15:10:47 +0200

author
attila
date
Fri, 05 Jul 2013 15:10:47 +0200
changeset 416
ce9cbe70f915
parent 415
edca88d3a03e
child 417
20b2c2dc20e8

8019819: scope symbol didn't get a slot in certain cases
Reviewed-by: hannesw, jlaskey, lagergren, sundar

src/jdk/nashorn/internal/codegen/Attr.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/codegen/CodeGenerator.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/codegen/CompilationPhase.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/codegen/FinalizeTypes.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/ir/FunctionNode.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/ir/LexicalContext.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/ir/Symbol.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/parser/Parser.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8019819.js file | annotate | diff | comparison | revisions
     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 +}

mercurial