Fri, 14 Jun 2013 13:53:40 +0200
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
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 +}