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
This commit is contained in:
parent
2904cada12
commit
f74c3ecd82
@ -35,6 +35,7 @@ import static jdk.nashorn.internal.codegen.CompilerConstants.SCOPE;
|
||||
import static jdk.nashorn.internal.codegen.CompilerConstants.SWITCH_TAG_PREFIX;
|
||||
import static jdk.nashorn.internal.codegen.CompilerConstants.THIS;
|
||||
import static jdk.nashorn.internal.codegen.CompilerConstants.VARARGS;
|
||||
import static jdk.nashorn.internal.ir.Symbol.IS_ALWAYS_DEFINED;
|
||||
import static jdk.nashorn.internal.ir.Symbol.IS_CONSTANT;
|
||||
import static jdk.nashorn.internal.ir.Symbol.IS_FUNCTION_SELF;
|
||||
import static jdk.nashorn.internal.ir.Symbol.IS_GLOBAL;
|
||||
@ -128,6 +129,8 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
|
||||
private final Deque<Type> returnTypes;
|
||||
|
||||
private int catchNestingLevel;
|
||||
|
||||
private static final DebugLogger LOG = new DebugLogger("attr");
|
||||
private static final boolean DEBUG = LOG.isEnabled();
|
||||
|
||||
@ -169,14 +172,14 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
if (functionNode.isVarArg()) {
|
||||
initCompileConstant(VARARGS, body, IS_PARAM | IS_INTERNAL, Type.OBJECT_ARRAY);
|
||||
if (functionNode.needsArguments()) {
|
||||
initCompileConstant(ARGUMENTS, body, IS_VAR | IS_INTERNAL, Type.typeFor(ScriptObject.class));
|
||||
initCompileConstant(ARGUMENTS, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.typeFor(ScriptObject.class));
|
||||
addLocalDef(ARGUMENTS.symbolName());
|
||||
}
|
||||
}
|
||||
|
||||
initParameters(functionNode, body);
|
||||
initCompileConstant(SCOPE, body, IS_VAR | IS_INTERNAL, Type.typeFor(ScriptObject.class));
|
||||
initCompileConstant(RETURN, body, IS_VAR | IS_INTERNAL, Type.OBJECT);
|
||||
initCompileConstant(SCOPE, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.typeFor(ScriptObject.class));
|
||||
initCompileConstant(RETURN, body, IS_VAR | IS_INTERNAL | IS_ALWAYS_DEFINED, Type.OBJECT);
|
||||
}
|
||||
|
||||
|
||||
@ -320,10 +323,12 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
final Block block = lc.getCurrentBlock();
|
||||
|
||||
start(catchNode);
|
||||
catchNestingLevel++;
|
||||
|
||||
// define block-local exception variable
|
||||
final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET);
|
||||
newType(def, Type.OBJECT);
|
||||
final Symbol def = defineSymbol(block, exception.getName(), IS_VAR | IS_LET | IS_ALWAYS_DEFINED);
|
||||
newType(def, Type.OBJECT); //we can catch anything, not just ecma exceptions
|
||||
|
||||
addLocalDef(exception.getName());
|
||||
|
||||
return true;
|
||||
@ -334,6 +339,9 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
final IdentNode exception = catchNode.getException();
|
||||
final Block block = lc.getCurrentBlock();
|
||||
final Symbol symbol = findSymbol(block, exception.getName());
|
||||
|
||||
catchNestingLevel--;
|
||||
|
||||
assert symbol != null;
|
||||
return end(catchNode.setException((IdentNode)exception.setSymbol(lc, symbol)));
|
||||
}
|
||||
@ -543,9 +551,19 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
assert lc.getFunctionBody(functionNode).getExistingSymbol(CALLEE.symbolName()) != null;
|
||||
lc.setFlag(functionNode.getBody(), Block.NEEDS_SELF_SYMBOL);
|
||||
newType(symbol, FunctionNode.FUNCTION_TYPE);
|
||||
} else if (!identNode.isInitializedHere()) { // NASHORN-448
|
||||
// here is a use outside the local def scope
|
||||
if (!isLocalDef(name)) {
|
||||
} else if (!identNode.isInitializedHere()) {
|
||||
/*
|
||||
* See NASHORN-448, JDK-8016235
|
||||
*
|
||||
* Here is a use outside the local def scope
|
||||
* the inCatch check is a conservative approach to handle things that might have only been
|
||||
* defined in the try block, but with variable declarations, which due to JavaScript rules
|
||||
* have to be lifted up into the function scope outside the try block anyway, but as the
|
||||
* flow can fault at almost any place in the try block and get us to the catch block, all we
|
||||
* know is that we have a declaration, not a definition. This can be made better and less
|
||||
* conservative once we superimpose a CFG onto the AST.
|
||||
*/
|
||||
if (!isLocalDef(name) || inCatch()) {
|
||||
newType(symbol, Type.OBJECT);
|
||||
symbol.setCanBeUndefined();
|
||||
}
|
||||
@ -572,6 +590,10 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
return end(identNode.setSymbol(lc, symbol));
|
||||
}
|
||||
|
||||
private boolean inCatch() {
|
||||
return catchNestingLevel > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* If the symbol isn't already a scope symbol, and it is either not local to the current function, or it is being
|
||||
* referenced from within a with block, we force it to be a scope symbol.
|
||||
@ -584,26 +606,26 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
}
|
||||
|
||||
private boolean symbolNeedsToBeScope(Symbol symbol) {
|
||||
if(symbol.isThis() || symbol.isInternal()) {
|
||||
if (symbol.isThis() || symbol.isInternal()) {
|
||||
return false;
|
||||
}
|
||||
boolean previousWasBlock = false;
|
||||
for(final Iterator<LexicalContextNode> it = lc.getAllNodes(); it.hasNext();) {
|
||||
for (final Iterator<LexicalContextNode> it = lc.getAllNodes(); it.hasNext();) {
|
||||
final LexicalContextNode node = it.next();
|
||||
if(node instanceof FunctionNode) {
|
||||
if (node instanceof FunctionNode) {
|
||||
// We reached the function boundary without seeing a definition for the symbol - it needs to be in
|
||||
// scope.
|
||||
return true;
|
||||
} else if(node instanceof WithNode) {
|
||||
if(previousWasBlock) {
|
||||
} else if (node instanceof WithNode) {
|
||||
if (previousWasBlock) {
|
||||
// We reached a WithNode; the symbol must be scoped. Note that if the WithNode was not immediately
|
||||
// preceded by a block, this means we're currently processing its expression, not its body,
|
||||
// therefore it doesn't count.
|
||||
return true;
|
||||
}
|
||||
previousWasBlock = false;
|
||||
} else if(node instanceof Block) {
|
||||
if(((Block)node).getExistingSymbol(symbol.getName()) == symbol) {
|
||||
} else if (node instanceof Block) {
|
||||
if (((Block)node).getExistingSymbol(symbol.getName()) == symbol) {
|
||||
// We reached the block that defines the symbol without reaching either the function boundary, or a
|
||||
// WithNode. The symbol need not be scoped.
|
||||
return false;
|
||||
@ -1700,8 +1722,8 @@ final class Attr extends NodeOperatorVisitor<LexicalContext> {
|
||||
}
|
||||
|
||||
private void pushLocalsBlock() {
|
||||
localDefs.push(localDefs.isEmpty() ? new HashSet<String>() : new HashSet<>(localDefs.peek()));
|
||||
localUses.push(localUses.isEmpty() ? new HashSet<String>() : new HashSet<>(localUses.peek()));
|
||||
localDefs.push(new HashSet<>(localDefs.peek()));
|
||||
localUses.push(new HashSet<>(localUses.peek()));
|
||||
}
|
||||
|
||||
private void popLocals() {
|
||||
|
@ -55,23 +55,25 @@ public final class Symbol implements Comparable<Symbol> {
|
||||
public static final int KINDMASK = (1 << 3) - 1; // Kinds are represented by lower three bits
|
||||
|
||||
/** Is this scope */
|
||||
public static final int IS_SCOPE = 1 << 4;
|
||||
public static final int IS_SCOPE = 1 << 4;
|
||||
/** Is this a this symbol */
|
||||
public static final int IS_THIS = 1 << 5;
|
||||
public static final int IS_THIS = 1 << 5;
|
||||
/** Can this symbol ever be undefined */
|
||||
public static final int CAN_BE_UNDEFINED = 1 << 6;
|
||||
public static final int CAN_BE_UNDEFINED = 1 << 6;
|
||||
/** Is this symbol always defined? */
|
||||
public static final int IS_ALWAYS_DEFINED = 1 << 8;
|
||||
/** Can this symbol ever have primitive types */
|
||||
public static final int CAN_BE_PRIMITIVE = 1 << 7;
|
||||
public static final int CAN_BE_PRIMITIVE = 1 << 9;
|
||||
/** Is this a let */
|
||||
public static final int IS_LET = 1 << 8;
|
||||
public static final int IS_LET = 1 << 10;
|
||||
/** Is this an internal symbol, never represented explicitly in source code */
|
||||
public static final int IS_INTERNAL = 1 << 9;
|
||||
public static final int IS_INTERNAL = 1 << 11;
|
||||
/** Is this a function self-reference symbol */
|
||||
public static final int IS_FUNCTION_SELF = 1 << 10;
|
||||
public static final int IS_FUNCTION_SELF = 1 << 12;
|
||||
/** Is this a specialized param? */
|
||||
public static final int IS_SPECIALIZED_PARAM = 1 << 11;
|
||||
public static final int IS_SPECIALIZED_PARAM = 1 << 13;
|
||||
/** Is this symbol a shared temporary? */
|
||||
public static final int IS_SHARED = 1 << 12;
|
||||
public static final int IS_SHARED = 1 << 14;
|
||||
|
||||
/** Null or name identifying symbol. */
|
||||
private final String name;
|
||||
@ -384,7 +386,7 @@ public final class Symbol implements Comparable<Symbol> {
|
||||
* Mark this symbol as one being shared by multiple expressions. The symbol must be a temporary.
|
||||
*/
|
||||
public void setIsShared() {
|
||||
if(!isShared()) {
|
||||
if (!isShared()) {
|
||||
assert isTemp();
|
||||
trace("SET IS SHARED");
|
||||
flags |= IS_SHARED;
|
||||
@ -416,6 +418,14 @@ public final class Symbol implements Comparable<Symbol> {
|
||||
return (flags & KINDMASK) == IS_PARAM;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if this symbol is always defined, which overrides all canBeUndefined tags
|
||||
* @return true if always defined
|
||||
*/
|
||||
public boolean isAlwaysDefined() {
|
||||
return isParam() || (flags & IS_ALWAYS_DEFINED) == IS_ALWAYS_DEFINED;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the range for this symbol
|
||||
* @return range for symbol
|
||||
@ -462,7 +472,9 @@ public final class Symbol implements Comparable<Symbol> {
|
||||
*/
|
||||
public void setCanBeUndefined() {
|
||||
assert type.isObject() : type;
|
||||
if (!isParam() && !canBeUndefined()) {//parameters are never undefined
|
||||
if (isAlwaysDefined()) {
|
||||
return;
|
||||
} else if (!canBeUndefined()) {
|
||||
assert !isShared();
|
||||
flags |= CAN_BE_UNDEFINED;
|
||||
}
|
||||
|
@ -149,9 +149,9 @@ public class JSONParser extends AbstractParser {
|
||||
@Override
|
||||
protected void scanNumber() {
|
||||
// Record beginning of number.
|
||||
final int start = position;
|
||||
final int startPosition = position;
|
||||
// Assume value is a decimal.
|
||||
TokenType type = TokenType.DECIMAL;
|
||||
TokenType valueType = TokenType.DECIMAL;
|
||||
|
||||
// floating point can't start with a "." with no leading digit before
|
||||
if (ch0 == '.') {
|
||||
@ -211,11 +211,11 @@ public class JSONParser extends AbstractParser {
|
||||
}
|
||||
}
|
||||
|
||||
type = TokenType.FLOATING;
|
||||
valueType = TokenType.FLOATING;
|
||||
}
|
||||
|
||||
// Add number token.
|
||||
add(type, start);
|
||||
add(valueType, startPosition);
|
||||
}
|
||||
|
||||
// ECMA 15.12.1.1 The JSON Lexical Grammar - JSONEscapeCharacter
|
||||
|
@ -919,6 +919,7 @@ public class Lexer extends Scanner {
|
||||
|
||||
/**
|
||||
* Scan over a string literal.
|
||||
* @param add true if we nare not just scanning but should actually modify the token stream
|
||||
*/
|
||||
protected void scanString(final boolean add) {
|
||||
// Type of string.
|
||||
@ -1614,6 +1615,12 @@ public class Lexer extends Scanner {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the correctly localized error message for a given message id format arguments
|
||||
* @param msgId message id
|
||||
* @param args format arguments
|
||||
* @return message
|
||||
*/
|
||||
protected static String message(final String msgId, final String... args) {
|
||||
return ECMAErrors.getMessage("lexer.error." + msgId, args);
|
||||
}
|
||||
|
51
nashorn/test/script/basic/JDK-8016235.js
Normal file
51
nashorn/test/script/basic/JDK-8016235.js
Normal file
@ -0,0 +1,51 @@
|
||||
/*
|
||||
* 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-8016235 : use before definition in catch block generated erroneous bytecode
|
||||
* as there is no guarantee anything in the try block has executed.
|
||||
*
|
||||
* @test
|
||||
* @run
|
||||
*/
|
||||
|
||||
function f() {
|
||||
try {
|
||||
var parser = {};
|
||||
} catch (e) {
|
||||
parser = parser.context();
|
||||
}
|
||||
}
|
||||
|
||||
function g() {
|
||||
try {
|
||||
return "apa";
|
||||
} catch (tmp) {
|
||||
//for now, too conservative as var ex declaration exists on the function
|
||||
//level, but at least the code does not break, and the analysis is driven
|
||||
//from the catch block (the rare case), not the try block (the common case)
|
||||
var ex = new Error("DOM Exception 5");
|
||||
ex.code = ex.number = 5;
|
||||
return ex;
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user