8016235: Use in catch block that may not have been executed in try block caused illegal byte code to be generated

Fri, 14 Jun 2013 13:53:40 +0200

author
lagergren
date
Fri, 14 Jun 2013 13:53:40 +0200
changeset 349
3efa56767847
parent 348
c5f783d83180
child 350
3d947baa33cc

8016235: Use in catch block that may not have been executed in try block caused illegal byte code to be generated
Reviewed-by: jlaskey, hannesw

src/jdk/nashorn/internal/codegen/Attr.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/ir/Symbol.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/parser/JSONParser.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/parser/Lexer.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8016235.js file | annotate | diff | comparison | revisions
     1.1 --- a/src/jdk/nashorn/internal/codegen/Attr.java	Thu Jun 13 20:50:24 2013 +0200
     1.2 +++ b/src/jdk/nashorn/internal/codegen/Attr.java	Fri Jun 14 13:53:40 2013 +0200
     1.3 @@ -35,6 +35,7 @@
     1.4  import static jdk.nashorn.internal.codegen.CompilerConstants.SWITCH_TAG_PREFIX;
     1.5  import static jdk.nashorn.internal.codegen.CompilerConstants.THIS;
     1.6  import static jdk.nashorn.internal.codegen.CompilerConstants.VARARGS;
     1.7 +import static jdk.nashorn.internal.ir.Symbol.IS_ALWAYS_DEFINED;
     1.8  import static jdk.nashorn.internal.ir.Symbol.IS_CONSTANT;
     1.9  import static jdk.nashorn.internal.ir.Symbol.IS_FUNCTION_SELF;
    1.10  import static jdk.nashorn.internal.ir.Symbol.IS_GLOBAL;
    1.11 @@ -128,6 +129,8 @@
    1.12  
    1.13      private final Deque<Type> returnTypes;
    1.14  
    1.15 +    private int catchNestingLevel;
    1.16 +
    1.17      private static final DebugLogger LOG   = new DebugLogger("attr");
    1.18      private static final boolean     DEBUG = LOG.isEnabled();
    1.19  
    1.20 @@ -169,14 +172,14 @@
    1.21          if (functionNode.isVarArg()) {
    1.22              initCompileConstant(VARARGS, body, IS_PARAM | IS_INTERNAL, Type.OBJECT_ARRAY);
    1.23              if (functionNode.needsArguments()) {
    1.24 -                initCompileConstant(ARGUMENTS, body, IS_VAR | IS_INTERNAL, Type.typeFor(ScriptObject.class));
    1.25 +                initCompileConstant(ARGUMENTS, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.typeFor(ScriptObject.class));
    1.26                  addLocalDef(ARGUMENTS.symbolName());
    1.27              }
    1.28          }
    1.29  
    1.30          initParameters(functionNode, body);
    1.31 -        initCompileConstant(SCOPE, body, IS_VAR | IS_INTERNAL, Type.typeFor(ScriptObject.class));
    1.32 -        initCompileConstant(RETURN, body, IS_VAR | IS_INTERNAL, Type.OBJECT);
    1.33 +        initCompileConstant(SCOPE, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.typeFor(ScriptObject.class));
    1.34 +        initCompileConstant(RETURN, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.OBJECT);
    1.35      }
    1.36  
    1.37  
    1.38 @@ -320,10 +323,12 @@
    1.39          final Block     block     = lc.getCurrentBlock();
    1.40  
    1.41          start(catchNode);
    1.42 +        catchNestingLevel++;
    1.43  
    1.44          // define block-local exception variable
    1.45 -        final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET);
    1.46 -        newType(def, Type.OBJECT);
    1.47 +        final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
    1.48 +        newType(def, Type.OBJECT); //we can catch anything, not just ecma exceptions
    1.49 +
    1.50          addLocalDef(exception.getName());
    1.51  
    1.52          return true;
    1.53 @@ -334,6 +339,9 @@
    1.54          final IdentNode exception = catchNode.getException();
    1.55          final Block  block        = lc.getCurrentBlock();
    1.56          final Symbol symbol       = findSymbol(block, exception.getName());
    1.57 +
    1.58 +        catchNestingLevel--;
    1.59 +
    1.60          assert symbol != null;
    1.61          return end(catchNode.setException((IdentNode)exception.setSymbol(lc, symbol)));
    1.62      }
    1.63 @@ -543,9 +551,19 @@
    1.64                  assert lc.getFunctionBody(functionNode).getExistingSymbol(CALLEE.symbolName()) != null;
    1.65                  lc.setFlag(functionNode.getBody(), Block.NEEDS_SELF_SYMBOL);
    1.66                  newType(symbol, FunctionNode.FUNCTION_TYPE);
    1.67 -            } else if (!identNode.isInitializedHere()) { // NASHORN-448
    1.68 -                // here is a use outside the local def scope
    1.69 -                if (!isLocalDef(name)) {
    1.70 +            } else if (!identNode.isInitializedHere()) {
    1.71 +                /*
    1.72 +                 * See NASHORN-448, JDK-8016235
    1.73 +                 *
    1.74 +                 * Here is a use outside the local def scope
    1.75 +                 * the inCatch check is a conservative approach to handle things that might have only been
    1.76 +                 * defined in the try block, but with variable declarations, which due to JavaScript rules
    1.77 +                 * have to be lifted up into the function scope outside the try block anyway, but as the
    1.78 +                 * flow can fault at almost any place in the try block and get us to the catch block, all we
    1.79 +                 * know is that we have a declaration, not a definition. This can be made better and less
    1.80 +                 * conservative once we superimpose a CFG onto the AST.
    1.81 +                 */
    1.82 +                if (!isLocalDef(name) || inCatch()) {
    1.83                      newType(symbol, Type.OBJECT);
    1.84                      symbol.setCanBeUndefined();
    1.85                  }
    1.86 @@ -572,6 +590,10 @@
    1.87          return end(identNode.setSymbol(lc, symbol));
    1.88      }
    1.89  
    1.90 +    private boolean inCatch() {
    1.91 +        return catchNestingLevel > 0;
    1.92 +    }
    1.93 +
    1.94      /**
    1.95       * If the symbol isn't already a scope symbol, and it is either not local to the current function, or it is being
    1.96       * referenced from within a with block, we force it to be a scope symbol.
    1.97 @@ -584,26 +606,26 @@
    1.98      }
    1.99  
   1.100      private boolean symbolNeedsToBeScope(Symbol symbol) {
   1.101 -        if(symbol.isThis() || symbol.isInternal()) {
   1.102 +        if (symbol.isThis() || symbol.isInternal()) {
   1.103              return false;
   1.104          }
   1.105          boolean previousWasBlock = false;
   1.106 -        for(final Iterator<LexicalContextNode> it = lc.getAllNodes(); it.hasNext();) {
   1.107 +        for (final Iterator<LexicalContextNode> it = lc.getAllNodes(); it.hasNext();) {
   1.108              final LexicalContextNode node = it.next();
   1.109 -            if(node instanceof FunctionNode) {
   1.110 +            if (node instanceof FunctionNode) {
   1.111                  // We reached the function boundary without seeing a definition for the symbol - it needs to be in
   1.112                  // scope.
   1.113                  return true;
   1.114 -            } else if(node instanceof WithNode) {
   1.115 -                if(previousWasBlock) {
   1.116 +            } else if (node instanceof WithNode) {
   1.117 +                if (previousWasBlock) {
   1.118                      // We reached a WithNode; the symbol must be scoped. Note that if the WithNode was not immediately
   1.119                      // preceded by a block, this means we're currently processing its expression, not its body,
   1.120                      // therefore it doesn't count.
   1.121                      return true;
   1.122                  }
   1.123                  previousWasBlock = false;
   1.124 -            } else if(node instanceof Block) {
   1.125 -                if(((Block)node).getExistingSymbol(symbol.getName()) == symbol) {
   1.126 +            } else if (node instanceof Block) {
   1.127 +                if (((Block)node).getExistingSymbol(symbol.getName()) == symbol) {
   1.128                      // We reached the block that defines the symbol without reaching either the function boundary, or a
   1.129                      // WithNode. The symbol need not be scoped.
   1.130                      return false;
   1.131 @@ -1700,8 +1722,8 @@
   1.132      }
   1.133  
   1.134      private void pushLocalsBlock() {
   1.135 -        localDefs.push(localDefs.isEmpty() ? new HashSet<String>() : new HashSet<>(localDefs.peek()));
   1.136 -        localUses.push(localUses.isEmpty() ? new HashSet<String>() : new HashSet<>(localUses.peek()));
   1.137 +        localDefs.push(new HashSet<>(localDefs.peek()));
   1.138 +        localUses.push(new HashSet<>(localUses.peek()));
   1.139      }
   1.140  
   1.141      private void popLocals() {
     2.1 --- a/src/jdk/nashorn/internal/ir/Symbol.java	Thu Jun 13 20:50:24 2013 +0200
     2.2 +++ b/src/jdk/nashorn/internal/ir/Symbol.java	Fri Jun 14 13:53:40 2013 +0200
     2.3 @@ -55,23 +55,25 @@
     2.4      public static final int KINDMASK = (1 << 3) - 1; // Kinds are represented by lower three bits
     2.5  
     2.6      /** Is this scope */
     2.7 -    public static final int IS_SCOPE         = 1 << 4;
     2.8 +    public static final int IS_SCOPE             = 1 <<  4;
     2.9      /** Is this a this symbol */
    2.10 -    public static final int IS_THIS          = 1 << 5;
    2.11 +    public static final int IS_THIS              = 1 <<  5;
    2.12      /** Can this symbol ever be undefined */
    2.13 -    public static final int CAN_BE_UNDEFINED = 1 << 6;
    2.14 +    public static final int CAN_BE_UNDEFINED     = 1 <<  6;
    2.15 +    /** Is this symbol always defined? */
    2.16 +    public static final int IS_ALWAYS_DEFINED    = 1 <<  8;
    2.17      /** Can this symbol ever have primitive types */
    2.18 -    public static final int CAN_BE_PRIMITIVE = 1 << 7;
    2.19 +    public static final int CAN_BE_PRIMITIVE     = 1 <<  9;
    2.20      /** Is this a let */
    2.21 -    public static final int IS_LET           = 1 << 8;
    2.22 +    public static final int IS_LET               = 1 << 10;
    2.23      /** Is this an internal symbol, never represented explicitly in source code */
    2.24 -    public static final int IS_INTERNAL      = 1 << 9;
    2.25 +    public static final int IS_INTERNAL          = 1 << 11;
    2.26      /** Is this a function self-reference symbol */
    2.27 -    public static final int IS_FUNCTION_SELF = 1 << 10;
    2.28 +    public static final int IS_FUNCTION_SELF     = 1 << 12;
    2.29      /** Is this a specialized param? */
    2.30 -    public static final int IS_SPECIALIZED_PARAM = 1 << 11;
    2.31 +    public static final int IS_SPECIALIZED_PARAM = 1 << 13;
    2.32      /** Is this symbol a shared temporary? */
    2.33 -    public static final int IS_SHARED = 1 << 12;
    2.34 +    public static final int IS_SHARED            = 1 << 14;
    2.35  
    2.36      /** Null or name identifying symbol. */
    2.37      private final String name;
    2.38 @@ -384,7 +386,7 @@
    2.39        * Mark this symbol as one being shared by multiple expressions. The symbol must be a temporary.
    2.40        */
    2.41       public void setIsShared() {
    2.42 -         if(!isShared()) {
    2.43 +         if (!isShared()) {
    2.44               assert isTemp();
    2.45               trace("SET IS SHARED");
    2.46               flags |= IS_SHARED;
    2.47 @@ -417,6 +419,14 @@
    2.48      }
    2.49  
    2.50      /**
    2.51 +     * Check if this symbol is always defined, which overrides all canBeUndefined tags
    2.52 +     * @return true if always defined
    2.53 +     */
    2.54 +    public boolean isAlwaysDefined() {
    2.55 +        return isParam() || (flags & IS_ALWAYS_DEFINED) == IS_ALWAYS_DEFINED;
    2.56 +    }
    2.57 +
    2.58 +    /**
    2.59       * Get the range for this symbol
    2.60       * @return range for symbol
    2.61       */
    2.62 @@ -462,7 +472,9 @@
    2.63       */
    2.64      public void setCanBeUndefined() {
    2.65          assert type.isObject() : type;
    2.66 -        if (!isParam() && !canBeUndefined()) {//parameters are never undefined
    2.67 +        if (isAlwaysDefined()) {
    2.68 +            return;
    2.69 +        } else if (!canBeUndefined()) {
    2.70              assert !isShared();
    2.71              flags |= CAN_BE_UNDEFINED;
    2.72          }
     3.1 --- a/src/jdk/nashorn/internal/parser/JSONParser.java	Thu Jun 13 20:50:24 2013 +0200
     3.2 +++ b/src/jdk/nashorn/internal/parser/JSONParser.java	Fri Jun 14 13:53:40 2013 +0200
     3.3 @@ -149,9 +149,9 @@
     3.4              @Override
     3.5              protected void scanNumber() {
     3.6                  // Record beginning of number.
     3.7 -                final int start = position;
     3.8 +                final int startPosition = position;
     3.9                  // Assume value is a decimal.
    3.10 -                TokenType type = TokenType.DECIMAL;
    3.11 +                TokenType valueType = TokenType.DECIMAL;
    3.12  
    3.13                  // floating point can't start with a "." with no leading digit before
    3.14                  if (ch0 == '.') {
    3.15 @@ -211,11 +211,11 @@
    3.16                          }
    3.17                      }
    3.18  
    3.19 -                    type = TokenType.FLOATING;
    3.20 +                    valueType = TokenType.FLOATING;
    3.21                  }
    3.22  
    3.23                  // Add number token.
    3.24 -                add(type, start);
    3.25 +                add(valueType, startPosition);
    3.26              }
    3.27  
    3.28              // ECMA 15.12.1.1 The JSON Lexical Grammar - JSONEscapeCharacter
     4.1 --- a/src/jdk/nashorn/internal/parser/Lexer.java	Thu Jun 13 20:50:24 2013 +0200
     4.2 +++ b/src/jdk/nashorn/internal/parser/Lexer.java	Fri Jun 14 13:53:40 2013 +0200
     4.3 @@ -919,6 +919,7 @@
     4.4  
     4.5      /**
     4.6       * Scan over a string literal.
     4.7 +     * @param add true if we nare not just scanning but should actually modify the token stream
     4.8       */
     4.9      protected void scanString(final boolean add) {
    4.10          // Type of string.
    4.11 @@ -1614,6 +1615,12 @@
    4.12          return null;
    4.13      }
    4.14  
    4.15 +    /**
    4.16 +     * Get the correctly localized error message for a given message id format arguments
    4.17 +     * @param msgId message id
    4.18 +     * @param args  format arguments
    4.19 +     * @return message
    4.20 +     */
    4.21      protected static String message(final String msgId, final String... args) {
    4.22          return ECMAErrors.getMessage("lexer.error." + msgId, args);
    4.23      }
     5.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     5.2 +++ b/test/script/basic/JDK-8016235.js	Fri Jun 14 13:53:40 2013 +0200
     5.3 @@ -0,0 +1,51 @@
     5.4 +/*
     5.5 + * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
     5.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     5.7 + * 
     5.8 + * This code is free software; you can redistribute it and/or modify it
     5.9 + * under the terms of the GNU General Public License version 2 only, as
    5.10 + * published by the Free Software Foundation.
    5.11 + * 
    5.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    5.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    5.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    5.15 + * version 2 for more details (a copy is included in the LICENSE file that
    5.16 + * accompanied this code).
    5.17 + * 
    5.18 + * You should have received a copy of the GNU General Public License version
    5.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    5.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    5.21 + * 
    5.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    5.23 + * or visit www.oracle.com if you need additional information or have any
    5.24 + * questions.
    5.25 + */
    5.26 +
    5.27 +/**
    5.28 + * JDK-8016235 : use before definition in catch block generated erroneous bytecode
    5.29 + * as there is no guarantee anything in the try block has executed. 
    5.30 + *
    5.31 + * @test
    5.32 + * @run 
    5.33 + */
    5.34 +
    5.35 +function f() {
    5.36 +    try {
    5.37 +	var parser = {};
    5.38 +    } catch (e) {
    5.39 +	parser = parser.context();
    5.40 +    }
    5.41 +}
    5.42 +
    5.43 +function g() { 
    5.44 +    try {
    5.45 +        return "apa";
    5.46 +    } catch (tmp) {
    5.47 +	//for now, too conservative as var ex declaration exists on the function
    5.48 +	//level, but at least the code does not break, and the analysis is driven
    5.49 +	//from the catch block (the rare case), not the try block (the common case)
    5.50 +        var ex = new Error("DOM Exception 5");
    5.51 +        ex.code = ex.number = 5;
    5.52 +        return ex;
    5.53 +    }
    5.54 +}

mercurial